Add Jest, Playwright and pytest tests #207
Conversation
fcollonval
changed the title
| "@jupyterlab/builder": "^3.1.0", | ||
| "@jupyterlab/galata": "^4.1.0", |
There was a problem hiding this comment.
i dunno man... while webpack+babel and friends are heavy, the galata/playwright stack is huge, and i've seen a lot of experienced folk struggle with getting to Works On My Machine with it. If this is going in-by-default, I feel like the pattern of putting the heavyweight browser tests in a separate ./ui-tests/package.json and yarn.lock which can be fired off from the main package.json makes more sense.
From a CI value perspective: i've found it pays good dividends for heavyweight ui tests to be in an entirely separate, dependent, os-matrixed job where the "as-would-be-released" .whl and .tar.gz assets are installed and tested against. This allows the ui tests to fail really fast if something isn't packaged properly, which is usually much worse than being off by a few pixels in screenshot comparisons.
| opens the server to the world and provide access to JupyterLab | ||
| JavaScript objects through the global window variable. | ||
| """ | ||
| from tempfile import mkdtemp |
There was a problem hiding this comment.
real estate in the root folder is precious: recommend putting this in the tests folder
| c.ServerApp.port_retries = 0 | ||
| c.ServerApp.open_browser = False | ||
|
|
||
| c.ServerApp.root_dir = mkdtemp(prefix='galata-test-') |
There was a problem hiding this comment.
this won't clean up after itself
There was a problem hiding this comment.
The setup is done like that to mimic pytest system (without the auto clean-up feat though).
The UI tests can easily fail if the filebrowser is crowded by existing files (due to past test runs or previous tests). To mitigate that problem, galata is creating a folder for each test (using the test name). That folder and its content will be deleted in the teardown step. So the temporary folder is likely to end up empty at the end of a test run; except if an error occurs. If an error occurs, the files are not removed (default behavior). So they can be access afterwards for introspection. That behavior is controlled by a fixture.
So I don't see any problem crowding the temporary folder as per definition it is the first place to clean if you end up having low disk space. And this is much easier than going through tens of projects folder for removing test artifacts.
| @@ -0,0 +1,8 @@ | |||
| import pytest | |||
|
|
|||
| pytest_plugins = ["jupyter_server.pytest_plugin"] | |||
There was a problem hiding this comment.
i thought the newer pytest started warning about doing this in a config (e.g. pyproject.toml, setup.cfg, tox.ini or pytest.ini)
There was a problem hiding this comment.
This is the way documented in the official documentation
| integration-tests: | ||
| name: Integration tests | ||
| needs: build | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
while ubuntu is a nice, predictable, well-behaved platform, windows is frankly where, from an overall integration testing perspective, one is most likely to get real data and catch evil things.
other fun things to find extremely predictable bugs:
- installing/running/serving from paths with non-ascii characters and spaces
- non-
c:-drive installs - running with a non-
/base url
- Handle server spin up in Playwright config - Cache browser download - Use one artifact - Don't constrain explicitly Python
|
@fcollonval You should be able to merge your changes after a rebase from |
|
Thanks @3coins CI is now green. |
|
Thanks! cc @3coins if you want to have a final look |
|
@jtpio |
|
@3coins I applied your suggestion and add the feature under a flag as requested during the JupyterLab meeting. Is the latest version good for you? |
|
@fcollonval |
Fixes #137