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.

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

Update CI #396

Merged
merged 5 commits into from
Nov 7, 2024
Merged

Update CI #396

merged 5 commits into from
Nov 7, 2024

Conversation

trungleduc
Copy link
Member

@trungleduc trungleduc commented Nov 5, 2024

Code changes

  • Reorganize test workflow

Screenshot 2024-11-05 144138

  • Drop python 3.8
  • Remove uneeded steps in the pre-commit job
  • Run UI test with packages generated from current PR, not from PYPI.
  • Relax Playwright UI test condition.
  • Add dev install script: calling python scripts/dev_install.py or yarn dev is enough to set up the dev enviroment.
  • Add update snapshot workflow

Copy link
Contributor

github-actions bot commented Nov 5, 2024

Binder 👈 Launch a Binder on branch trungleduc/jupyter-collaboration/update-ci

@trungleduc
Copy link
Member Author

@jtpio I'm getting this error in the Check Release job: ValueError: Tag v3.0.0 already exists. Do you know what happened?

@jtpio
Copy link
Member

jtpio commented Nov 5, 2024

Looks like it may be related to the changes to the yarn.lock file?

It seems it's not able to bump the version to 3.0.1 in the workflow, which could explain the existing tag issue:

image

@jtpio
Copy link
Member

jtpio commented Nov 5, 2024

Probably not related but something that should also be fixed on CI: #380

@trungleduc
Copy link
Member Author

Looks like it may be related to the changes to the yarn.lock file?

It seems it's not able to bump the version to 3.0.1 in the workflow, which could explain the existing tag issue:

image

Thanks!

@trungleduc
Copy link
Member Author

Probably not related but something that should also be fixed on CI: #380

Ok i will take care of this

@trungleduc
Copy link
Member Author

Failed UI test looks weird but i'm not sure it relates to this PR

  • Actual

initialization-open-notebook-guest-actual

  • Expected

initialization-open-notebook-guest-expected

Copy link
Collaborator

@davidbrochart davidbrochart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @trungleduc, that's great.
Since you dropped Python 3.8 in the CI, do you want to update the dependency in pyproject.toml too?
Also, can you reproduce the new lines in the cell stream output locally?

@trungleduc
Copy link
Member Author

do you want to update the dependency in pyproject.toml too?

isn't it a breaking change? I thought that main is targeting 3.1.x

@davidbrochart
Copy link
Collaborator

I don't think we should make a major release for that. Python 3.8 has reached end of life, I think all Jupyter projects should drop support for it too.

@jtpio
Copy link
Member

jtpio commented Nov 6, 2024

Right it's probably fine to drop support for a Python version in a minor release. For example JupyterLab dropped support for Python 3.6 in JupyterLab 3.3: https://jupyterlab.readthedocs.io/en/latest/getting_started/changelog.html#id298

As long as it's visible in the changelog and/or the docs.

@jtpio
Copy link
Member

jtpio commented Nov 6, 2024

So the UI tests seem to be failing consistently after a restart. It would be worth looking into it as otherwise we would have to live with a failing CI, which is going to be annoying very quickly.

@trungleduc
Copy link
Member Author

So the UI tests seem to be failing consistently after a restart. It would be worth looking into it as otherwise we would have to live with a failing CI, which is going to be annoying very quickly.

This is an issue of JupyterLab, I opened the issue here jupyterlab/jupyterlab#16934

@trungleduc
Copy link
Member Author

@jtpio, how should we proceed with this PR? Should we wait for a new JupyterLab patch version or update the snapshot?

@jtpio
Copy link
Member

jtpio commented Nov 7, 2024

Does this behavior happen for real users of jupyter-collaboration, not just in the UI tests?

If so, it will probably make sense to update to the "wrong" snapshot for now to indicate this is an expected issue. And then fix it when a new JupyterLab release with the fix is available?

@trungleduc
Copy link
Member Author

trungleduc commented Nov 7, 2024

Does this behavior happen for real users of jupyter-collaboration, not just in the UI tests?

If so, it will probably make sense to update to the "wrong" snapshot for now to indicate this is an expected issue. And then fix it when a new JupyterLab release with the fix is available?

I can reproduce it locally jupyterlab/jupyterlab#16934

it will probably make sense to update to the "wrong" snapshot for now to indicate this is an expected issue. And then fix it when a new JupyterLab release with the fix is available?

I agree

@jtpio
Copy link
Member

jtpio commented Nov 7, 2024

OK then let's update the snapshot so "CI passes" and the PR can be merged?

Thanks @trungleduc for looking into this!

@trungleduc
Copy link
Member Author

it's green!

@davidbrochart davidbrochart merged commit 3f9b0db into jupyterlab:main Nov 7, 2024
27 checks passed
@trungleduc trungleduc mentioned this pull request Nov 7, 2024
trungleduc added a commit to trungleduc/jupyter-collaboration that referenced this pull request Nov 7, 2024
* Update CI

* Add dev install script

* Automatic application of license header

* Add update snapshots workflow

* Update snapshots

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
trungleduc added a commit to trungleduc/jupyter-collaboration that referenced this pull request Nov 7, 2024
* Update CI

* Add dev install script

* Automatic application of license header

* Add update snapshots workflow

* Update snapshots

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
trungleduc added a commit to trungleduc/jupyter-collaboration that referenced this pull request Nov 7, 2024
* Update CI

* Add dev install script

* Automatic application of license header

* Add update snapshots workflow

* Update snapshots

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
davidbrochart pushed a commit that referenced this pull request Nov 8, 2024
* Update CI

* Add dev install script

* Automatic application of license header

* Add update snapshots workflow

* Update snapshots

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants