Fix various tests when running on Fedora/Wayland/Gnome Shell (PR #26934)
Scott Talbert
This fixes a number of test issues when running the tests on Fedora under Wayland/Gnome Shell.
There are still about 10 test cases that fail, but I haven't been able to sort out those yet (they're intermittent).
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26934
Commit Summary
- 19be9b0 Check /lib64 before /lib in DynamicLibrary::Load test
- d3583ef Use REQUIRE instead of CHECK before indexing key event vectors in tests
- da8f5f8 Don't fail EventClone if wxHelpEvent ctor triggers a wx assert
- fc3812b Disable UI tests by default when running as a native Wayland client
- 95be15e Add missing EnableUITests() guard to tests using wxUIActionSimulator
- b739561 Don't check restored window position under native Wayland
- d5714ab Use client size, not outer size, when sizing test window in ClippingBoxTestCase::wxPaintDC
- 377d377 Don't rely on Update() being synchronous for GTK3 paint-dependent tests
- 5650310 Fix Window::Refresh test: skip child-not-painted check on Wayland
- 4587ccd Add dummy sibling window in TextCtrlTestCase to avoid GTK lone-child sizing
- 5f69d3e Apply lone-child-fills-parent workaround to GTK3 too in wxWindow::SetClientSize
File Changes
(18 files)
- M tests/controls/buttontest.cpp (6)
- M tests/controls/gridtest.cpp (17)
- M tests/controls/radiobuttontest.cpp (3)
- M tests/controls/spinctrldbltest.cpp (9)
- M tests/controls/spinctrltest.cpp (12)
- M tests/controls/styledtextctrltest.cpp (6)
- M tests/controls/textctrltest.cpp (7)
- M tests/controls/treectrltest.cpp (24)
- M tests/controls/windowtest.cpp (14)
- M tests/events/clone.cpp (25)
- M tests/events/keyboard.cpp (36)
- M tests/events/propagation.cpp (8)
- M tests/graphics/clippingbox.cpp (6)
- M tests/misc/dynamiclib.cpp (2)
- M tests/persistence/tlw.cpp (23)
- M tests/test.cpp (15)
- M tests/validators/valnum.cpp (3)
- M tests/window/clientsize.cpp (6)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26934.patch
- https://github.com/wxWidgets/wxWidgets/pull/26934.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
Thanks for the fixes! Most of them should definitely be applied, but I think a couple can be improved by applying them to all ports and not just wxGTK and I don't understand one of them (wxHelpEvent), could you please explain? TIA!
In tests/controls/gridtest.cpp:
> + // Update() is a no-op under GTK3/Wayland, so wait for the actual + // paint event instead of relying on it being synchronous.
I'd say that we should do this under all platforms, waiting for the paint event is higher level and hence better than just blindly yielding.
In tests/controls/textctrltest.cpp:
> @@ -197,6 +199,11 @@ void TextCtrlTestCase::CreateText(long extraStyles)
{
const long style = m_style | extraStyles;
const int h = (style & wxTE_MULTILINE) ? TEXT_HEIGHT : -1;
+#if defined(__WXGTK__)
This shouldn't be GTK-specific, this is how it works in all ports. So something else must explain the failures under wxGTK.
> + // wxHelpEvent::GuessOrigin(), called from its default ctor, + // can trigger a wx assert here because it can't determine
I think this is a real problem uncovered by the test and should be fixed instead of being suppressed.
But I don't see any asserts in wxGetKeyState() when using Wayland, where exactly is it?
In tests/events/propagation.cpp:
> @@ -187,8 +187,16 @@ class TestScrollWindow : public wxScrolledWindow
g_str.clear();
#endif // __WXGTK__ || __WXQT__
+#ifdef __WXGTK__
Again, I'd rather do this for all platforms instead of adding a special case for wxGTK.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Scott Talbert
@swt2c commented on this pull request.
> + // wxHelpEvent::GuessOrigin(), called from its default ctor, + // can trigger a wx assert here because it can't determine
Yes, admittedly, this is a real problem, but wasn't sure about how to fix it. We really need some sort of bool wxCanKeyKeyState(wxKeyCode) that returns true/false if it's possible to get the key state for a given key. In the case where wxHelpEvent calls wxGetKeyState() it could at least return Origin_Unknown which would be better that what it does currently.
This is where the assert is happening: https://github.com/wxWidgets/wxWidgets/blob/master/src/unix/utilsx11.cpp#L2633
You don't see that one?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> + // wxHelpEvent::GuessOrigin(), called from its default ctor, + // can trigger a wx assert here because it can't determine
Sorry, I misread the code :-(
I think we need to either just remove the assert (after all, we already just return false silently for GTK3 < 3.4.0 — although I don't think we support GTK < 3.6 since a very long time...) or add a new wxIsKeyPressed() returning a 3-state enum with values meaning "yes", "no" and "unknown".
The assert was originally added by you in 1033fb0 (Fix wxGetKeyState() on non-X11 wxGTK backends (e.g., Wayland), 2016-09-02), so I think it's up to you to decide what do you prefer. Unless @paulcor has some overriding considerations?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Scott Talbert
@swt2c commented on this pull request.
> + // wxHelpEvent::GuessOrigin(), called from its default ctor, + // can trigger a wx assert here because it can't determine
Ah yes, I can't believe that was nearly 10 years ago.
OK, since this one isn't really a test fix, I'll pull this commit from the PR and address separately.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Scott Talbert
@swt2c pushed 8 commits.
- 323a645 Disable UI tests by default when running as a native Wayland client
- 57d2358 Add missing EnableUITests() guard to tests using wxUIActionSimulator
- 905c3cc Don't check restored window position under native Wayland
- b1ebdc4 Use client size, not outer size, when sizing test window in ClippingBoxTestCase::wxPaintDC
- 2c9eb73 Don't rely on Update() being synchronous for GTK3 paint-dependent tests
- 7a1f3f1 Fix Window::Refresh test: skip child-not-painted check on Wayland
- cb79802 Add dummy sibling window in TextCtrlTestCase to avoid GTK lone-child sizing
- 47c85b7 Apply lone-child-fills-parent workaround to GTK3 too in wxWindow::SetClientSize
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Scott Talbert
@swt2c pushed 4 commits.
- 224a8d3 Don't rely on Update() being synchronous for GTK3 paint-dependent tests
- 54c2d9d Fix Window::Refresh test: skip child-not-painted check on Wayland
- 0e96f1d Add dummy sibling window in TextCtrlTestCase to avoid GTK lone-child sizing
- 6835467 Apply lone-child-fills-parent workaround to GTK3 too in wxWindow::SetClientSize
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Scott Talbert
@swt2c commented on this pull request.
In tests/controls/gridtest.cpp:
> + // Update() is a no-op under GTK3/Wayland, so wait for the actual + // paint event instead of relying on it being synchronous.
I started to do this, but then realized that wxWaitForPaint is only implemented for wxGTK, so I stopped, especially since I don't really have a way to test MSW and OSX outside of GitHub Actions, so I'd rather not do this.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Scott Talbert
@swt2c commented on this pull request.
In tests/controls/textctrltest.cpp:
> @@ -197,6 +199,11 @@ void TextCtrlTestCase::CreateText(long extraStyles)
{
const long style = m_style | extraStyles;
const int h = (style & wxTE_MULTILINE) ? TEXT_HEIGHT : -1;
+#if defined(__WXGTK__)
Done.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Scott Talbert
@swt2c commented on this pull request.
> + // wxHelpEvent::GuessOrigin(), called from its default ctor, + // can trigger a wx assert here because it can't determine
Removed.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Scott Talbert
@swt2c commented on this pull request.
In tests/events/propagation.cpp:
> @@ -187,8 +187,16 @@ class TestScrollWindow : public wxScrolledWindow
g_str.clear();
#endif // __WXGTK__ || __WXQT__
+#ifdef __WXGTK__
Ditto on wxWaitForPaint only being wxGTK.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
In tests/controls/gridtest.cpp:
> + // Update() is a no-op under GTK3/Wayland, so wait for the actual + // paint event instead of relying on it being synchronous.
Oops, sorry for completely forgetting about this :-(
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.