Skip to content

gh-124621: Emscripten: Add smoke test for using pyrepl in Chrome #137004

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

Merged

Conversation

hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Jul 22, 2025

Uses playwright. This should make it less likely that we break Python startup in the browser.

This should make it less likely that we break Python startup in the
browser.
Copy link
Contributor

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to having a smoke test; this looks like it will do the job.

Two comments:

  1. Would it be worth adding this to CI somehow? It's great to have a test, but if that test isn't run automatically, it's not going to catch anything.

  2. Regardless of whether we add this to CI, we should add "how to run the smoke test" details to the Emscripten section of the README.

@bedevere-app
Copy link

bedevere-app bot commented Jul 22, 2025

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

@freakboy3742
Copy link
Contributor

  1. Would it be worth adding this to CI somehow?

... and then I see python/buildmaster-config#616 :-)

We still need to add documentation for how to use this smoke test.

Copy link
Contributor

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a data point: the run_test.sh script didn't work for me initially. I added a shebang and chmod +x, then in the browser_test directory, ran:

rkm@eunectes browser_test % ./run_test.sh                        

added 54 packages, and audited 55 packages in 774ms

15 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

╔═════════════════════════════════════════════════════════════════╗
║ ATTENTION: "chrome" is already installed on the system!         ║
║                                                                 ║
║ "chrome" installation is not hermetic; installing newer version ║
║ requires *removal* of a current installation first.             ║
║                                                                 ║
║ To *uninstall* current version and re-install latest "chrome":  ║
║                                                                 ║
║ - Close all running instances of "chrome", if any               ║
║ - Use "--force" to install browser:                             ║
║                                                                 ║
║     npx playwright install --force chrome                       ║
║                                                                 ║
║ <3 Playwright Team                                              ║
╚═════════════════════════════════════════════════════════════════╝

[WebServer] (node:89215) [DEP0066] DeprecationWarning: OutgoingMessage.prototype._headers is deprecated
[WebServer] (Use `node --trace-deprecation ...` to show where the warning was created)

Running 1 test using 1 worker
××F

  1) [chromium] › index.spec.ts:3:5 › has title ────────────────────────────────────────────────────

    Error: browserType.launch: Executable doesn't exist at /Users/rkm/Library/Caches/ms-playwright/chromium_headless_shell-1181/chrome-mac/headless_shell
    ╔═════════════════════════════════════════════════════════════════════════╗
    ║ Looks like Playwright Test or Playwright was just installed or updated. ║
    ║ Please run the following command to download new browsers:              ║
    ║                                                                         ║
    ║     npx playwright install                                              ║
    ║                                                                         ║
    ║ <3 Playwright Team                                                      ║
    ╚═════════════════════════════════════════════════════════════════════════╝

    Retry #1 ───────────────────────────────────────────────────────────────────────────────────────

    Error: browserType.launch: Executable doesn't exist at /Users/rkm/Library/Caches/ms-playwright/chromium_headless_shell-1181/chrome-mac/headless_shell
    ╔═════════════════════════════════════════════════════════════════════════╗
    ║ Looks like Playwright Test or Playwright was just installed or updated. ║
    ║ Please run the following command to download new browsers:              ║
    ║                                                                         ║
    ║     npx playwright install                                              ║
    ║                                                                         ║
    ║ <3 Playwright Team                                                      ║
    ╚═════════════════════════════════════════════════════════════════════════╝

    attachment #1: trace (application/zip) ─────────────────────────────────────────────────────────
    test-results/index-has-title-chromium-retry1/trace.zip
    Usage:

        npx playwright show-trace test-results/index-has-title-chromium-retry1/trace.zip

    ────────────────────────────────────────────────────────────────────────────────────────────────

    Retry #2 ───────────────────────────────────────────────────────────────────────────────────────

    Error: browserType.launch: Executable doesn't exist at /Users/rkm/Library/Caches/ms-playwright/chromium_headless_shell-1181/chrome-mac/headless_shell
    ╔═════════════════════════════════════════════════════════════════════════╗
    ║ Looks like Playwright Test or Playwright was just installed or updated. ║
    ║ Please run the following command to download new browsers:              ║
    ║                                                                         ║
    ║     npx playwright install                                              ║
    ║                                                                         ║
    ║ <3 Playwright Team                                                      ║
    ╚═════════════════════════════════════════════════════════════════════════╝

  1 failed
    [chromium] › index.spec.ts:3:5 › has title ─────────────────────────────────────────────────────

