Harden Cypress E2E test against environment variability + document Cypress test best practices

Can we also add some documentation on how to launch a debugger within a cypress test, it was a zero-config approach for Vitest but I can't get it to work with Cypress

Moved @larowlan's remark in #2 into the issue summary.

@jessebaker should this also handle

Throwing in a requirement that I think all Cypress tests should be written using the principals/api outlined by https://testing-library.com/docs/cypress-testing-library/intro/ for all the reasons given on the Testing Library website.

— written by you at #3452585-9: Cypress test infrastructure clean-up + baseline E2E test → ?

Can we enforce that using eslint?

I've done a little bit of digging this morning. It doesn't seem like there is a cypress-testing-library eslint ruleset out there - the eslint-plugin-testing-library that exists doesn't support cypress-testing-library.

1. "eslint-plugin-testing-library doesn't support cypress-testing-library (there is not much to report from it actually)." https://github.com/testing-library/eslint-plugin-testing-library/issues/...
2. "There is nothing to cover in Cypress Testing Library. Actually, this plugin must be excluded from Cypress tests." https://github.com/testing-library/eslint-plugin-testing-library/issues/80

There is an official set of Cypress eslint ruleseslint-plugin-cypress https://github.com/cypress-io/eslint-plugin-cypress - however I'm fairly sure most will be too restrictive.

Of the listed rules that are offered in that package they recommend 4 - I think we could perhaps use 3 out of the 4 they recommend

• no-assigning-return-values disallow assigning return values of cy calls
• no-async-tests disallow using async/await in Cypress test cases
• unsafe-to-chain-command disallow actions within chains

I think we will need to allow this one though, because in my experience there is always some need to wait ESPECIALLY when dealing with iFrames and testing things within them. Perhaps we could flag this rule but not make it mandatory?

• no-unnecessary-waiting disallow waiting for arbitrary time periods

Let's get the proposal in #6 reviewed by the Cypress expert on the team :)

no-unnecessary-waiting is fine, Cypress' retry-ability is built into every query+assertion which pretty much eliminates the need to explicitly add waits at all, even for elements - it's essentially built into the framework already. If the default retry-ability time is not sufficient for a specific query, it can be adjusted on a per-query basis.

Great!

But does that mean you are +1 to the other 3 rules @jessebaker proposed?

But does that mean you are +1 to the other 3 rules @jessebaker proposed?

Good to officially state! Yes I'm good with the other rules too.

"no-unnecessary-waiting is fine"

I'm not convinced this won't prove to be a pain point later on. With all the best intent in the world, sometimes (and again especially when dealing with iFrames) you just need to wait(100ms) because for whatever reason the document loading event is firing out of time or there is a re-render occuring after the initial render and you just need to ensure that it has happened so you don't get stale references to removed DOM elements.

Maybe we should include it as a warning and accept that we may have to allow the 1 line "skip this rule" comment when we need to use it?

Maybe we should include it as a warning and accept that we may have to allow the 1 line "skip this rule" comment when we need to use it?

Yep, if it is needed then that instance of it can be skipped and should be accompanied by a specifics-laden (as in something more substantial than "it the only way":upside_down_face: ) comment explaining why.

📌 End-to-end test that tests both the client (UI) and server Active is in, so now let's get this done ASAP too, to make writing/expanding Cypress tests as productive as possible 🙏

#11 + #12 + #13 sound like consensus has been reached. 👍

Let's now enforce that using eslint!

~1 month later, and there's many more Cypress tests now — yay! They've already saved us a TON of time!

But now we have a new problem, one we're all too familiar with from the Drupal core space 😅:

Cypress tests are currently too brittle, probably due to the variability of the GitLab CI runners that drupal.org operates 😬

1. First CI run: a11y.cy.js failed: https://git.drupalcode.org/project/experience_builder/-/jobs/2391963
2. Second CI run: undo-redo.cy.js failed: https://git.drupalcode.org/project/experience_builder/-/jobs/2392056
3. Third CI run: xb-general.cy.js failed: https://git.drupalcode.org/project/experience_builder/-/jobs/2392110
4. Fourth CI run: success at least: https://git.drupalcode.org/project/experience_builder/-/jobs/2392137

