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

Non destructive disk test improvements #54

Conversation

pcahyna
Copy link
Member

@pcahyna pcahyna commented Oct 21, 2019

Includes #43 and #51

dwlehman and others added 28 commits September 25, 2019 14:58
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.
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.
It seems that the "get required packages" task (invocation of blivet with
packages_only: true) silently (without even reporting changed: true) changes
mounts. Add an assert for that. (It would be better to make it a separate unit
test.)
Otherwise super() fails.

Not necessary with Python 3, but necessary with Python 2
(where classic classes are the default)
Copy link
Collaborator

@dwlehman dwlehman left a comment

Choose a reason for hiding this comment

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

Weird, I'm pretty sure I did this at some point as well. Looks good.

@dwlehman
Copy link
Collaborator

Added to #43.

@dwlehman dwlehman closed this Oct 23, 2019
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.

2 participants