-
Notifications
You must be signed in to change notification settings - Fork 74
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
Makefile: Update Cockpit lib to e8cf93050356f; fix adding disks to transient VM, and lots of test bugs #1778
Makefile: Update Cockpit lib to e8cf93050356f; fix adding disks to transient VM, and lots of test bugs #1778
Conversation
cf2069d
to
d0b5402
Compare
The |
Most failures are shallow (unclosed dialogs and such). TestMachinesDisks.testAddDiskDirPool is much harder: It's a race condition which happens to work most of the time with Chromium, but if you wait/sit after the What's not obvious is how to properly wait for the |
d0b5402
to
5960bb9
Compare
Not done yet, but pushing what I have as I need to run out for a bit. |
Selecting a new action from the main page while a dialog is open only worked with our `ph_mouse()` cheat, not with proper clicks.
Otherwise the list obscures the dialog's action buttons, so they are not clickable (only with `ph_mouse()` which circumvents the browser).
5960bb9
to
e794803
Compare
Meh, this is a completely stupid failure:
That's nonsense -- plenty of other methods in the same class use |
The disk kebabs keep jumping around while the Disk table initializes and re-renders, which is a race condition when trying to click on them with the actual browser mouse. So robustify the tests to wait for the table to be complete. This mitigates the issue, and also provides some nice extra validation. This was already done in a few places, generalize this into a `waitDisks()` helper. Unfortunately in some cases waiting for the entries is still not enough, but I can't figure out what else to wait on.. Do an extra sleep for the table to settle down.
e794803
to
cce3bc3
Compare
cce3bc3
to
a07c76a
Compare
phew I figured out |
TestMachinesDisks.testAddDiskDirPool's "transient VM" test case ommediately opened the "Add Disk" dialog after undefining the VM. This was a race condition which the test actually "won" in most cases: The `UNDEFINED` event did not yet bubble through the UI, so the "Add Disk" dialog still thought that the VM was persistent. Fix that by waiting until the UI picked up the event and considers the VM transient. This exposed a bug in the dialog's fill() function: It tried to check the value of the "Permanent" checkbox, but that doesn't exist at all for transient VMs. Fix up the logic and assert the latter for transient VMs. Now that this test is working correctly, it uncovered a bug in the code: Trying to add a disk to a transient VM fails with > Requested operation is not valid: transient domains do not have any persistent config because the `persistent` state was initialized to "true". Fix that. https://issues.redhat.com/browse/RHEL-17677
a07c76a
to
504c26b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, all changes make sense.
No description provided.