check-storage-msdos: Fix race by waiting for UI to update #9359
Conversation
|
ubuntu-stable seems to have a relevant test failure (same test), which I am able to reproduce locally on fedora-28. That said, I have no idea how your changes might have caused that. Probably there is more here that needs fixing...
|
|
@allisonkarlitskaya , this will be most probably fixed by #9360 . |
From the journal: This might have been fixed in more recent versions of UDisks2, it's probably worth trying to dig that out. |
| @@ -73,7 +73,7 @@ class TestStorage(StorageCase): | |||
| # not offered. | |||
| self.content_row_action(3, "Create Partition") | |||
| self.dialog_wait_open() | |||
| self.assertEqual(b.attr("[value='dos-extended']", "class"), "disabled") | |||
| b.wait_present("li[value='dos-extended'].disabled") | |||
There was a problem hiding this comment.
This fix seems harmless, and regardless of the other failures, I have yet to see anything bad introduced by this change.
There was a problem hiding this comment.
However, it's not motivated by the code, or is it? When the dialog opens, the dropdown has been created and will not change while the dialog is open.
There might well be a race condition somewhere, but the race is over once the dialog is open, I think.
There was a problem hiding this comment.
Hmm, the original test was very strict: If the entry had any class in addition to "disabled", it would fail. Is that the case maybe after moving to full React?
There was a problem hiding this comment.
@mvollmer , right, the code does not seem changing it once initially set.
I observed this issue when moving to React, not sure if it was there before.
Adding "wait to render" seemed to fix it and this change looks harmless to me in this case.
There's mixture of jquery and React in the code, hard to say if it is related without deep investigation.
I can extend the test for uniqueness of the disabled if needed. What do you think?
Observed within work on #9263 .