Remove Cypress remaining tests/files

[viniciusdc]

Reference Issues or PRs

closes #2337

What does this implement/fix?

Put a x in the boxes that apply

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds a feature)
• Breaking change (fix or feature that would cause existing features not to work as expected)
• Documentation Update
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no API changes)
• Build related changes
• Other (please describe):

Testing

• Did you test the pull request locally?
• Did you add new tests?

Any other comments?

[viniciusdc]

This will only remove the related files of the cypress from the code base. There are some enhancements initially outlined in the previous comments of #2337 that will be addressed in a future PR

[dcmcand]

@viniciusdc is this one ready to go? Can you merge the latest develop into it?

[viniciusdc]

Thanks @dcmcand, I was waiting for a fix to be included in the playwright tests. That's addressed now, ready for review

[marcelovilla]

@viniciusdc wouldn't it be better to address the missing functionality tests in this PR so we don't lose them after merging this PR? I'm fine if you prefer to address that in a separate PR but since this is not a priority, I think it would make sense to add that here.

[viniciusdc]

I see your point, @marcelovilla, and I was also going in that direction but found a significant blocker: the current Notebook class for the playwright tests does not support the level of control I was expecting (even though the test itself is simple, I can't break it down in the way it's now).

So, it would take some time to refactor the Nebari executor class to give modularization enough to get this all working.

Thus, as this original issue was to remove Cypress itself, I considered that its out of scope based on the level of changes.

[marcelovilla]

@viniciusdc I see, thanks for clarifying that. And I agree it would be highly beneficial to refactor that class to allow easier incorporation of new tests. Feel free to merge this if you want!

[krassowski]

the current Notebook class for the playwright tests does not support the level of control I was expecting

FYI, galata is playwright wrapper for testing Jupyter https://github.com/jupyterlab/jupyterlab/blob/main/galata/README.md and comes with guarantees about API stability (if you upgrade JupyterLab version you just upgrade to new galata and your tests should still work).

[krassowski]

So this PR would remove:

• sanity check for conda-store (which admittedly failed recently [BUG] - conda-store UI is broken after upgrading conda-store to 2024.6.1 #2600)
• test for UI menu customizations
• test for custom jupyterlab overrides overrides.json

If it is to be merged as-is (without rewriting the these tests in playwright) I would feel less confident submitting patches in the future and would have to spend more time manually testing things unrelated to patches I would be submitting.

[viniciusdc]

FYI, galata is playwright wrapper for testing Jupyter jupyterlab/jupyterlab@main/galata/README.md and comes with guarantees about API stability (if you upgrade JupyterLab version you just upgrade to new galata and your tests should still work).

This looks interesting. Thanks for sharing @krassowski

[viniciusdc]

So this PR would remove:

• sanity check for conda-store (which admittedly failed recently [BUG] - conda-store UI is broken after upgrading conda-store to 2024.6.1 #2600)
• test for UI menu customizations
• test for custom jupyterlab overrides overrides.json

If it is to be merged as-is (without rewriting the these tests in playwright) I would feel less confident submitting patches in the future and would have to spend more time manually testing things unrelated to patches I would be submitting.

I can see the first item, but I don't understand the others... right now cypress only does what's in here https://github.com/nebari-dev/nebari/pull/2577/files#diff-dc76a5b211dc3fa7e5b704f7fdc28c48fce3d2aaeece83e480b057e28109ffd0

The rest of the tests that including spinning up a user instance, have already been moved to Playwright in here https://github.com/nebari-dev/nebari/tree/develop/tests/tests_e2e/playwright

I think @marcelovilla outlined that better in his comment here: #2337 (comment)

[marcelovilla]

I think @krassowski is referring to these checks:

nebari/tests/tests_e2e/cypress/e2e/main.cy.js

Lines 62 to 65 in f2f5a5b

	cy.get('div.jp-LauncherCard[title="VS Code [↗]"]').should('exist');
	// Should reflect theme set by default_settings
	cy.get('body[data-jp-theme-name="JupyterLab Dark"]').should('exist');

[viniciusdc]

gotcha thanks @marcelovilla , and sorry @krassowski when you said theme and patching I was thinking more in terms of the overlall nebari theme, I missed that one.

In overall I agree that we need those to be tested, I can include the ones above as part of the Notebook tests (if they are not there already), but my only comment was that we can't test URLs using the current playwright structure because on how these tests are executed right now by using classes. I am working on another PR, just that the conda-store stuff broken down first

[marcelovilla]

There's a lot of room for improvement in terms of actually testing that extensions work as expected (maybe user journeys rather than just visiting their URLs) and Jupyter customizations overall. Galata seems really useful, by the way. We don't have the best tests with Cypress, but at least we have some useful checks and those would be lost if we don't migrate them to Playwright.

I know our Playwright tests can use some refactoring to allow new tests and checks but maybe this PR can wait until we achieve that. The only benefit I see in removing the Cypress tests is having less dependencies and less things to maintain but we won't actually see an immediate value after removing them, besides maybe a couple minutes less for each integration tests run. On the other hand, we'll lose value by removing the checks.

[viniciusdc]

You are both right, that's said to me it looks like extending playwright should be integrated first and then merging this PR to remove the actual tests. I will open a new PR, adding the missing features outlined above, and once that's merged, we can merge this one.

[viniciusdc]

Playwright tests are now passing, though we might also need to address the issue with test_jupyterhub_ssh.py

[viniciusdc]

superseded by #2670