From 6e86b36811c3a246c00791cee752a4c71abfa7b0 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 21 Mar 2024 11:20:49 -0700 Subject: [PATCH 01/17] See error --- tests/playwright/shiny/session/flush/test_on_flush.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/playwright/shiny/session/flush/test_on_flush.py b/tests/playwright/shiny/session/flush/test_on_flush.py index b59feced9..32faedb21 100644 --- a/tests/playwright/shiny/session/flush/test_on_flush.py +++ b/tests/playwright/shiny/session/flush/test_on_flush.py @@ -25,8 +25,9 @@ def test_output_image_kitchen(page: Page, local_app: ShinyAppProc) -> None: local_app.close() # Wait up to 3 seconds for the app to close and print the logs. (Should be ~ instant) - local_app.stdout.wait_for(lambda x: "test4" in x, 3) + local_app.stdout.wait_for(lambda x: "test4" in x, 10) stdout = str(local_app.stdout) + print(stdout) out_indexes = [ stdout.index("session ended - sync - test1"), stdout.index("session ended - async - test2"), From 51c5850fa0c34e95da61a89ff5d98eea44502cfc Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 21 Mar 2024 11:42:48 -0700 Subject: [PATCH 02/17] Add more debug statements --- tests/playwright/shiny/session/flush/test_on_flush.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/playwright/shiny/session/flush/test_on_flush.py b/tests/playwright/shiny/session/flush/test_on_flush.py index 32faedb21..eadb2c76f 100644 --- a/tests/playwright/shiny/session/flush/test_on_flush.py +++ b/tests/playwright/shiny/session/flush/test_on_flush.py @@ -21,13 +21,16 @@ def test_output_image_kitchen(page: Page, local_app: ShinyAppProc) -> None: "'bx-3-second-flushed', 'by-3-second-flushed', 'c-3-flushed')" ) + print("closing app") # Verify `on_ended` callbacks are called in the correct order (and cancelled) local_app.close() + print("waiting for output") # Wait up to 3 seconds for the app to close and print the logs. (Should be ~ instant) local_app.stdout.wait_for(lambda x: "test4" in x, 10) stdout = str(local_app.stdout) - print(stdout) + print(f"stdout: `{stdout}`") + print(local_app.stdout) out_indexes = [ stdout.index("session ended - sync - test1"), stdout.index("session ended - async - test2"), From 8087cf726ec84d8616fee655738767edbe3010f8 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Thu, 21 Mar 2024 12:00:32 -0700 Subject: [PATCH 03/17] more debugging statements --- tests/playwright/conftest.py | 2 ++ tests/playwright/shiny/session/flush/test_on_flush.py | 1 + 2 files changed, 3 insertions(+) diff --git a/tests/playwright/conftest.py b/tests/playwright/conftest.py index f51d25dc6..237261898 100644 --- a/tests/playwright/conftest.py +++ b/tests/playwright/conftest.py @@ -136,6 +136,7 @@ def __init__(self, proc: subprocess.Popen[str], port: int): self.proc = proc self.port = port self.url = f"http://127.0.0.1:{port}/" + print(f"App url is {self.url}") self.stdout = OutputStream(proc.stdout or dummyio()) self.stderr = OutputStream(proc.stderr or dummyio()) threading.Thread(group=None, target=self._run, daemon=True).start() @@ -187,6 +188,7 @@ def run_shiny_app( timeout_secs: float = 10, bufsize: int = 64 * 1024, ) -> ShinyAppProc: + if port == 0: port = shiny._utils.random_port() diff --git a/tests/playwright/shiny/session/flush/test_on_flush.py b/tests/playwright/shiny/session/flush/test_on_flush.py index eadb2c76f..f5dc1409a 100644 --- a/tests/playwright/shiny/session/flush/test_on_flush.py +++ b/tests/playwright/shiny/session/flush/test_on_flush.py @@ -29,6 +29,7 @@ def test_output_image_kitchen(page: Page, local_app: ShinyAppProc) -> None: # Wait up to 3 seconds for the app to close and print the logs. (Should be ~ instant) local_app.stdout.wait_for(lambda x: "test4" in x, 10) stdout = str(local_app.stdout) + print(local_app.stderr) print(f"stdout: `{stdout}`") print(local_app.stdout) out_indexes = [ From b59d589fb306c983c7b586dda3409f6fad115d97 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Fri, 22 Mar 2024 13:07:28 -0700 Subject: [PATCH 04/17] add other OS --- .github/workflows/pytest.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml index b0ac38ed0..0763284b5 100644 --- a/.github/workflows/pytest.yaml +++ b/.github/workflows/pytest.yaml @@ -59,7 +59,7 @@ jobs: strategy: matrix: python-version: ["3.12", "3.11", "3.10", "3.9", "3.8"] - os: [ubuntu-latest] + os: [ubuntu-latest, windows-latest, macOS-latest] fail-fast: false steps: From 3d627e47163c9f83fae790a8e27aede213145989 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Fri, 22 Mar 2024 13:07:57 -0700 Subject: [PATCH 05/17] Add more debugging statements --- shiny/session/_session.py | 3 +++ tests/playwright/conftest.py | 1 + 2 files changed, 4 insertions(+) diff --git a/shiny/session/_session.py b/shiny/session/_session.py index 12bc3e578..d17211c3f 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -242,7 +242,9 @@ async def _run_session_end_tasks(self) -> None: self._has_run_session_end_tasks = True try: + print(f"Invoking {self._on_ended_callbacks.count()} on ended callbacks") await self._on_ended_callbacks.invoke() + print("After invoking on ended callbacks") finally: self.app._remove_session(self) @@ -773,6 +775,7 @@ def on_ended( : A function that can be used to cancel the registration. """ + print("adding on_ended callback") return self._on_ended_callbacks.register(wrap_async(fn)) # ========================================================================== diff --git a/tests/playwright/conftest.py b/tests/playwright/conftest.py index 237261898..da678070a 100644 --- a/tests/playwright/conftest.py +++ b/tests/playwright/conftest.py @@ -143,6 +143,7 @@ def __init__(self, proc: subprocess.Popen[str], port: int): def _run(self) -> None: self.proc.wait() + sleep(0.5) if self.proc.stdout is not None: self.proc.stdout.close() if self.proc.stderr is not None: From eaa3cc9df1427dd492dc119139facf40fd36ac56 Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Fri, 22 Mar 2024 13:27:41 -0700 Subject: [PATCH 06/17] run tests serially instead of parallel --- pytest.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index 7efe317fc..3b06f043e 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,4 +1,4 @@ [pytest] asyncio_mode=strict testpaths=tests/pytest/ -addopts = --strict-markers --durations=6 --durations-min=5.0 --browser webkit --browser firefox --browser chromium --numprocesses auto --tracing=retain-on-failure --video=retain-on-failure +addopts = --strict-markers --durations=6 --durations-min=5.0 --browser chromium --tracing=retain-on-failure --video=retain-on-failure From 35424e654d8b960ef7b08388968738b2a765140f Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Fri, 22 Mar 2024 13:41:30 -0700 Subject: [PATCH 07/17] restrict uvicorn version to 0.28.1 --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 617a42393..04e320e6d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -34,7 +34,7 @@ setup_requires = setuptools install_requires = typing-extensions>=4.0.1 - uvicorn>=0.16.0;platform_system!="Emscripten" + uvicorn==0.28.1;platform_system!="Emscripten" starlette websockets>=10.0 python-multipart From 859a39d43e6f1fb2fcc8ca0eef0c875f85e709c8 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 22 Mar 2024 17:02:17 -0400 Subject: [PATCH 08/17] Revert "run tests serially instead of parallel" This reverts commit eaa3cc9df1427dd492dc119139facf40fd36ac56. --- pytest.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index 3b06f043e..7efe317fc 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,4 +1,4 @@ [pytest] asyncio_mode=strict testpaths=tests/pytest/ -addopts = --strict-markers --durations=6 --durations-min=5.0 --browser chromium --tracing=retain-on-failure --video=retain-on-failure +addopts = --strict-markers --durations=6 --durations-min=5.0 --browser webkit --browser firefox --browser chromium --numprocesses auto --tracing=retain-on-failure --video=retain-on-failure From 26801429bb9f871da8f72c3b8d2ebdf917126dd8 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 22 Mar 2024 17:02:26 -0400 Subject: [PATCH 09/17] Revert "Add more debugging statements" This reverts commit 3d627e47163c9f83fae790a8e27aede213145989. --- shiny/session/_session.py | 3 --- tests/playwright/conftest.py | 1 - 2 files changed, 4 deletions(-) diff --git a/shiny/session/_session.py b/shiny/session/_session.py index d17211c3f..12bc3e578 100644 --- a/shiny/session/_session.py +++ b/shiny/session/_session.py @@ -242,9 +242,7 @@ async def _run_session_end_tasks(self) -> None: self._has_run_session_end_tasks = True try: - print(f"Invoking {self._on_ended_callbacks.count()} on ended callbacks") await self._on_ended_callbacks.invoke() - print("After invoking on ended callbacks") finally: self.app._remove_session(self) @@ -775,7 +773,6 @@ def on_ended( : A function that can be used to cancel the registration. """ - print("adding on_ended callback") return self._on_ended_callbacks.register(wrap_async(fn)) # ========================================================================== diff --git a/tests/playwright/conftest.py b/tests/playwright/conftest.py index da678070a..237261898 100644 --- a/tests/playwright/conftest.py +++ b/tests/playwright/conftest.py @@ -143,7 +143,6 @@ def __init__(self, proc: subprocess.Popen[str], port: int): def _run(self) -> None: self.proc.wait() - sleep(0.5) if self.proc.stdout is not None: self.proc.stdout.close() if self.proc.stderr is not None: From b4236b977c700ef0162943fb63a59d6830a984a0 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 22 Mar 2024 17:02:31 -0400 Subject: [PATCH 10/17] Revert "add other OS" This reverts commit b59d589fb306c983c7b586dda3409f6fad115d97. --- .github/workflows/pytest.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml index 0763284b5..b0ac38ed0 100644 --- a/.github/workflows/pytest.yaml +++ b/.github/workflows/pytest.yaml @@ -59,7 +59,7 @@ jobs: strategy: matrix: python-version: ["3.12", "3.11", "3.10", "3.9", "3.8"] - os: [ubuntu-latest, windows-latest, macOS-latest] + os: [ubuntu-latest] fail-fast: false steps: From 3e571e814da72bda217597fed66ce1a45b2a3362 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 22 Mar 2024 17:02:37 -0400 Subject: [PATCH 11/17] Revert "more debugging statements" This reverts commit 8087cf726ec84d8616fee655738767edbe3010f8. --- tests/playwright/conftest.py | 2 -- tests/playwright/shiny/session/flush/test_on_flush.py | 1 - 2 files changed, 3 deletions(-) diff --git a/tests/playwright/conftest.py b/tests/playwright/conftest.py index 237261898..f51d25dc6 100644 --- a/tests/playwright/conftest.py +++ b/tests/playwright/conftest.py @@ -136,7 +136,6 @@ def __init__(self, proc: subprocess.Popen[str], port: int): self.proc = proc self.port = port self.url = f"http://127.0.0.1:{port}/" - print(f"App url is {self.url}") self.stdout = OutputStream(proc.stdout or dummyio()) self.stderr = OutputStream(proc.stderr or dummyio()) threading.Thread(group=None, target=self._run, daemon=True).start() @@ -188,7 +187,6 @@ def run_shiny_app( timeout_secs: float = 10, bufsize: int = 64 * 1024, ) -> ShinyAppProc: - if port == 0: port = shiny._utils.random_port() diff --git a/tests/playwright/shiny/session/flush/test_on_flush.py b/tests/playwright/shiny/session/flush/test_on_flush.py index f5dc1409a..eadb2c76f 100644 --- a/tests/playwright/shiny/session/flush/test_on_flush.py +++ b/tests/playwright/shiny/session/flush/test_on_flush.py @@ -29,7 +29,6 @@ def test_output_image_kitchen(page: Page, local_app: ShinyAppProc) -> None: # Wait up to 3 seconds for the app to close and print the logs. (Should be ~ instant) local_app.stdout.wait_for(lambda x: "test4" in x, 10) stdout = str(local_app.stdout) - print(local_app.stderr) print(f"stdout: `{stdout}`") print(local_app.stdout) out_indexes = [ From edd441c84eb3d6b2fcc471ce965621cb9d8c40d4 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 22 Mar 2024 17:02:45 -0400 Subject: [PATCH 12/17] Revert "Add more debug statements" This reverts commit 51c5850fa0c34e95da61a89ff5d98eea44502cfc. --- tests/playwright/shiny/session/flush/test_on_flush.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/playwright/shiny/session/flush/test_on_flush.py b/tests/playwright/shiny/session/flush/test_on_flush.py index eadb2c76f..32faedb21 100644 --- a/tests/playwright/shiny/session/flush/test_on_flush.py +++ b/tests/playwright/shiny/session/flush/test_on_flush.py @@ -21,16 +21,13 @@ def test_output_image_kitchen(page: Page, local_app: ShinyAppProc) -> None: "'bx-3-second-flushed', 'by-3-second-flushed', 'c-3-flushed')" ) - print("closing app") # Verify `on_ended` callbacks are called in the correct order (and cancelled) local_app.close() - print("waiting for output") # Wait up to 3 seconds for the app to close and print the logs. (Should be ~ instant) local_app.stdout.wait_for(lambda x: "test4" in x, 10) stdout = str(local_app.stdout) - print(f"stdout: `{stdout}`") - print(local_app.stdout) + print(stdout) out_indexes = [ stdout.index("session ended - sync - test1"), stdout.index("session ended - async - test2"), From 68f3541fde0537e0e8170df79d7c2337885fa72b Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 22 Mar 2024 17:02:54 -0400 Subject: [PATCH 13/17] Revert "See error" This reverts commit 6e86b36811c3a246c00791cee752a4c71abfa7b0. --- tests/playwright/shiny/session/flush/test_on_flush.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/playwright/shiny/session/flush/test_on_flush.py b/tests/playwright/shiny/session/flush/test_on_flush.py index 32faedb21..b59feced9 100644 --- a/tests/playwright/shiny/session/flush/test_on_flush.py +++ b/tests/playwright/shiny/session/flush/test_on_flush.py @@ -25,9 +25,8 @@ def test_output_image_kitchen(page: Page, local_app: ShinyAppProc) -> None: local_app.close() # Wait up to 3 seconds for the app to close and print the logs. (Should be ~ instant) - local_app.stdout.wait_for(lambda x: "test4" in x, 10) + local_app.stdout.wait_for(lambda x: "test4" in x, 3) stdout = str(local_app.stdout) - print(stdout) out_indexes = [ stdout.index("session ended - sync - test1"), stdout.index("session ended - async - test2"), From c01c6abd794fb3efec0e3f7efaced933b8c67054 Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Fri, 22 Mar 2024 17:03:41 -0400 Subject: [PATCH 14/17] Remove wait --- tests/playwright/shiny/session/flush/test_on_flush.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/playwright/shiny/session/flush/test_on_flush.py b/tests/playwright/shiny/session/flush/test_on_flush.py index b59feced9..877fc40a7 100644 --- a/tests/playwright/shiny/session/flush/test_on_flush.py +++ b/tests/playwright/shiny/session/flush/test_on_flush.py @@ -24,8 +24,6 @@ def test_output_image_kitchen(page: Page, local_app: ShinyAppProc) -> None: # Verify `on_ended` callbacks are called in the correct order (and cancelled) local_app.close() - # Wait up to 3 seconds for the app to close and print the logs. (Should be ~ instant) - local_app.stdout.wait_for(lambda x: "test4" in x, 3) stdout = str(local_app.stdout) out_indexes = [ stdout.index("session ended - sync - test1"), From 719daf550e2055c94b9480adaac705eb2b2d47db Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Tue, 26 Mar 2024 12:19:12 -0700 Subject: [PATCH 15/17] verify session ended messages on txtoutput instead of stdout --- tests/playwright/shiny/session/flush/app.py | 15 ++++++++--- .../shiny/session/flush/test_on_flush.py | 25 ++++++------------- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/tests/playwright/shiny/session/flush/app.py b/tests/playwright/shiny/session/flush/app.py index a6591e31d..39c046887 100644 --- a/tests/playwright/shiny/session/flush/app.py +++ b/tests/playwright/shiny/session/flush/app.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +from typing import List from shiny import App, Inputs, Outputs, Session, reactive, render, ui @@ -37,8 +38,10 @@ ui.output_text_verbatim("all_txt", placeholder=True), ui.tags.span("Flush events: "), ui.output_text_verbatim("flush_txt", placeholder=True), - ui.tags.span("Flushed: "), + ui.tags.span("Flushed events: "), ui.output_text_verbatim("flushed_txt", placeholder=True), + ui.tags.span("Session end events (refresh App to add events): "), + ui.output_text_verbatim("session_end_txt", placeholder=True), ui.tags.script( """ $(document).on('shiny:connected', function(event) { @@ -55,18 +58,20 @@ ), ) +session_ended_messages: List[str] = [] + def server(input: Inputs, output: Outputs, session: Session): def on_ended_sync(txt: str): def _(): - print(txt) + session_ended_messages.append(txt) return _ def on_ended_async(txt: str): async def _(): await asyncio.sleep(0) - print(txt) + session_ended_messages.append(txt) return _ @@ -181,5 +186,9 @@ def flush_txt(): def flushed_txt(): return str(flushed_vals.get()) + @render.text + def session_end_txt(): + return str(session_ended_messages) + app = App(app_ui, server) diff --git a/tests/playwright/shiny/session/flush/test_on_flush.py b/tests/playwright/shiny/session/flush/test_on_flush.py index 877fc40a7..53209cbd5 100644 --- a/tests/playwright/shiny/session/flush/test_on_flush.py +++ b/tests/playwright/shiny/session/flush/test_on_flush.py @@ -20,21 +20,10 @@ def test_output_image_kitchen(page: Page, local_app: ShinyAppProc) -> None: "('a-3-flushed', 'bx-3-first-flushed', 'by-3-first-flushed', " "'bx-3-second-flushed', 'by-3-second-flushed', 'c-3-flushed')" ) - - # Verify `on_ended` callbacks are called in the correct order (and cancelled) - local_app.close() - - stdout = str(local_app.stdout) - out_indexes = [ - stdout.index("session ended - sync - test1"), - stdout.index("session ended - async - test2"), - stdout.index("session ended - async - test3"), - stdout.index("session ended - sync - test4"), - ] - for i in range(len(out_indexes)): - index = out_indexes[i] - assert index >= 0 - # Make sure they are ordered correctly - if i > 0: - prev_index = out_indexes[i - 1] - assert index > prev_index + # Session end messages have not flushed yet + OutputTextVerbatim(page, "session_end_txt").expect_value("[]") + page.reload() + # Session end messages have flushed + OutputTextVerbatim(page, "session_end_txt").expect_value( + "['session ended - sync - test1', 'session ended - async - test2', 'session ended - async - test3', 'session ended - sync - test4']" + ) From 03e468fbd67f0b340ba32a4a1202c2d6117500cd Mon Sep 17 00:00:00 2001 From: Karan Gathani Date: Tue, 26 Mar 2024 12:21:28 -0700 Subject: [PATCH 16/17] unskip test from #1241 --- .../shiny/session/flush/test_on_flush.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/tests/playwright/shiny/session/flush/test_on_flush.py b/tests/playwright/shiny/session/flush/test_on_flush.py index bccc3ea05..4d14a3321 100644 --- a/tests/playwright/shiny/session/flush/test_on_flush.py +++ b/tests/playwright/shiny/session/flush/test_on_flush.py @@ -1,26 +1,10 @@ -import os - -import pytest from conftest import ShinyAppProc from controls import OutputTextVerbatim from playwright.sync_api import Page -on_ci = os.environ.get("CI", "False") == "true" - def test_output_image_kitchen(page: Page, local_app: ShinyAppProc) -> None: - if on_ci: - # 2024-03-22: The tests pass locally, but do not pass on CI. It started with - # https://github.com/posit-dev/py-shiny/commit/2f75a9076c567690f2a6a647ec07cfa9e558ff9c - # , but the changes in that commit were unrelated. We had believe it was an - # update in uvicorn, but removing all other debug statements did not fix the - # issue. - # 2024-03-22 Barret: When inspecting the issue, we found that many log - # INFO entries have different port values. This is concerning. - # https://github.com/posit-dev/py-shiny/pull/1236 - pytest.skip("Error with stdout / stderr on CI. Related #1236") - page.goto(local_app.url) OutputTextVerbatim(page, "all_txt").expect_value( From 333182e649bd57caa93954a25b7768249ecb54ae Mon Sep 17 00:00:00 2001 From: Barret Schloerke Date: Tue, 26 Mar 2024 15:27:45 -0400 Subject: [PATCH 17/17] Discard changes to setup.cfg --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 56dcc02e9..dd80a373c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -34,7 +34,7 @@ setup_requires = setuptools install_requires = typing-extensions>=4.0.1 - uvicorn==0.28.1;platform_system!="Emscripten" + uvicorn>=0.16.0;platform_system!="Emscripten" starlette websockets>=10.0 python-multipart