#3465105-14: Follow-up for #3459235: make server not redirect when requesting a client-side-only route →

Let's stabilize those tests by making them more resilient against environment variability, and then document those best practices.

Also, AFAICT

cy.intercept('**').as('all')

cy.wait('@all').its('response.statusCode').should('be.oneOf', [200,300])

— https://stackoverflow.com/a/66472116

would save a lot of headaches? 😅

Two out of the three fails mentioned in #15 are failing due to application errors, not anything that can be mitigated by changing the test. I think it would be reasonable to include fixing that underlying instability in this issue. First commit won't have anything for this but I'll have something shortly.

The other failure appears to be due to a click inside a component not making it to the event listener that would trigger the process of opening the contextual form. The request to get the form contents from the back end is never made, so network wait would not address this specific problem. One thing I do notice is that the element missing the click is not in the viewport. While this isn't necessary for Cypress (clearly this is passing the majority of the time) I wonder if out-of-viewport + iframe is resulting in occasional bad coordinates. The first commit in the MR is changing the elements in the test to be in-viewport ones and lets see if that helps.

due to application errors

What do you mean? Server responses? A race condition bug in the client/UI?

(All 4 test runs are for the same commit.)

I wonder if out-of-viewport + iframe is resulting in occasional bad coordinates.

Ah yes, this seems plausible! We encountered similar functional JS test edge cases in Drupal core's test suite.

@bnjmnm is working on this, reflecting that here :)

Already loving what I'm seeing! 🤩👏

What do you

mean? Server responses? A race condition bug in the client/UI?

Race condition - it is looking for offsetHeight on iframe contents and occasionally that property does not yet exist. By having it handle that gracefully, it stops the error and the code simply executes again when the contents are ready & with the expected offsetHeight, this additional execution also happens more quickly than any human could detect.

The MR is-undrafted and adds

• the graceful offsetHeight mentioned aboveabove
• handles the uuid tracking the the router test more effectively - I should have remembered that the original approach is a Cypress DO NOT DO THIS.
• Installed and set upcypress-terminal-report which provides richer console output when a Cypress test fails

That makes sense, thanks!

Review

This looks like a very solid improvement!

I'm missing two things currently:

1. documented best practices — but perhaps that is just a combination of A) linking to https://docs.cypress.io/guides/core-concepts/introduction-to-cypress, B) pointing to xb-general.cy.js?
2. an answer to → but perhaps the answer to that is simply https://docs.cypress.io/api/commands/debug#__docusaurus_skipToContent_fa...?

Assigning to @jessebaker for final review.

@jessebaker: if my guessed solutions for those two missing things make sense to you, feel free to add them and merge this MR? 😄

RE #24

1. I've added a link to the Cypress intro and also the Testing Library Guiding principals page to the Readme.
2. I'm not sure I fully understand the question. The intention with Cypress is to use GUI if you are writing or debugging tests. I'm not sure what functionality you are hoping for but once you have the Cypress GUI loaded you have full access to the browser debugging tools, console etc. @TravisCarden is working on a 0 config DDEV environment for running the Cypress tests in https://www.drupal.org/project/experience_builder/issues/3458369 📌 DDEV support for Cypress tests Fixed

Considering how many MRs are negatively affected by flakey tests (in that we have to re-run tests due to timing problems like @bnjmnm described in #23), like we've been doing for a few days now (see #15), I'm going ahead and merging this to reduce false negatives (ephemeral pipeline failures).

I think https://docs.cypress.io/api/commands/debug is easy enough to discover from https://docs.cypress.io/guides/core-concepts/introduction-to-cypress (which we're now linking): type "debug" in the search box and you'll end up on that very page 👍

@jessebaker: this needs your final sign-off to merge. 🙏

Automatically closed - issue fixed for 2 weeks with no activity.