Attempt to fix failing tests in current main

[GlobalMin]

THIS IS SOMEWHERE BETWEEN A DRAFT AND A NORMAL PR

Note: This pull request is created to address the current integration test failure on the main branch. I'm fairly sure that the edits here do fix it but I can't verify fully until someone reruns the checks for the updates to the original PR. I will modify this note if that happens and it works as I intended.

---

---

Hi EB team. 👋

What is the issue

The current main branch is failing the UI-integration tests which leads to noticeable ❌ as the status of passing required checks. This is likely not an issue for the core functionality but it's always nice to be able to keep main ✅ , so here's my attempt to help with that.

Reproducing/context

When I clone the latest main branch which comes PR #147 and I run the UI tests locally with this flow taken from the README in the ui-tests folder I get a failing test.

cd ./ui-tests
jlpm install
jlpm playwright install

jlpm playwright test -u

I get this error:

      26 |     const nbPanel = await page.notebook.getNotebookInPanel();
      27 |
    > 28 |     expect(await nbPanel!.screenshot()).toMatchSnapshot();
         |                                         ^
      29 |   });
      30 | });
      31 |

which looks to be the exact line and issue with the failing test right now. Nice.

Fix (maybe)

Simply changing that to jlpm playwright test --update-snapshots locally fixes this and the tests pass but from my reading around this repo I think the way to handle it within a Github PR is to use the comment please update playwright snapshots and then rerunning the tests. I was able to do this successfully on a forked repo so I'm hopeful this works.

[github-actions]

👈 Launch a Binder on branch GlobalMin/jupyterlab-myst/main

[GlobalMin]

Hi 👋 @rowanc1 @agoose77.

Could one of you approve the blocked workflows so I can verify the modification I made is fixing a failed test?

[GlobalMin]

This is the exact scenario I outlined here: #154 . I think there are other key checks that matter for this PR but the failure is something I can't control or prevent unless I'm missing something.

[GlobalMin]

please update playwright snapshots

[rowanc1]

Looks like the placement of the JupyterLab alert changed! Thanks for the help!

Could you delete the darwin screenshot? My thinking so far on that is that we should only keep the snapshots that are created on CI tracked in the git repo.

[GlobalMin]

Looks like the placement of the JupyterLab alert changed! Thanks for the help!

Could you delete the darwin screenshot? My thinking so far on that is that we should only keep the snapshots that are created on CI tracked in the git repo.

Sure, happy to make an effort to learn and adopt the development guidelines and I agree that your comment is the best way to go to avoid any weird local machine glitches creating more problems if they get pushed. So I would just remove the darwin addition which is really just reverting to the current main right? If so, then...

I'm trying to decide the cleanest way to handle this between:

• Continue with this PR - for simplicity and time the proper fixes can be continued here and either the roundabout commits are fine or we can clean them up.
• Close this PR and build off of current main - My current understanding of how to fix this is actually no code changes need to happen. It just needs an empty commit to create a PR, then adding please update playwright snapshots as a comment should update and fix.

I would choose the 2nd idea. Do you agree?

[rowanc1]

I would say continue with this PR is the easiest!

We will squash the history when we merge, such that there is only a single commit to the main branch.

[rowanc1]

please update playwright snapshots

[welcome]

Congrats on your first merged pull request in this project! 🎉


Thank you for contributing, we are very proud of you! ❤️

[rowanc1]

Thanks for your help with this @GlobalMin!

[GlobalMin]

@rowanc1

I am excited to contribute something to this project! 💻 I plan on trying to help with low hanging fruit like this for the next few months at least while I'm on sabbatical. Really love the mission of EB and I'll get to stretch my skills as well.

Squash syntax question

I spent way too long trying to do the squashing from my fork and got myself into some infinite loop. Just for my education, did you use this syntax just now to squash?

git reset --soft HEAD~2

or git merge --squash?

[rowanc1]

There is a button in the github UI:

I actually have no idea how to squash via the command line. :)

Thanks again for your help, help on docs and READMEs and tests are very helpful! Excited to have you on board!