Details
Diff Detail
- Repository
- R4 Zanshin
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
This window needs to actually get focus for centralView->hasFocus() to work, so I think the real fix is
page.show(); page.activateWindow(); QVERIFY(QTest::qWaitForWindowActive(&page));
(see also https://bugreports.qt.io/browse/QTBUG-71137 which isn't an actual bug, but has more info on the above API)
You didn't replace qWaitForWindowExposed with qWaitForWindowActive.
But note that you should do that (activateWindow + qWaitForWindowActive) *only* if the test is about focus.
I don't see how the last hunk, shouldDisplayMessageOnError, needs focus, for instance. The first two hunks do need it. The one after that doesn't, etc.
The reason to not abuse activateWindow + qWaitForWindowActive is that it's annoying when trying to use one's computer during ctest :-)
Address dfaure's comment
Indeed I totally missed the "Exposed" vs "Active" and got carried a bit on the last test.
IMO, most tests need the window active though since they revolve around keyboard input and shortcuts.
| tests/units/widgets/pageviewtest.cpp | ||
|---|---|---|
| 564 | Well, my point is you don't need focus when explicitly sending a keyevent to a widget. | |
| tests/units/widgets/pageviewtest.cpp | ||
|---|---|---|
| 564 | Yes, but the centralView needs to have received the focus still (see above the setFocus() call). | |
Ah, OK. Weird though, widget code doesn't typically check hasFocus() so I'm not sure why this is needed. Oh well.