Clone of #43 to test LGTM #55
Conversation
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.)
It seems to check only for Python 3 bugs for the storage role, according to https://lgtm.com/help/lgtm/analysis-faqs#how-python-version-identified it supports only one Python version and the logs for the storage role check show that it selected Python 3 here: https://lgtm.com/projects/g/linux-system-roles/storage/logs/languages/lang:python Python 2 could be specifically selected: |
|
@tyll how is then possible that it found a very similar problem in the network role, here: https://github.com/linux-system-roles/network/pull/131/files#diff-d32c4e7fad7beec907c9d463e17228d4R1376 ? Is it using Python 2 for network? If so, it should probably be configured to use Python 2 for all the roles. |
|
#62 contains a config file to select python 2, you can include the commit here to test if it properly identifies the expected bugs. |
LGTM defaults to Python 3 for the storage role because it was created recently. Since it needs to support Python 2 as well, configure it explicitly.
pcahyna
force-pushed
the
non-destructive
branch
from
de7516a to
3363b26
Compare
|
This pull request introduces 3 alerts when merging 3363b26 into 378239c - view on LGTM.com new alerts:
|
|
@tyll it works, thanks, the relevant alert is "1 for 'super' in old style class". What is surprising is that despite the correct alerts, the test still shows as passed (green). |
No description provided.