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

Fix visual tests, allow to disable termianal cursor blinking #15524

Merged
merged 15 commits into from Dec 13, 2023

Conversation

krassowski
Copy link
Member

References

Code changes

  • add performance.throttleNetwork galata helper and use it to solve flakiness of completer test when trying to catch the loading indicator,
  • move code for documentation tests which required serial mode from "general" to dedicated "overview" file, make them run in serial
  • make customization docs tests run in serial
  • mock the terminal tree command output for terminal snapshot
  • switch to data directory in a few places to avoid test- directories form concurrent tests showing up in the snapshots

User-facing changes

We always supported cursorBlink in API but it was not exposed. This PR adds it to user-facing schema. We need it for tests, and for accessibility, but we should remove it from backport and only release in 4.1.

Screenshot from 2023-12-13 01-11-50

Backwards-incompatible changes

None

Copy link

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

@krassowski krassowski marked this pull request as ready for review December 13, 2023 03:42
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Awesome 🚀
Thanks @krassowski

I have a minor suggestion. Otherwise it is good for me.

galata/src/helpers/performance.ts Outdated Show resolved Hide resolved
galata/test/jupyterlab/completer.test.ts Outdated Show resolved Hide resolved
@jtpio
Copy link
Member

jtpio commented Dec 13, 2023

Thank you @krassowski!!

Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
@krassowski
Copy link
Member Author

In one run these two failed, on re-run they passed. I think these will be addressed by #15375.

Notice:   2 failed
    [jupyterlab] › test/jupyterlab/debugger.test.ts:123:7 › Debugger Tests › Start debug session (Script) 
    [jupyterlab] › test/jupyterlab/notebook-run.test.ts:33:7 › Notebook Run › Run Notebook and capture cell outputs 

Good to merge?

Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

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

Thanks!

@krassowski krassowski merged commit c26a0e7 into jupyterlab:main Dec 13, 2023
77 of 79 checks passed
@krassowski
Copy link
Member Author

@meeseeksdev please backport to 4.0.x

Copy link

lumberbot-app bot commented Dec 13, 2023

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout 4.0.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 c26a0e7cced6eea64a7f0908a7cded8e4c8874fb
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #15524: Fix visual tests, allow to disable termianal cursor blinking'
  1. Push to a named branch:
git push YOURFORK 4.0.x:auto-backport-of-pr-15524-on-4.0.x
  1. Create a PR against branch 4.0.x, I would have named this PR:

"Backport PR #15524 on branch 4.0.x (Fix visual tests, allow to disable termianal cursor blinking)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

krassowski added a commit to krassowski/jupyterlab that referenced this pull request Dec 29, 2023
* Try to make the test terminal snapshot more robust

by mock `tree . -L 2` command

* Workaround `test-` dirs issue by capturing `/data` instead of `/`

* Update Playwright Snapshots

* Run customization docs snapshots in serial

* Ignore all test files in galata tests

* Update Playwright Snapshots

* Disable concurrency in `build:examples` to fix flaky test

This was already use in `build:src`

* Ensure no flaky focus indicator on snapshot

(cherry picked from commit c26a0e7)
krassowski added a commit that referenced this pull request Jan 30, 2024
* Backport PR #15524: Fix visual tests

* Try to make the test terminal snapshot more robust

by mock `tree . -L 2` command

* Workaround `test-` dirs issue by capturing `/data` instead of `/`

* Update Playwright Snapshots

* Run customization docs snapshots in serial

* Ignore all test files in galata tests

* Update Playwright Snapshots

* Disable concurrency in `build:examples` to fix flaky test

This was already use in `build:src`

* Ensure no flaky focus indicator on snapshot

(cherry picked from commit c26a0e7)

* Update Playwright Snapshots

* Revert some snapshot updates

* Update Playwright Snapshots

* Try to fix "General › Welcome" documentation snapshot

---------

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.

Flaky Linux Tests / Linux (examples, 3.11) (pull_request) test (concurrency again?)
3 participants