Window size stays the same after minimizing and unmizing the window #2837
Conversation
…on Windows platform.
|
Oops :). Maybe there is the same problem on Gtk, I will check that. |
testbed/tests/window/test_window.py
Outdated
| @@ -443,6 +443,20 @@ async def test_visibility(app, second_window, second_window_probe): | |||
|
|
|||
| assert not second_window_probe.is_minimized | |||
|
|
|||
| second_window.size = (450, 200) | |||
| second_window_probe.minimize() | |||
There was a problem hiding this comment.
Is there a need to do another min/unmin pass here? Can't we add the assertion to the existing min/unmin pass?
There was a problem hiding this comment.
There is no need to, I only did it to make it clear what the test is testing, but it's not worth it. I will merge it with the existing one.
There was a problem hiding this comment.
A good instinct, to be sure; however, the testbed tests are a little weird in this regard.
In most cases, I'd overwhelmingly agree that you should keep tests isolated; I'd even suggest that it should be a completely standalone test case (so, a standalone test_minimized_window_doesnt_resize, or similar) such that each test case validates a single known problem or property.
However, the testbed suite has a significant setup cost for each test, and the tests themselves can be quite slow. As a result, there's a benefit to doing as few actual operations as possible, and having as few tests as possible. We'll still introduce a new test case if that test has a unique setup, or there's a concern about state leaking between parts of a test - but the threshold for what we're willing to accept in the bounds of a single test case is a lot larger in the testbed.
There was a problem hiding this comment.
That's understandable, I will add it to the existing one. Any ideas why CI is failing on Linux?
There was a problem hiding this comment.
I look at a look and... heh... you're going to kick yourself :-)
You know how the unminimize test is in a block checking if the backend supports_unminimize?
Want to guess what platform requires us to have that check? :-)
The problem is that XWindows (and even more so, Wayland) has Strong Opinions (tm) about what an app is allowed to do with a window. They believe that minimize/maximize (and a bunch of other operations) are the domain of the window manager, not the app - and so minimise operations are, at best, unreliable; and in some cases, straight up not implemented. So - the API exists, but it's documented as "it might not work" on GTK - and we skip the test because we literally can't rely on it working in a live app test.
However, it might still work - and in the case of CI operation, minimising does work. The window is successfully being minimized. But then, it isn't unminimized, so the assertion about post-unminimised size fails. In this case - indent the assertion about window size, and the test will pass.
There was a problem hiding this comment.
Well that's one thing I didn't know :-). Thanks a lot for the detailed explanation.
Fixes #2729.
Followed the solution outlined on the issue discussion and added a new test to verify it works.
PR Checklist: