Replace blivet tests with custom GUI tests #51

2021-10-06T04:41:28Z

akatch commented

2021-10-06 04:41:28 +00:00

(Migrated from github.com)

Description

This pull request includes all resources needed to replace the blivet partitioning GUI tests with tests written specifically for the Rocky-included "custom" partitioning GUI. Fixes #9.

How has this been tested?

I reloaded the FIF templates and executed the tests as follows:

./fifloader.py --clean --load templates.fif.json
openqa-cli api -X POST isos ISO=Rocky-8.4-x86_64-dvd1.iso ARCH=x86_64 DISTRI=rocky FLAVOR=dvd-iso VERSION=8.4 BUILD=$(date +%Y%m%d.%H%M%S).0 PACKAGE_SET=server

With this PR, the suites install_custom_gui_lvm_ext4@uefi and install_custom_gui_lvm_ext4@64bit softfail with some AVCs, but otherwise pass.

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules

## Description This pull request includes all resources needed to replace the blivet partitioning GUI tests with tests written specifically for the Rocky-included "custom" partitioning GUI. Fixes #9. ## How has this been tested? I reloaded the FIF templates and executed the tests as follows: ``` ./fifloader.py --clean --load templates.fif.json openqa-cli api -X POST isos ISO=Rocky-8.4-x86_64-dvd1.iso ARCH=x86_64 DISTRI=rocky FLAVOR=dvd-iso VERSION=8.4 BUILD=$(date +%Y%m%d.%H%M%S).0 PACKAGE_SET=server ``` With this PR, the suites `install_custom_gui_lvm_ext4@uefi` and `install_custom_gui_lvm_ext4@64bit` softfail with some AVCs, but otherwise pass. ## Checklist - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] Any dependent changes have been merged and published in downstream modules

lumarel commented

2021-10-06 20:42:51 +00:00

(Migrated from github.com)

As we talked already in MM/IRC, beside a few needles which could have a better detection box, everything looks really good up to now!

tcooper commented

2021-10-07 06:08:54 +00:00

(Migrated from github.com)

install_custom_gui_lvm_ext4@64bit was a soft fail (only) for the avc: denied { getattr } for pid=### comm="login" we are often seeing otherwise disk_custom_lvm_ext4_postinstall passed.

`install_custom_gui_lvm_ext4@64bit` was a soft fail (only) for the `avc: denied { getattr } for pid=### comm="login"` we are often seeing otherwise `disk_custom_lvm_ext4_postinstall` passed.

lumarel commented

2021-10-11 14:20:28 +00:00

(Migrated from github.com)

Ran another round, also after the rebase everything looks good!

akatch (Migrated from github.com) reviewed 2021-10-21 03:51:39 +00:00

lib/anaconda.pm

akatch (Migrated from github.com) commented

2021-10-21 03:51:39 +00:00

This is probably a Perl question, but I wanted to call out this behavior and get some team insight. Currently the raidl arg is specified as an int; I suspect specifying it as a string would make this problem go away, but that feels lazy. What is the correct way to handle this behavior in this case?

This is probably a Perl question, but I wanted to call out this behavior and get some team insight. Currently the `raidl` arg is specified as an int; I suspect specifying it as a string would make this problem go away, but that feels lazy. What is the correct way to handle this behavior in this case?

tcooper (Migrated from github.com) reviewed 2021-10-21 14:44:18 +00:00

lib/anaconda.pm

tcooper (Migrated from github.com) commented

2021-10-21 14:44:17 +00:00

My guess without digging further is that this is intended as a boolean value (Perl doesn't have a true Boolean type) where (int)0 = FALSE and all other values are TRUE in the if() statement. Looking through other test files (like the postinstall check) it seems only RAID1 is currently supported although this isn't clearly documented.

My guess without digging further is that this is intended as a boolean value (Perl doesn't have a true Boolean type) where `(int)0` = `FALSE` and all other values are `TRUE` in the `if()` statement. Looking through other test files (like the postinstall check) it seems only RAID1 is currently supported although this isn't clearly documented.

lumarel commented

2021-10-21 19:04:42 +00:00

(Migrated from github.com)

Okay I can confirm now, all test suites behave normal!
The new tests and needles work as they should 👌🏻

Okay I can confirm now, all test suites behave normal! The new tests and needles work as they should 👌🏻

lumarel (Migrated from github.com) reviewed 2021-10-21 19:16:24 +00:00

lib/anaconda.pm

lumarel (Migrated from github.com) commented

2021-10-21 19:16:24 +00:00

I looked a bit further through the code and I have to say... I found a mismatch 🤔
The only place such similar parameter is used is in disk_custom_gui_software_raid.pm:23 which uses the parameter raidl (most likely for raidlevel) with the integer 1 for I suppose raid level 1

This means we should either rewrite the one usage in line disk_custom_gui_software_raid.pm:23 or might be better, for future awareness, change raid1 in the lines of anaconda.pm 185, 191 and 233 to raidl,
uncomment lines 238 and 239 and comment or remove 240.

I looked a bit further through the code and I have to say... I found a mismatch 🤔 The only place such similar parameter is used is in `disk_custom_gui_software_raid.pm:23` which uses the parameter `raidl` (most likely for raidlevel) with the integer 1 for I suppose raid level 1 This means we should either rewrite the one usage in line `disk_custom_gui_software_raid.pm:23` or might be better, for future awareness, change `raid1` in the lines of anaconda.pm 185, 191 and 233 to `raidl`, uncomment lines 238 and 239 and comment or remove 240.

lumarel (Migrated from github.com) reviewed 2021-10-21 19:31:38 +00:00

lib/anaconda.pm

lumarel (Migrated from github.com) commented

2021-10-21 19:31:38 +00:00

Just have noticed that the whole section is new, as well as the whole disk_custom_guid_software_raid.pm ... so yeah if I reread what I have written, I'm still on the point that it should be called raidl or raidlevel, as I think this should not be limited to raid 1
Off course this will make more work to make sure you have enough disks for the different raidlevels but I just wouldn't make it limited to raid1 🤔

Just have noticed that the whole section is new, as well as the whole disk_custom_guid_software_raid.pm ... so yeah if I reread what I have written, I'm still on the point that it should be called `raidl` or `raidlevel`, as I think this should not be limited to raid 1 Off course this will make more work to make sure you have enough disks for the different raidlevels but I just wouldn't make it limited to raid1 🤔

lumarel commented

2021-10-23 22:56:42 +00:00

(Migrated from github.com)

Looked through everything again, and also checked the raid install in detail, looks good ✔

🎉 1

tcooper (Migrated from github.com) reviewed 2021-10-25 04:57:05 +00:00

needles/anaconda/partitioning/custom-gui/rocky-anaconda_custom_part_add-20211008.json

						
				@ -0,0 +12,4 @@

				  "tags": [

				    "ENV-DISTRI-rocky",

				    "LANGUAGE-english",

				    "anaconda_custom_part_add"

tcooper (Migrated from github.com) commented

2021-10-25 04:57:05 +00:00

Not critical but you didn't specify LANGUAGE-english in this needle.

Not critical but you didn't specify `LANGUAGE-english` in this needle.

tcooper (Migrated from github.com) reviewed 2021-10-25 05:09:36 +00:00

needles/anaconda/partitioning/custom-gui/rocky-anaconda_custom_size_unit-20211005.json

tcooper (Migrated from github.com) commented

2021-10-25 05:09:36 +00:00

It's unclear to me why there are three needles matching for anaconda_custom_size. They have slightly different locations and the two have a click but one does not. In my disk_custom_gui_standard_partition_ext4@x86_64 test the needle matched here is rocky-anaconda_custom_size-20211010. Are rocky-anaconda_custom_size-20211005 and rocky-anaconda_custom_size-20211007 needed at all?

It's unclear to me why there are three needles matching for `anaconda_custom_size`. They have slightly different locations and the two have a click but one does not. In my `disk_custom_gui_standard_partition_ext4@x86_64` test the needle matched here is `rocky-anaconda_custom_size-20211010`. Are `rocky-anaconda_custom_size-20211005` and `rocky-anaconda_custom_size-20211007` needed at all?

tcooper (Migrated from github.com) reviewed 2021-10-25 05:16:10 +00:00

tcooper (Migrated from github.com) left a comment

After some backflips in my test machine (all my own issues) all non-UEFI passed in the partitioning steps for @x86_64 (I cannot test UEFI or aarch64). Tests had soft fails for AVC in _consolve_avc_crash only.

See comments about possibly unneeded needles. It might be good now to see if / where these are actually matching (if they are) as it seems unnecessary to have three different needles for the same match (unless it is).

An explanation of that and plan to address (if needed) is all that should be necessary to get full approval and merge.

After some backflips in my test machine (all my own issues) all non-UEFI passed in the partitioning steps for `@x86_64` (I cannot test UEFI or aarch64). Tests had soft fails for AVC in `_consolve_avc_crash` only. See comments about possibly unneeded needles. It might be good now to see if / where these are actually matching (if they are) as it seems unnecessary to have three different needles for the same match (unless it is). An explanation of that and plan to address (if needed) is all that should be necessary to get full approval and merge.

akatch (Migrated from github.com) reviewed 2021-10-25 23:20:52 +00:00

needles/anaconda/partitioning/custom-gui/rocky-anaconda_custom_size_unit-20211005.json

akatch (Migrated from github.com) commented

2021-10-25 23:20:52 +00:00

I don't think those needles are needed at all. I have removed them in my most recent commit and am rerunning all the tests to make sure nothing broke.

Sign in to join this conversation.

No reviewers