mwouts / jupytext

Jupyter Notebooks as Markdown Documents, Julia, Python or R scripts
https://jupytext.readthedocs.io
MIT License
6.6k stars 386 forks source link

Update JS dependencies #1229

Closed mwouts closed 4 months ago

github-actions[bot] commented 5 months ago

Thank you for making this pull request.

Did you know? You can try it on Binder: Binder:lab or Binder:notebook.

Also, the version of Jupytext developed in this PR can be installed with pip:

HATCH_BUILD_HOOKS_ENABLE=true pip install git+https://github.com/mwouts/jupytext.git@update_js_dependencies

(this requires nodejs, see more at Developing Jupytext)

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.73%. Comparing base (c37f3e0) to head (e827858).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1229 +/- ## ======================================= Coverage 97.73% 97.73% ======================================= Files 29 29 Lines 4464 4464 ======================================= Hits 4363 4363 Misses 101 101 ``` | [Flag](https://app.codecov.io/gh/mwouts/jupytext/pull/1229/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | Coverage Δ | | |---|---|---| | [external](https://app.codecov.io/gh/mwouts/jupytext/pull/1229/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `75.17% <ø> (ø)` | | | [functional](https://app.codecov.io/gh/mwouts/jupytext/pull/1229/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `88.51% <ø> (ø)` | | | [integration](https://app.codecov.io/gh/mwouts/jupytext/pull/1229/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `77.28% <ø> (ø)` | | | [unit](https://app.codecov.io/gh/mwouts/jupytext/pull/1229/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `66.56% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mwouts commented 5 months ago

Hi @mahendrapaipuri , I am trying to update the UI tests as they fail on the CI, but I am not sure how to do this? Thanks!

mahendrapaipuri commented 5 months ago

Why are you updating the screenshots? If you look into the new screenshots you are trying to update, there are kernels that are being picked from your local test env like itables, javascript, etc. But in our UI test env, we are just installing bash kernel to test the extension with a non-python kernel.

Normally, if you dont touch the extension code, you dont have to run the UI tests locally. If you really want to include more kernels in the test env (which I dont think it is necessary), you will need to update [test-ui] dependencies in pyproject.toml to install these kernels and then update screenshots.

Does it make sense?

mwouts commented 5 months ago

Hi @mahendrapaipuri , thanks for your comment! Indeed you're correct I don't want to add a new kernel on the CI. The reason why I am trying to update the UI tests is that they currently fail on the main branch (see e.g. this run).

As I was not able to trigger an update through please update playwright snapshots (see my attempts above) I tried to update them locally, but as you point out that comes with more differences. Do you think you could trigger a UI test update, maybe on another branch? Thanks!

mahendrapaipuri commented 5 months ago

Hello @mwouts Understood. Some pointers that might be useful here:

mwouts commented 5 months ago

Thanks @mahendrapaipuri ! Oh it's great that I can download the updated UI results, I will make a PR from this! Re the automation, thanks for the pointers too.