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

Benchmark report

The execution time (in milliseconds) are grouped by test file, test type and browser.
For each case, the following values are computed: min <- [1st quartile - median - 3rd quartile] -> max.

The mean relative comparison is computed with 95% confidence.

LGTM

Well, unfortunately most of my tests still fail. My feeling is that we should do this when there is at least one other person that has problems with local UI tests. Otherwise this PR would be only about fixing my local dev environment issues.

Well, unfortunately most of my tests still fail. My feeling is that we should do this when there is at least one other person that has problems with local UI tests. Otherwise this PR would be only about fixing my local dev environment issues.

I do. My super inefficient workflow was to open a PR on my own fork to get the CI to kick in, download artifacts and then cleanly push the correct screenshot in the PR to this repo. Having majority of the tests pass locally would be super useful for good developer experience; the final 20% may not be worth the effort, but if we can get 80% to pass that would be a big step forward.

What things still cause problems on your setup?

@krassowski that sounds familiar to me. Only that I have polluted the tests on the main repo (sorry) ... so let's get on with this.

1. the commit a7f5c5d should fix the collapsible-headings test. @krassowski can you confirm this locally?

2. the commit d306bdc shows something interesting. It definitely improves the contextmenu test. However:
   
   Is this antialiasing?
   @fcollonval just to be sure, can you check the font of ui elements on your system?
   Update: a default pixelmatch threshold of 0.3 in playwright-config.ts helps. But this is potentially dangerous. A simple check (add whitespace between ** and 3 in test "Execute Code cell") shows that differences are still detected with 0.3 threshold.

3. Now I have still lots of fails with italics
   

I don't understand that one, though. Chrome tells me it's rendering DejaVu Sans Mono.

I guess I'm lucky. Running on stock Debian 11, running the 158 tests on 4 workers:

  12 failed
    [jupyterlab] › test/jupyterlab/general.test.ts:8:3 › General Tests › Launch Screen =============
    [jupyterlab] › test/jupyterlab/general.test.ts:13:3 › General Tests › Enter Simple Mode ========
    [jupyterlab] › test/jupyterlab/general.test.ts:28:3 › General Tests › Toggle Dark theme ========
    [jupyterlab] › test/jupyterlab/notebook-toolbar.test.ts:78:3 › Notebook Toolbar › Run cell =====
    [jupyterlab] › test/jupyterlab/sidebars.test.ts:60:3 › Sidebars › Capture File Browser on right 
    [jupyterlab] › test/jupyterlab/terminal.test.ts:21:5 › Terminal › Theme › Light theme terminal inherit 
    [jupyterlab] › test/jupyterlab/terminal.test.ts:33:5 › Terminal › Theme › Light theme terminal light 
    [jupyterlab] › test/jupyterlab/terminal.test.ts:43:5 › Terminal › Theme › Light theme terminal dark 
    [jupyterlab] › test/jupyterlab/terminal.test.ts:53:5 › Terminal › Theme › Dark theme terminal inherit 
    [jupyterlab] › test/jupyterlab/terminal.test.ts:66:5 › Terminal › Theme › Dark theme terminal light 
    [jupyterlab] › test/jupyterlab/terminal.test.ts:77:5 › Terminal › Theme › Dark theme terminal dark 
    [jupyterlab] › test/jupyterlab/toc.test.ts:217:3 › Table of Contents › Open context menu =======
  2 flaky
    [jupyterlab] › test/jupyterlab/debugger.test.ts:33:3 › Debugger Tests › Start debug session ====
    [jupyterlab] › test/jupyterlab/notebook-create.test.ts:41:3 › Notebook Create › Create a Code cell 
  144 passed (9m)

Terminal related tests are expected to fail. The general tests and the sidebar one are failing because I'm using nb_conda_kernels and so all my environments show up in the launcher (we could mock kernelspec endpoint to improve that one).

Regarding the font for menu, here are the details:

Regarding the discrepancy on the font, I don't know if it could come from anti-aliasing. But I'm a bit afraid of using higher threshold (the default seems to be 0.2). The small discrepancy like the difference between triangle icon showing collapsed/expanded may not be catch. But it is a risk we need to evaluate against dev' comfort.

@schmidi314 in your screencast with font diff, was the test ran twice? If so was the diff identical in the two attempts?

in your screencast with font diff, was the test ran twice? If so was the diff identical in the two attempts?

Just re-ran it twice manually. Yes they are the same.

As for the font: Thanks, @fcollonval , that matches with my config.

But I'm a bit afraid of using higher threshold (the default seems to be 0.2).

Yes, I know what you mean. But with many of my tests failing with minor font issues (the italics stuff I mean), I'm gaining more confidence with the 0.3 threshold.

@fcollonval can you please check fc-list | grep 'Menlo\|Consolas'

fc-list | grep 'Menlo|Consolas'

This returns no match

@krassowski @jtpio would you mind running galata test on this PR locally and report the test results? If possible could you test it a second time not specifying the snapshot threshold in galata/src/playwright-config.ts?

Another observation:

Shouldn't the result be like "actual", rather than "expected"?

I was going through PRs added to the 3.2.x milestone and found this one.

Is it still relevant, and if so should it be bumped to 3.3.x or 4.0? Thanks!

Moving to 4.0, it is a maintenance PR for dev comfort (but it will be a good one to get...).

Instead of fighting with the OS fonts, another path could be to override the theme font (removing the system-ui one) for the UI tests.

How shall we proceed here? For me this PR makes testing a little easier. But I haven't managed to get the italics issue under control. So it would be interesting how it works for the other devs to choose which commits to keep.

Apologies that no one responded here @schmidi314. I would suggest to keep the part changing fonts but not the part relaxing threshold and merge this after resolving conflicts. If it helps even a bit then it's good to go - does not need to be perfect.

@fcollonval would your docker image PR supersede this PR?

@fcollonval would your docker image PR supersede this PR?

Yes it will

@fcollonval If #15916 supersedes this, and you're planning to merge this in during the next month, would you prefer to close this PR and tag #15916 with the 4.3.0 milestone?