Repository navigation
Conversation
A Playwright test records when the browser receives each output's "recalculating" and "recalculated" messages, and fails when they arrive together (#1381). A one-off workflow runs it on windows-latest against main, main without the #1388 yields (the negative control), this branch, and this branch without its remaining yield.
On Windows, a websocket write made while an earlier write is still in flight goes out only when the event loop runs again. A synchronous renderer blocks the loop, so the output's "recalculating" message reached the client together with its value and the busy indicator never showed (#1381). A yield after the message lets it go out first.
jat255
added this pull request to stack #2516
October 5, 2026 13:44
The other Playwright jobs run on Ubuntu, where the #1381 busy-indicator test passes with or without its fix. A Windows job runs it where it can fail, and the PR gate requires it. This replaces the one-off check workflow.
An output now yields before it renders, so the first render of span_summary can run after the session_start span has closed. The test still checks that the span closes.
On Windows, `playwright install --with-deps` only adds the Media Foundation feature, which takes about 3 minutes and which headless Chromium doesn't need. A new PLAYWRIGHT_INSTALL_ARGS Makefile variable (default `--with-deps`) lets the job install only Chromium's headless shell.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On Windows, most outputs never show their busy indicator while a synchronous render function runs. This PR sends each output's "recalculating" message before the output renders, and adds a test that measures this in the browser.
Refs #1381 (closed in 2024 by #1388, but it still occurs on
main). Refs #2508, for its #1381 regression check. Stacked on #2524.FYI, this adds a new playwright test on Windows to catch/cover the failing behavior and fix. I put it in a new job since it's conceptually different than the other jobs that already exist, but I can rope it into the other playwright suite, if desired.
Summary
Before an output renders, Shiny sends a "recalculating" message, which turns on the output's busy indicator in the browser. On Windows, a websocket write made while an earlier write is still in flight goes out only when the event loop runs again. A synchronous render function blocks the loop, so the message reached the browser together with the output's new value, and the indicator never showed.
shiny/session/_session.py).test_busy_indicator_progress.py).playwright-windowsCI job runs the test onwindows-latest, and the "PR checks" gate requires it (pytest.yaml).Review notes
session_startspan. Because of the yield, it now runs just after that span ends. Its span is still a child of the session'sreactive_updatecycle span, and that span starts insidesession_start. An async render function already behaved this way. Theexpress-session-startOTel test asserted the old timing, so it now only waits for the first render and still checks thatsession_startcloses.shiny/reactive/_reactives.py) made no measurable difference on Windows. I did not remove it, because that is outside the scope of this PR.Testing
A temporary workflow (not in this PR) ran the test ten times for each variant on
windows-latest(run):mainAn earlier run had the same result for
mainwith the #1388 yields removed. On macOS, the test passes with or without the fix. The full pytest suite passes, and black, flake8, and pyright are clean on the changed files.