-
-
Notifications
You must be signed in to change notification settings - Fork 689
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
Add polling based window waiting for testbed #3047
Changes from 2 commits
081ac8f
dfbe752
03d2bfc
b0de666
8bb7443
bf48d6a
4f0bb3d
f403967
e528998
3a1a9ed
c44667c
a936bd2
1b12681
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
The `wait_for_window()` window probe method, now has a polling based waiting mechanism. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
import asyncio | ||
|
||
from rubicon.objc import objc_id, send_message | ||
|
||
from toga_cocoa.libs import NSWindow, NSWindowStyleMask | ||
|
@@ -27,16 +29,26 @@ async def wait_for_window( | |
message, | ||
minimize=False, | ||
full_screen=False, | ||
state_switch_not_from_normal=False, | ||
expected_state=None, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we still using the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like, I had missed cleanup of unused parameters. I have removed them now. |
||
): | ||
await self.redraw( | ||
message, | ||
delay=( | ||
1.75 | ||
if state_switch_not_from_normal | ||
else 0.75 if full_screen else 0.5 if minimize else 0.1 | ||
), | ||
message, delay=(1 if full_screen else 0.5 if minimize else 0.1) | ||
) | ||
if expected_state: | ||
timeout = 5 | ||
polling_interval = 0.1 | ||
exception = None | ||
loop = asyncio.get_running_loop() | ||
start_time = loop.time() | ||
while (loop.time() - start_time) < timeout: | ||
try: | ||
assert self.instantaneous_state == expected_state | ||
return | ||
except AssertionError as e: | ||
exception = e | ||
await asyncio.sleep(polling_interval) | ||
continue | ||
raise exception | ||
|
||
def close(self): | ||
self.native.performClose(None) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -167,8 +167,10 @@ async def test_menu_minimize(app, app_probe): | |
await app_probe.redraw("Extra window added") | ||
|
||
app_probe.activate_menu_minimize() | ||
|
||
await window1_probe.wait_for_window("Extra window minimized", minimize=True) | ||
# Wait for window animation before assertion. | ||
await window1_probe.wait_for_window( | ||
"Extra window minimized", expected_state=WindowState.MINIMIZED | ||
) | ||
assert window1_probe.is_minimized | ||
|
||
|
||
|
@@ -201,19 +203,19 @@ async def test_presentation_mode(app, app_probe, main_window, main_window_probe) | |
screen_window_dict[window_information["paired_screen"]] = window_information[ | ||
"window" | ||
] | ||
|
||
# Add delay to ensure windows are visible after animation. | ||
# Wait for window animation before assertion. | ||
await main_window_probe.wait_for_window("All Test Windows are visible") | ||
|
||
# Enter presentation mode with a screen-window dict via the app | ||
app.enter_presentation_mode(screen_window_dict) | ||
# Add delay to ensure windows are visible after animation. | ||
await main_window_probe.wait_for_window( | ||
"App is in presentation mode", full_screen=True | ||
) | ||
assert app.in_presentation_mode | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the test changing? The test shouldn't be changing at all if all we're doing is changing how the test is evaluated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have only changed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, but as a result of this change, you're not waiting for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously, wait_for_window() was pausing the test for a fixed delay, and so it didn't matter on which window probe wait_for_window() was called. Hence, I had used main_window probe for wait_for_window(). But, now since wait_for_window() checks each window individually to confirm its state transition, so waiting for main_window isn't required anymore. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but main window isn't in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have reverted the call to |
||
# All the windows should be in presentation mode. | ||
for window_information in window_information_list: | ||
# Wait for window animation before assertion. | ||
await window_information["window_probe"].wait_for_window( | ||
"App is in presentation mode", expected_state=WindowState.PRESENTATION | ||
) | ||
assert app.in_presentation_mode | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assertion only needs to be executed once, not once per window. It needs to be after we've waited for all windows, so putting it after the for loop would make sense to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have put the assertion after the for loop. |
||
assert ( | ||
window_information["window_probe"].instantaneous_state | ||
== WindowState.PRESENTATION | ||
|
@@ -238,12 +240,14 @@ async def test_presentation_mode(app, app_probe, main_window, main_window_probe) | |
|
||
# Exit presentation mode | ||
app.exit_presentation_mode() | ||
await main_window_probe.wait_for_window( | ||
"App is not in presentation mode", full_screen=True | ||
) | ||
|
||
assert not app.in_presentation_mode | ||
# All the windows should have exited presentation mode. | ||
for window_information in window_information_list: | ||
# Wait for window animation before assertion. | ||
await window_information["window_probe"].wait_for_window( | ||
"App is not in presentation mode", expected_state=WindowState.NORMAL | ||
) | ||
assert not app.in_presentation_mode | ||
assert ( | ||
window_information["window_probe"].instantaneous_state == WindowState.NORMAL | ||
), f"{window_information['window'].title}:" | ||
|
@@ -273,7 +277,7 @@ async def test_window_presentation_exit_on_another_window_presentation( | |
window2.content = toga.Box(style=Pack(background_color=CORNFLOWERBLUE)) | ||
window1.show() | ||
window2.show() | ||
# Add delay to ensure windows are visible after animation. | ||
# Wait for window animation before assertion. | ||
await main_window_probe.wait_for_window("Test windows are shown") | ||
|
||
assert not app.in_presentation_mode | ||
|
@@ -282,47 +286,49 @@ async def test_window_presentation_exit_on_another_window_presentation( | |
|
||
# Enter presentation mode with window2 | ||
app.enter_presentation_mode([window2]) | ||
# Add delay to ensure windows are visible after animation. | ||
await main_window_probe.wait_for_window( | ||
"App is in presentation mode", full_screen=True | ||
# Wait for window animation before assertion. | ||
await window2_probe.wait_for_window( | ||
"App is in presentation mode", expected_state=WindowState.PRESENTATION | ||
) | ||
assert app.in_presentation_mode | ||
assert window2_probe.instantaneous_state == WindowState.PRESENTATION | ||
assert window1_probe.instantaneous_state != WindowState.PRESENTATION | ||
|
||
# Enter presentation mode with window1, window2 no longer in presentation | ||
app.enter_presentation_mode([window1]) | ||
# Add delay to ensure windows are visible after animation. | ||
await main_window_probe.wait_for_window( | ||
"App is in presentation mode", full_screen=True | ||
# Wait for window animation before assertion. | ||
await window1_probe.wait_for_window( | ||
"App is in presentation mode", expected_state=WindowState.PRESENTATION | ||
) | ||
assert app.in_presentation_mode | ||
assert window1_probe.instantaneous_state == WindowState.PRESENTATION | ||
assert window2_probe.instantaneous_state != WindowState.PRESENTATION | ||
|
||
# Exit presentation mode | ||
app.exit_presentation_mode() | ||
await main_window_probe.wait_for_window( | ||
"App is not in presentation mode", full_screen=True | ||
# Wait for window animation before assertion. | ||
await window1_probe.wait_for_window( | ||
"App is not in presentation mode", expected_state=WindowState.NORMAL | ||
) | ||
assert not app.in_presentation_mode | ||
assert window1_probe.instantaneous_state != WindowState.PRESENTATION | ||
assert window2_probe.instantaneous_state != WindowState.PRESENTATION | ||
|
||
# Enter presentation mode again with window1 | ||
app.enter_presentation_mode([window1]) | ||
# Add delay to ensure windows are visible after animation. | ||
await main_window_probe.wait_for_window( | ||
"App is in presentation mode", full_screen=True | ||
# Wait for window animation before assertion. | ||
await window1_probe.wait_for_window( | ||
"App is in presentation mode", expected_state=WindowState.PRESENTATION | ||
) | ||
assert app.in_presentation_mode | ||
assert window1_probe.instantaneous_state == WindowState.PRESENTATION | ||
assert window2_probe.instantaneous_state != WindowState.PRESENTATION | ||
|
||
# Exit presentation mode | ||
app.exit_presentation_mode() | ||
await main_window_probe.wait_for_window( | ||
"App is not in presentation mode", full_screen=True | ||
# Wait for window animation before assertion. | ||
await window1_probe.wait_for_window( | ||
"App is not in presentation mode", expected_state=WindowState.NORMAL | ||
) | ||
assert not app.in_presentation_mode | ||
assert window1_probe.instantaneous_state != WindowState.PRESENTATION | ||
|
@@ -354,73 +360,74 @@ async def test_presentation_mode_exit_on_window_state_change( | |
window2.content = toga.Box(style=Pack(background_color=CORNFLOWERBLUE)) | ||
window1.show() | ||
window2.show() | ||
# Add delay to ensure windows are visible after animation. | ||
# Wait for window animation before assertion. | ||
await main_window_probe.wait_for_window("Test windows are shown") | ||
|
||
# Enter presentation mode | ||
app.enter_presentation_mode([window1]) | ||
# Add delay to ensure windows are visible after animation. | ||
await main_window_probe.wait_for_window( | ||
"App is in presentation mode", full_screen=True | ||
# Wait for window animation before assertion. | ||
await window1_probe.wait_for_window( | ||
"App is in presentation mode", expected_state=WindowState.PRESENTATION | ||
) | ||
|
||
assert app.in_presentation_mode | ||
assert window1_probe.instantaneous_state == WindowState.PRESENTATION | ||
|
||
# Changing window state of main window should make the app exit presentation mode. | ||
window1.state = new_window_state | ||
# Add delay to ensure windows are visible after animation. | ||
await main_window_probe.wait_for_window( | ||
# Wait for window animation before assertion. | ||
await window1_probe.wait_for_window( | ||
"App is not in presentation mode" f"\nTest Window 1 is in {new_window_state}", | ||
minimize=True if new_window_state == WindowState.MINIMIZED else False, | ||
full_screen=True if new_window_state == WindowState.FULLSCREEN else False, | ||
expected_state=new_window_state, | ||
) | ||
|
||
assert not app.in_presentation_mode | ||
assert window1_probe.instantaneous_state == new_window_state | ||
|
||
# Reset window states | ||
window1.state = WindowState.NORMAL | ||
# Wait for window animation before assertion. | ||
await window1_probe.wait_for_window( | ||
"All test windows are in WindowState.NORMAL", expected_state=WindowState.NORMAL | ||
) | ||
window2.state = WindowState.NORMAL | ||
# Add delay to ensure windows are visible after animation. | ||
await main_window_probe.wait_for_window( | ||
"All test windows are in WindowState.NORMAL", | ||
minimize=True if new_window_state == WindowState.MINIMIZED else False, | ||
full_screen=True if new_window_state == WindowState.FULLSCREEN else False, | ||
# Wait for window animation before assertion. | ||
await window2_probe.wait_for_window( | ||
"All test windows are in WindowState.NORMAL", expected_state=WindowState.NORMAL | ||
) | ||
assert window1_probe.instantaneous_state == WindowState.NORMAL | ||
assert window2_probe.instantaneous_state == WindowState.NORMAL | ||
|
||
# Enter presentation mode again | ||
app.enter_presentation_mode([window1]) | ||
# Add delay to ensure windows are visible after animation. | ||
await main_window_probe.wait_for_window( | ||
"App is in presentation mode", | ||
minimize=True if new_window_state == WindowState.MINIMIZED else False, | ||
full_screen=True if new_window_state == WindowState.FULLSCREEN else False, | ||
# Wait for window animation before assertion. | ||
await window1_probe.wait_for_window( | ||
"App is in presentation mode", expected_state=WindowState.PRESENTATION | ||
) | ||
assert app.in_presentation_mode | ||
assert window1_probe.instantaneous_state == WindowState.PRESENTATION | ||
|
||
# Changing window state of extra window should make the app exit presentation mode. | ||
window2.state = new_window_state | ||
# Add delay to ensure windows are visible after animation. | ||
await main_window_probe.wait_for_window( | ||
# Wait for window animation before assertion. | ||
await window2_probe.wait_for_window( | ||
"App is not in presentation mode" f"\nTest Window 2 is in {new_window_state}", | ||
minimize=True if new_window_state == WindowState.MINIMIZED else False, | ||
full_screen=True if new_window_state == WindowState.FULLSCREEN else False, | ||
expected_state=new_window_state, | ||
) | ||
|
||
assert not app.in_presentation_mode | ||
assert window2_probe.instantaneous_state == new_window_state | ||
|
||
# Reset window states | ||
window1.state = WindowState.NORMAL | ||
# Wait for window animation before assertion. | ||
await window1_probe.wait_for_window( | ||
"All test windows are in WindowState.NORMAL", expected_state=WindowState.NORMAL | ||
) | ||
window2.state = WindowState.NORMAL | ||
# Add delay to ensure windows are visible after animation. | ||
await main_window_probe.wait_for_window( | ||
"All test windows are in WindowState.NORMAL", | ||
minimize=True if new_window_state == WindowState.MINIMIZED else False, | ||
full_screen=True if new_window_state == WindowState.FULLSCREEN else False, | ||
# Wait for window animation before assertion. | ||
await window2_probe.wait_for_window( | ||
"All test windows are in WindowState.NORMAL", expected_state=WindowState.NORMAL | ||
) | ||
assert window1_probe.instantaneous_state == WindowState.NORMAL | ||
assert window2_probe.instantaneous_state == WindowState.NORMAL | ||
|
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.
What's the difference between
wait_for_window(full_screen=True)
andwait_for_window(expected_state=FULLSCREEN)
? Looking at the tests, you've updated most (if not all) of the existing calls to use the new parameter... so why keep the old one? If it's purely because of the "cleanup" mechanism - that's something that can be replaced with a literalself.redraw()
- essentially reproducing the old implementation ofwait_for_window
for the one place we need a "fixed" interval. Or, add await_for_window_close()
probe method, if that capability is needed in more than one place.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.
Yes, the old parameters are only used for
window_cleanup
. Adding await_for_window_close()
probe method would allow us to remove the old parameters entirely.