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

tests: some stabilization changes #655

Merged
merged 12 commits into from
Mar 3, 2025

Conversation

KKoukiou
Copy link
Contributor

No description provided.

@KKoukiou KKoukiou force-pushed the udevadm_settle_tests branch 9 times, most recently from fab841f to 64ea325 Compare February 21, 2025 08:27
@KKoukiou KKoukiou marked this pull request as ready for review February 21, 2025 08:38
@KKoukiou KKoukiou requested review from martinpitt and adamkankovsky and removed request for martinpitt February 21, 2025 08:47
adamkankovsky
adamkankovsky previously approved these changes Feb 21, 2025
Copy link
Contributor

@adamkankovsky adamkankovsky left a comment

Choose a reason for hiding this comment

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

For some reason the tests started crashing in the part where s.udevadm_settle() was separated, but maybe it's just a flake

@adamkankovsky adamkankovsky dismissed their stale review February 21, 2025 10:08

For some reason the tests started crashing in the part where s.udevadm_settle() was separated, but maybe it's just a flake

@KKoukiou KKoukiou force-pushed the udevadm_settle_tests branch from 64ea325 to 88b8ab3 Compare February 21, 2025 13:06
@KKoukiou
Copy link
Contributor Author

For some reason the tests started crashing in the part where s.udevadm_settle() was separated, but maybe it's just a flake

Yes found the issue, thanks.

@KKoukiou KKoukiou force-pushed the udevadm_settle_tests branch 7 times, most recently from 3c0db92 to dd65e4f Compare February 24, 2025 17:47
@KKoukiou KKoukiou force-pushed the udevadm_settle_tests branch 3 times, most recently from 705dc8b to 4400c4d Compare February 25, 2025 06:59
@KKoukiou KKoukiou force-pushed the udevadm_settle_tests branch 8 times, most recently from f05a8b9 to 4529323 Compare February 27, 2025 11:03
@KKoukiou
Copy link
Contributor Author

I know the single remaining failure problem, I will fix it tomorrow.

… and teardown

* Ensure disks all disks are removed on both setup and teardown
* Run udevadm settle after disk adding/removal operations
* When adding disks, specify the target (vda, vdb). Often disks were
  added with /dev/vdb altough /dev/vda was expected
I suspect there is some cleanup missing for these.
I believe there is some race condition with blivet not parsing the partitions
propertly.
@KKoukiou KKoukiou force-pushed the udevadm_settle_tests branch from 2fc320b to 91f793a Compare February 28, 2025 08:03
Copy link
Contributor

@adamkankovsky adamkankovsky left a comment

Choose a reason for hiding this comment

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

The code looks good, thank you very much for the changes for code readability and fixing flakes. The only thing that might be useful is to squash some commits into each other to remove duplicates.

Copy link
Contributor

@rvykydal rvykydal left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you. I like the setup moved to setup.

@KKoukiou
Copy link
Contributor Author

KKoukiou commented Mar 3, 2025

TestStorageMountPointsRAID_E2E seems to have broken in the meantime.
I will debug seperately to recover first from the completely red CI

@KKoukiou KKoukiou merged commit 9906e26 into rhinstaller:main Mar 3, 2025
15 of 17 checks passed
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.

3 participants