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

webui: tests: extend the TestInstallationProgress test to include reboot #4981

Merged
merged 2 commits into from
Aug 8, 2023

Conversation

jelly
Copy link
Contributor

@jelly jelly commented Jul 28, 2023

No description provided.

@jelly
Copy link
Contributor Author

jelly commented Jul 28, 2023

https://access.redhat.com/solutions/41976 might be a solution for our non-working rebooting

@KKoukiou KKoukiou changed the title WebUI: test: add an installation test webui: tests: extend the TestInstallationProgress test to include reboot Aug 3, 2023
@KKoukiou KKoukiou marked this pull request as ready for review August 3, 2023 12:29
@KKoukiou KKoukiou force-pushed the allow-reboot branch 2 times, most recently from 98128e9 to a147a2d Compare August 3, 2023 14:13
@KKoukiou KKoukiou requested a review from M4rtinK August 3, 2023 14:36
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

@martinpitt can you help me with the last remaining test failure here?

The problem is that if they installation fails, and we reboot the machine, the destination disk is unbootable, and the test fails on teardown because of broken ssh connection.

Is there a way to tell the test to skip teardown maybe?

@M4rtinK
Copy link
Contributor

M4rtinK commented Aug 7, 2023

Don't end to end test already support & perform machine reboot & simple booted system testing ? I am a bit confused on which level this testing flow works so that it adds another implementation of the reboot logic. Could it be perhaps shared with what the end-to-end tests use ? :)

@KKoukiou
Copy link
Contributor

KKoukiou commented Aug 7, 2023

Don't end to end test already support & perform machine reboot & simple booted system testing ? I am a bit confused on which level this testing flow works so that it adds another implementation of the reboot logic. Could it be perhaps shared with what the end-to-end tests use ? :)

This is already shared with what e2e tests use. The the helper file moved from the e2e tests direcotyr to the shared folder.

@martinpitt
Copy link
Contributor

martinpitt commented Aug 7, 2023

The problem is that if they installation fails, and we reboot the machine, the destination disk is unbootable, and the test fails on teardown because of broken ssh connection.

Is there a way to tell the test to skip teardown maybe?

Not right now. You have to modify testlib.py to support some "expect broken machine" flag in MachineCase, which you can set inside a test. Then tearDown() could check that flag and skip the explicit cleanup.

However, that won't help for the staged things to addCleanup(), for that you'd need to rummage in unittest.TestCase's guts to delete the handler list.

Would it perhaps be easier to avoid the situation by not rebooting the VM in tests where it's known and expected to fail anyway?

For making this possible add `--wait` option to virt-install invocation [0].
This now allows the VM to reboot when reboot is requested, whilst
previously it was just shutting down.

[0] https://listman.redhat.com/archives/virt-tools-list/2011-November/002869.html

Now that the VM reboots when reboot is requests it's not possible to
test the "Quit" button as on boot.iso the Quit button actually reboots
the VM into the unbootable disk (as the installation did not finish).
The later state will fail the test teardown, as the target VM is not
reachable.
Therefore the tests checking the 'Quit' button were removed.

Co-authored-by: Katerina Koukiou <kkoukiou@redhat.com>
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

@M4rtinK do you have more concerns on this PR apart from the comments which I answered bellow?

@KKoukiou
Copy link
Contributor

KKoukiou commented Aug 8, 2023

/kickstart-test --waive webui-only

@KKoukiou KKoukiou merged commit 76d3f3d into rhinstaller:master Aug 8, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants