-
Notifications
You must be signed in to change notification settings - Fork 36
Run notebooks in Jupyter Lite as part of ci (Emscripten xeus-cpp automated testing) #415
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
base: main
Are you sure you want to change the base?
Run notebooks in Jupyter Lite as part of ci (Emscripten xeus-cpp automated testing) #415
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #415 +/- ##
=======================================
Coverage 81.94% 81.94%
=======================================
Files 21 21
Lines 853 853
Branches 87 87
=======================================
Hits 699 699
Misses 154 154 🚀 New features to boost your workflow:
|
438015e to
2d68e5c
Compare
|
See here https://github.com/compiler-research/xeus-cpp/actions/runs/19553412423/job/55990268776?pr=415#step:7:605 for an example of what the output looks like when the notebook it gets once finished running being different to the reference notebook (in this case the difference comes from the fact the notebook didn't have enough time to finish running on the Github runner) Here is the output for when the workflow output cannot be seen anymore |
| options = ChromeOptions() | ||
| options.add_argument("--headless") | ||
| options.add_argument("--no-sandbox") | ||
| driver = webdriver.Chrome(options=options) | ||
|
|
||
| elif args.driver == "firefox": | ||
| options = FirefoxOptions() | ||
| options.add_argument("--headless") | ||
| driver = webdriver.Firefox(options=options) |
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.
Note to self, these headless options should be enabled by command line argument, in case the user locally wants to see the notebook running in a gui. Only needed due to ci needing them run in a headless browser
5ef8ec6 to
519adf9
Compare
| ) | ||
| ) | ||
| actions.move_to_element(run_all_menu).click().perform() | ||
| time.sleep(200) |
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.
These time.sleeps to run all the cells will depend on the power of the cpu running the tests. Adjusted for now to be long enough that the notebook has enough time to run on the Github runners. Locally these times can be a lot shorter.
Long term this should be switched out without something which determines if the kernel has gone idle, and aborts if he has taken too long (assumed notebook has stalled at this point). This I feel can be done as part of subsequent PR though.
0e28cd7 to
3eaff7e
Compare
|
Hi, I would recommend starting out with testing the demo notebook instead of the smallpt notebook. Testing the smallpt notebook doesn't really add value. Our smallpt notebook gives an impression that SDL is usable in JupyterLite but it's just a gimmick to be fair 😅 . if you check the notebook, we use SDL only to render to a static bitmap instead of using it for any "dynamic" opengl canvas related stuff. As xeus-cpp is run in the worker thread and we need the main thread for any events or dynamic stuff .... we're just masking SDL to convert a raw stream to a bitmap (we don't need sdl for this) So testing the smallpt notebook != SDL works through xeus-cpp-lite. No events, no dynamics, no canvases For events (mouse/keyboard etc), dynamic canvases ... we probably can put the jupyter widgets (ipywidgets, xwidgets) to use for now. Check example notebooks here for the above. |
Hi @anutosh491 For I am just trying to show a technique for running any notebook in Jupyter lite using Selenium. I am not too fussed at the moment that it isn't actually making use of SDL. I picked smallpt just because its the easiest case to start off with since it requires no user input besides executing all the cells. The ci now shows it works for Firefox and Chrome. It works locally for Safari too. I am just debugging what is going on with the download part for this browser, since it doesn't appear to be downloading the notebook for this browser in the ci. Once I have this working completely in the ci, my next goal is to work on the normal demo notebook, and add to the script so it can deal with the case when a notebook requests user input (std::cin case). The next stage I was hoping we could make a series of reference notebooks to run through the ci which check we don't break things between PRs (these don't necessarily need to be ones we deploy, and can be in a separate compiler research repo). Basically the script will be modified for increasingly difficult to execute notebooks until we have something that can deal with most notebooks we give it. |
|
@anutosh491 I have now got the most basic notebook (smallpt.ipynb) to try and run completely in the ci (basic in the sense it requires no human interaction beyond running all the cell), even if it doesn't show much. I will now move onto getting the demo notebook to run in the ci, so we can deal with requests for user input. In the meantime you might want to think of ideas for notebooks that it might be good to be able to run through the ci. |
1847aa0 to
a92a91c
Compare
a92a91c to
0710b5b
Compare
Description
Please include a summary of changes, motivation and context for this PR.
This PR is the first in a series of PRs to add automated testing to check changes don't break xeus-cpps existing jupyter lite notebooks, in a ci friendly way. This PR first PR deals with smallpt.ipynb . The workflow for checking nothing is broken
First run python script with driver option set to browser you want to run the notebook in (the ci will cover all options, firefox, chome and safari). This should be run while the jupyter server command is running, with --settings-overrides=overrides.json as the first option to provide the download button.
This will download the notebook once it has finished running (the reason for adding overrides.json to add Download option to notebook menu bar). This config to add this was taken from https://jupyterlite.readthedocs.io/en/latest/howto/configure/settings.html . Right clicking the file, finding the download option and clicking didn't work consistently, and never after the notebook had finished running in the case of Safari.
Final step is compare the fully executed notebook in repo (which will be used as referenced) against the fully run notebook downloaded as part of the python script using nbdiff which comes from jupyters nbdime https://github.com/jupyter/nbdime .
If the notebooks are the same nbdiff outputs nothing and the ci carries on. If they are different, nbdiff returns a non zero value and the ci stops with the outputted diff.
This idea is effectively option 2 suggested here #259 (comment), but I am using Selenium instead of Playwright. I have checked the above steps locally, but am setting this PR to draft until I have had a chance to update the documentation and ci to do these automated notebook tests.
This PR will add the
Fixes # (issue)
Type of change
Please tick all options which are relevant.