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 with jupyter-ydoc v3.0.2 #412

Merged
merged 4 commits into from
Dec 2, 2024

Conversation

davidbrochart
Copy link
Collaborator

This should use jupyter-ydoc v3.0.2, and UI tests should fail because we updated the snapshots here.

Copy link
Contributor

github-actions bot commented Dec 2, 2024

Binder 👈 Launch a Binder on branch davidbrochart/jupyter_collaboration/jupyter-ydoc-v3.0.2

@trungleduc
Copy link
Member

please update snapshots

@krassowski
Copy link
Member

It looks like the tests did not fail here even though 3.0.2 was installed?

I see jupyter_ydoc-3.0.2-py3-none-any.whl is used.

@davidbrochart
Copy link
Collaborator Author

Indeed, I don't understand.

@davidbrochart
Copy link
Collaborator Author

Oh but running UI tests locally I see that the text is rendered in one line, maybe we changed the snapshots again after #396?

@davidbrochart
Copy link
Collaborator Author

My bad, locally I have as expected:
image

And as actual (I changed to Lorem2 in the code source):
image

@trungleduc
Copy link
Member

it looks ok now

@trungleduc
Copy link
Member

please update snapshots

@davidbrochart
Copy link
Collaborator Author

it looks ok now

But there are 4 failed tests now?

@davidbrochart
Copy link
Collaborator Author

It's hard to believe that lowering maxDiffPixelRatio makes the test fail, otherwise we were not testing anything before? The difference was huge, because we had a text in one column instead of one line.

@trungleduc trungleduc closed this Dec 2, 2024
@trungleduc trungleduc reopened this Dec 2, 2024
@davidbrochart
Copy link
Collaborator Author

The updated snapshots look good:
image

But isn't it revealing an issue in the tests themselves?

@trungleduc
Copy link
Member

It's hard to believe that lowering maxDiffPixelRatio makes the test fail, otherwise we were not testing anything before? The difference was huge, because we had a text in one column instead of one line.

the diff is computed by the number of changed pixels, so maybe for the text, the diff is big but not too much in terms of pixel number.

@davidbrochart
Copy link
Collaborator Author

Fair enough, but then tests should likely not have failed in #396 too.

@trungleduc
Copy link
Member

Fair enough, but then tests should likely not have failed in #396 too.

yeah, flakiness is the beauty of the UI tests.

@krassowski
Copy link
Member

What do you think about excluding 3.0.0 and 3.0.1 from dependencies using != in:

@davidbrochart
Copy link
Collaborator Author

Good point, let's do it.

@davidbrochart davidbrochart changed the title Empty commit Update with jupyter-ydoc v3.0.2 Dec 2, 2024
@davidbrochart davidbrochart merged commit a4f6108 into jupyterlab:main Dec 2, 2024
28 checks passed
@davidbrochart davidbrochart deleted the jupyter-ydoc-v3.0.2 branch December 2, 2024 18:52
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