Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use docker on CI to speed it up #15916

Draft
wants to merge 40 commits into
base: main
Choose a base branch
from
Draft

Use docker on CI to speed it up #15916

wants to merge 40 commits into from

Conversation

fcollonval
Copy link
Member

@fcollonval fcollonval commented Mar 5, 2024 •
edited
Loading

References

Fixes #13600
Closes #11682

Code changes

Building on top of the Docker file we are maintaining for dev (via devcontainer or directly), this switches the CI stack to first build a test docker image and feed it to all most of the tests.

It modifies slightly jupyterlab.browser_check to only install a single web browser based on JLAB_BROWSER_TYPE (that variable was already defined in browser-test.js).

The speed up seen is substantial as creating the docker image take ~8min. But most job execution using it take 3 to 4 min less. It also allows for sharding the integration tests to speed up the overall execution.

Remaining:

  • Access data outside the docker to store as artifacts (like for integration or release tests) --> needs to use volume
  • Should we speed up the build by caching outside of the Docker image the yarn cache (and potentially the pip cache). This will fasten the build when the yarn.lock changes.
  • Should we push a docker image per PR? The issue is that the currently test image is 2.5Go. So if we go that path, I think the only reasonable image would be a minimal python image with JupyterLab installed (assets non-minimized) to allow user testing but not all CI tests.

More optimization:

  • Cache Playwright chromium flavor.

User-facing changes

None

Backwards-incompatible changes

None

@jupyterlab-probot jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@fcollonval
Copy link
Member Author

First test is encouraging, building the docker image took 6minutes then the 3 tests passing took 3min less (-20% to -35%) than the current job.

@fcollonval fcollonval force-pushed the maintenance/docker-ci branch from ba04b7f to 8c30068 Compare March 17, 2024 13:59
@fcollonval
Copy link
Member Author

Sharding of playwright tests is working well. But of course lots of tests are failing likely because the OS config has changed (we are running in the docker). A good side-effect of this PR will be the ability for dev to update the snapshots locally - meaning I'll keep a workflow for it; but we should advice knowledgeable dev to prefer updating just what is needed locally.

@fcollonval fcollonval mentioned this pull request Mar 19, 2024
Merged
@jtpio jtpio mentioned this pull request Mar 20, 2024
Closed
@fcollonval
Copy link
Member Author

Just to keep a trace; at the call it was requested to merge the integration tests reports (as this PR splits them). It is supported by Playwright: https://playwright.dev/docs/test-sharding#merging-reports-from-multiple-shards

@fcollonval fcollonval force-pushed the maintenance/docker-ci branch from 4e8acd0 to dd4b665 Compare March 25, 2024 09:50
@fcollonval fcollonval force-pushed the maintenance/docker-ci branch 2 times, most recently from 5577d7c to 7a08e6a Compare March 26, 2024 13:35
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 28, 2024
View reviewed changes
examples/example_check.py
str(Path(os.environ.get("DOCKER_VOLUME", "")).joinpath("pw-test-results")),
]
)
except BaseException:

Check notice

Code scanning / CodeQL

Except block handles 'BaseException' Note

Except block directly handles BaseException.
examples/example_check.py
...
try:
await run_async_process(["ls", "-l", str(results_target)])
except BaseException:

Check notice

Code scanning / CodeQL

Except block handles 'BaseException' Note

Except block directly handles BaseException.
examples/example_check.py
]
)
except BaseException:
...

Check notice

Code scanning / CodeQL

Statement has no effect Note

This statement has no effect.
examples/example_check.py
try:
await run_async_process(["ls", "-l", str(results_target)])
except BaseException:
...

Check notice

Code scanning / CodeQL

Statement has no effect Note

This statement has no effect.
@krassowski krassowski mentioned this pull request Apr 1, 2024
Open
This was referenced Apr 9, 2024
Merged
Merged
@JasonWeill JasonWeill mentioned this pull request Jul 3, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@fcollonval fcollonval

Labels
documentation maintenance pkg:application pkg:apputils pkg:attachments pkg:cell-toolbar pkg:cells pkg:celltags pkg:codeeditor pkg:codemirror pkg:completer pkg:console pkg:coreutils pkg:csvviewer pkg:debugger pkg:docmanager pkg:docregistry pkg:documentsearch pkg:extensionmanager pkg:filebrowser pkg:fileeditor pkg:help pkg:htmlviewer pkg:hub pkg:imageviewer pkg:inspector pkg:javascript pkg:json pkg:launcher pkg:logconsole pkg:lsp pkg:mainmenu pkg:markdownviewer pkg:mathjax pkg:mermaid pkg:metadataform pkg:notebook pkg:observables pkg:outputarea pkg:pdf pkg:pluginmanager pkg:property-inspector pkg:rendermime pkg:running pkg:services pkg:settingeditor pkg:settingregistry pkg:shortcuts pkg:statedb pkg:statusbar pkg:terminal pkg:themes pkg:toc pkg:tooltip pkg:translation pkg:ui-components pkg:vega pkg:workspaces tag:Examples tag:Testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve CI jobs speed
1 participant
@fcollonval