Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: lvm pool members test fix #449

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

japokorn
Copy link
Collaborator

@japokorn japokorn commented Jun 6, 2024

tests_lvm_pool_members started to fail. It tried to create a device with a requested size (20m) that was less than minimal allowed size (300m) for that type of volume. Role automatically resized the device to allowed size. That lead to discrepancy in actual and expected size values.

Increasing the requested device size to be same or larger than minimal fixes the issue.

Copy link

codecov bot commented Jun 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 11.64%. Comparing base (acd20be) to head (64d6890).
Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #449      +/-   ##
==========================================
- Coverage   16.54%   11.64%   -4.91%     
==========================================
  Files           2        8       +6     
  Lines         284     1795    +1511     
  Branches       79        0      -79     
==========================================
+ Hits           47      209     +162     
- Misses        237     1586    +1349     
Flag Coverage Δ
sanity ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@japokorn
Copy link
Collaborator Author

japokorn commented Jun 6, 2024

[citest]

@spetrosi
Copy link
Contributor

spetrosi commented Jun 6, 2024

Sounds like it should be a tests: rather than fix: because it doesn't fix anything in the role. Please change the PR title.

Copy link
Collaborator

@vojtechtrefny vojtechtrefny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. XFS recently changes its minimal size requirement to 300 MiB so that's the reason behind this.

@japokorn japokorn changed the title fix: lvm pool members test fix test: lvm pool members test fix Jun 6, 2024
tests_lvm_pool_members started to fail. It tried to create a device with
a requested size (20m) that was less than minimal allowed size (300m) for that type of
volume. Role automatically resized the device to allowed size. That lead to discrepancy
in actual and expected size values.

Increasing the requested device size to be same or larger than minimal fixes the
issue.
@japokorn
Copy link
Collaborator Author

japokorn commented Jun 6, 2024

Changed PR name and commit message to start with test instead of fix

@japokorn japokorn merged commit ee740b7 into linux-system-roles:main Jun 6, 2024
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants