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: Make some stratis tests nondestructive #19487

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

martinpitt
Copy link
Member

@martinpitt martinpitt commented Oct 16, 2023

test{Basic,Alerts,Cli} don't reboot and thus can run in our nondestructive VMs and on the Testing Farm. Move the others into a new TestStorageStratisReboot test class (that way around, the pixel tests have a lot less renaming noise). Change them to use loop devices, and ensure that failed tests clean up after themselves by stopping+destroying all leftover pools and creating mount points in vm_tmpdir.

This paves the way for doing reverse dependency testing for stratis changes.

https://issues.redhat.com/browse/COCKPIT-1070


There's unfortunately some pixel noise, as the dialog previously had the "QEMU ..." name, and now with the loop devices they don't have a serial/label; and there's one rename.

@martinpitt
Copy link
Member Author

martinpitt commented Oct 16, 2023

Hmm, I could avoid some pixel noise by keeping TestStorageStratis as-is, and renaming the other class to TestStorageStratisReboot

Update: Done.

@martinpitt martinpitt force-pushed the stratis branch 2 times, most recently from c3bbe1a to f7ea2a0 Compare October 16, 2023 08:09
@martinpitt martinpitt added the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Oct 16, 2023
@martinpitt
Copy link
Member Author

Works on our CI now, but still fails on TF. Missing stratis dependency at least.

@martinpitt martinpitt force-pushed the stratis branch 3 times, most recently from ea82fe6 to 7d34829 Compare October 16, 2023 10:35
@martinpitt
Copy link
Member Author

martinpitt commented Oct 16, 2023

This is a "fun" failure. There is some leaked /dev/loop12 (possibly from TestStoragePartitions.testPartitions? -- no, it's not, the test would fail earlier otherwise), and that makes .sidepanel-row:contains(/dev/loop1)" ambiguous. Unfortunately the sidebar doesn't have an attribute with the precise device name.

Update: This debug log shows that it's not a leak, it's just that losetup decides to actually allocate these names:

XXXX testBasic loop assignment /dev/loop0 /dev/loop12 /dev/loop1 /dev/loop2 /dev/loop3

I'll try to fix them.

@martinpitt martinpitt removed the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Oct 16, 2023
@martinpitt martinpitt marked this pull request as ready for review October 16, 2023 15:32
test{Basic,Alerts,Cli} don't reboot and thus can run in our
nondestructive VMs and on the Testing Farm. Move the others into a new
TestStorageStratisReboot test class (that way around, the pixel tests
have a lot less renaming noise). Change them to use loop devices, and
ensure that failed tests clean up after themselves by
stopping+destroying all leftover pools and creating mount points in
vm_tmpdir.

Change the test to always start stratisd.service, as the newly installed
package in TF doesn't auto-start the unit.

This paves the way for doing reverse dependency testing for stratis
changes.

https://issues.redhat.com/browse/COCKPIT-1070
@mvollmer
Copy link
Member

Unfortunately the sidebar doesn't have an attribute with the precise device name.

The redesign will fix this! :-)

Copy link
Member

@mvollmer mvollmer left a comment

Choose a reason for hiding this comment

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

Thanks!

@mvollmer mvollmer merged commit 7e01711 into cockpit-project:main Oct 17, 2023
@martinpitt martinpitt deleted the stratis branch October 17, 2023 08:00
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