I manually ran npx playwright install chrome --force, which definitely downloaded something; but didn't change the error.

However, if I manually ran npx playwright install:

rkm@eunectes browser_test % npx playwright install
Downloading Chromium 139.0.7258.5 (playwright build v1181) from https://cdn.playwright.dev/dbazure/download/playwright/builds/chromium/1181/chromium-mac-arm64.zip
128.4 MiB [====================] 100% 0.0s
Chromium 139.0.7258.5 (playwright build v1181) downloaded to /Users/rkm/Library/Caches/ms-playwright/chromium-1181
Downloading Chromium Headless Shell 139.0.7258.5 (playwright build v1181) from https://cdn.playwright.dev/dbazure/download/playwright/builds/chromium/1181/chromium-headless-shell-mac-arm64.zip
81.7 MiB [====================] 100% 0.0s
Chromium Headless Shell 139.0.7258.5 (playwright build v1181) downloaded to /Users/rkm/Library/Caches/ms-playwright/chromium_headless_shell-1181
Downloading Firefox 140.0.2 (playwright build v1489) from https://cdn.playwright.dev/dbazure/download/playwright/builds/firefox/1489/firefox-mac-arm64.zip
86.4 MiB [====================] 100% 0.0s
Firefox 140.0.2 (playwright build v1489) downloaded to /Users/rkm/Library/Caches/ms-playwright/firefox-1489
Downloading Webkit 26.0 (playwright build v2191) from https://cdn.playwright.dev/dbazure/download/playwright/builds/webkit/2191/webkit-mac-15-arm64.zip
70 MiB [====================] 100% 0.0s
Webkit 26.0 (playwright build v2191) downloaded to /Users/rkm/Library/Caches/ms-playwright/webkit-2191

the next execution passed (although it warned about the pre-existing chrome):

rkm@eunectes browser_test % ./run_test.sh         

added 54 packages, and audited 55 packages in 877ms

15 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

╔═════════════════════════════════════════════════════════════════╗
║ ATTENTION: "chrome" is already installed on the system!         ║
║                                                                 ║
║ "chrome" installation is not hermetic; installing newer version ║
║ requires *removal* of a current installation first.             ║
║                                                                 ║
║ To *uninstall* current version and re-install latest "chrome":  ║
║                                                                 ║
║ - Close all running instances of "chrome", if any               ║
║ - Use "--force" to install browser:                             ║
║                                                                 ║
║     npx playwright install --force chrome                       ║
║                                                                 ║
║ <3 Playwright Team                                              ║
╚═════════════════════════════════════════════════════════════════╝

[WebServer] (node:96919) [DEP0066] DeprecationWarning: OutgoingMessage.prototype._headers is deprecated
[WebServer] (Use `node --trace-deprecation ...` to show where the warning was created)

Running 1 test using 1 worker
·
  1 passed (6.7s)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to have this gitignore here, rather than at the top level of the repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's nice not to pollute the top level gitignore?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm the opposite - I prefer to know where the single point of configuration is, rather than potentially hunting every level of a directory tree. However, this wouldn't be the first "non-root" .gitignore, and I don't feel that strongly about it, so I won't stand in the way of progress.

hoodmane added 2 commits July 23, 2025 10:34
* Add shebang
* chmod +x
* npx playwright install
@hoodmane hoodmane requested a review from brettcannon as a code owner July 23, 2025 08:50
@brettcannon brettcannon removed their request for review July 23, 2025 19:17
@freakboy3742 freakboy3742 added the needs backport to 3.14 bugs and security fixes label Jul 24, 2025
Copy link
Contributor

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me - the extra updates to the run script are a nice touch.

@freakboy3742 freakboy3742 merged commit ae4d27e into python:main Jul 24, 2025
77 of 81 checks passed
@miss-islington-app
Copy link

Thanks @hoodmane for the PR, and @freakboy3742 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 24, 2025
pythonGH-137004)

Adds a mechanism to test browser-based initialisation of the Python interpreter,
via a Playwright headless browser instance.
(cherry picked from commit ae4d27e)

Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Jul 24, 2025

GH-137067 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jul 24, 2025
@hoodmane hoodmane deleted the emscripten-pyrepl-browser-smoke-test branch July 24, 2025 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS-emscripten skip news topic-repl Related to the interactive shell
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants