-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
gh-124621: Emscripten: Add smoke test for using pyrepl in Chrome #137004
Conversation
This should make it less likely that we break Python startup in the browser.
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.
+1 to having a smoke test; this looks like it will do the job.
Two comments:
-
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.
-
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.
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 And if you don't make the requested changes, you will be put in the comfy chair! |
... and then I see python/buildmaster-config#616 :-) We still need to add documentation for how to use this smoke test. |
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.
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)
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.
Any reason to have this gitignore here, rather than at the top level of the repo?
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.
I think it's nice not to pollute the top level gitignore?
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.
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.
* Add shebang * chmod +x * npx playwright install
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.
Looks good to me - the extra updates to the run script are a nice touch.
Thanks @hoodmane for the PR, and @freakboy3742 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
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>
GH-137067 is a backport of this pull request to the 3.14 branch. |
Uses playwright. This should make it less likely that we break Python startup in the browser.