Many fixes for the test in PR #43 #46
Conversation
pcahyna
force-pushed
the
non-destructive-test
branch
from
caf9da2 to
38feb52
Compare
A type of None means no formatting. An exception is when there is a blkid-reported type that blivet doesn't have special handling for, in which case type will be None but the name attribute will reflect the type reported by blkid. In this special case we treat the formatting as though it had a non-None type.
tests/tests_disk_errors.yml
Outdated
| disks: "{{ unused_disks }}" | ||
| # type: partition | ||
| volumes: | ||
| - name: bar |
There was a problem hiding this comment.
I think we already have a test that does this for LVM, no?
There was a problem hiding this comment.
This is very well possible. The test did not work as it was (with partition) so I made a small change to improve it.
tests/tests_disk_errors.yml
Outdated
| type: partition | ||
| # type: partition | ||
| volumes: | ||
| - name: bar |
There was a problem hiding this comment.
It's implemented, just an incorrect key in the class lookup dict. It's fixed in #43.
There was a problem hiding this comment.
I will revert this part then
While both before and after it should trigger an error in the role, and therefore both are valid tests, a valid array with a nonexistent disk is more likely to be what was intended, and therefore preferable. (The former variant actually checked for bad syntax, not for handling of nonexistent disks.)
The latter would be a valid test as well, but a less important one, and currently it does not work properly. Do not use ignore_errors: yes on the test block, it makes all assert pointless. Use a rescue task instead.
- this makes all test asserts pointless. Instead catch the error using rescue. Actually verify that data heve not been destroyed.
and check for errors properly. Verify again that data have not been lost. Do not use a pool type of partition, seems to be unimplemented. Create one volume in the pool, the role does not cope with empty pools. Note that even with those changes the role application fails - apparently it refuses to create a VG when there is already a filesystem, even with safe mode off.
|
I did a force-push to update 'Fix logic of safe mode guard on reformat.' Picking that up should help at least with the failure CI is hitting now. |
@dwlehman Where did you push it? I don't see any new commit. |
|
Sorry, I see it now. |
pcahyna
force-pushed
the
non-destructive-test
branch
from
38feb52 to
0494d06
Compare
|
Yeah, sorry about that. I didn't realize you were following my work so closely at the time. I won't be doing any more force-pushes to avoid the confusion they can cause. |
|
No problem, just please avoid touching the same test. But since you need a test for your work, I think including this PR in your PR would be better. By the way the rebase did not help. It still fails (unmounts the filesystem and therefore the test believes it lost data). |
|
By the way, I created a separate assert for the problem that is now causing the test to fail: #47 |
To be applied on top of it.
Note that the tests still fail despite the fixes. The original version passed by accident - ignore_errors was set on all relevant tasks.