switch to browser testing in unit tests#12998
Conversation
🪼 branch checks and previews
Install Gradio from this PR pip install https://gradio-pypi-previews.s3.amazonaws.com/5774a420411baf64c8aea0c7fb61e8ab3d535219/gradio-6.9.0-py3-none-any.whlInstall Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@5774a420411baf64c8aea0c7fb61e8ab3d535219#subdirectory=client/python"Install Gradio JS Client from this PR npm install https://gradio-npm-previews.s3.amazonaws.com/5774a420411baf64c8aea0c7fb61e8ab3d535219/gradio-client-2.1.0.tgz |
🦄 change detectedThis Pull Request includes changes to the following packages.
✅ Changeset approved by @pngwn
|
freddyaboulton
left a comment
There was a problem hiding this comment.
Looks good @pngwn ! This is very great. Just a note that the client tests are running in the browser but looks like the code still expects to run in node so its failing in ci.
|
Addressed the comments and fixed the client tests. The client tests were still running in a fake browser environment in node but since we changed I've abstracted the The client tests now run correctly in pure node and a real browser. |
|
cc @43081j! |
|
We were running clients tests in their own steps but some of them were also run as part of the main load of tests, i fixed that. The unit tests are actually now faster than before. Maybe because of the cleanup, maybe because fake browser environments are actually very heavy, hard to say. So any performance concerns were misplaced on my part. |
| } | ||
|
|
||
| if (!IS_NODE) { | ||
| globalThis.Buffer = FakeBuffer as unknown as BufferConstructor; |
There was a problem hiding this comment.
Should these tests be running at all outside node? In the production code, how do we end up with Buffer in browsers?
There was a problem hiding this comment.
The client is a standalone package designed to run in both the browser and node, so we want to test it for both. We do actually instantiate the client in SSR too in the main gradio app.
There was a problem hiding this comment.
Oh right i get you now, let me try to remember.
There was a problem hiding this comment.
I'm trying to remember and I think I had some issue (only in tests) with the Buffer global not being defined but maybe I need to look at this more closely to see if this is necessary at all.
the short version is that we prefer to use the File class which is supported in both node and browsers but we currently support a node version where file isn't available so we have some kind of switch to flip between them, so maybe there is an issue in the client itself. We can probably drop support for that node version now tbh, its a few versions old.
There was a problem hiding this comment.
But to answer the question, I don't think we actually try to get a Buffer outside of node, rather the fact that it wasn't present was causing an issue.
Description
This revives the old browser testing PR. This is the minimum changes to get our unit tests running in a browser and all passing. Assuming Ci is green this should be good but if some ppl could pull it down and run the tests, that would be great too.
AI Disclosure
We encourage the use of AI tooling in creating PRs, but the any non-trivial use of AI needs be disclosed. E.g. if you used Claude to write a first draft, you should mention that. Trivial tab-completion doesn't need to be disclosed. You should self-review all PRs, especially if they were generated with AI.
🎯 PRs Should Target Issues
Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.
Not adhering to this guideline will result in the PR being closed.
Testing and Formatting Your Code
PRs will only be merged if tests pass on CI. We recommend at least running the backend tests locally, please set up your Gradio environment locally and run the backed tests:
bash scripts/run_backend_tests.shPlease run these bash scripts to automatically format your code:
bash scripts/format_backend.sh, and (if you made any changes to non-Python files)bash scripts/format_frontend.sh