chore(ci): review of Cypress steps

[johncowen]

We've recently had queries around our test approach and as to whether it is the root of several issues we are seeing lately.

Despite the fact that we are seeing these issues totally outside of Cypress testing, I wanted to do the due diligence, listen to this opinion and do anything I personally could do to act on it. So I spent some time reviewing our Cypress steps specifically looking for any potential issues.

---

One good thing about our declarative (and purposefully restrictive) approach is that I didn't have to review all our tests to look for potential issues. As long as our ~20 or so steps are good then we know its highly likely that all our tests are also.

---

I went through each step and checked that any steps that contained loops are actually waiting on each iteration by adding a cy.wait(1000) within each iteration and then verifying that was waiting whilst the test was running.

Most of these steps are one liners, but I also looked for any potentially problematic asynchronous code.

When doing this, I found one single place where I saw something that I decided to change:

In our When I visit the {string} URL step (an important step), I found that the check for [data-testid-root="mesh-app"] being visible (i.e. is our application there?) will execute before we mock the environment using the information from our Given the environment step.

Ideally we should ensure that the environment is mocked before checking for existence of our application.

Note: the When I visit the {string} URL will not move on until both the environment is mocked and the application is visible, not matter in which order they occur. Just these two checks can be in the order: "is the application visible?", then "mock the environment". Ideally we should always mock the environment first, then check that the application is visible.

Importantly, whether the environment is mocked or not does not affect whether the application will be visible or not (and subsequently timeout if not visible). But still, as I found it I wanted to amend it even though its a bit of a nit.

I also checked where we are seeing these issues, and we never mock out these environment variables in the tests over there, so this change here changes nothing over there.

In reviewing and tracing through certain codepaths, I also added a comment to make it more obvious that cy.intercept is being used. Longer term I'm pretty certain we'll end up moving the call to cy.intercept here in place instead of its call site being elsewhere.

I could not find anywhere else that in my opinion could be problematic.

If anyone has further questions or wants more clarification here please let me know.

Lastly, I may not merge this for quite some time (I don't want to change too many related things at once i.e. science), but in the same respect I wanted to get something up for visibility and approval.

[netlify]

✅ Deploy Preview for kuma-gui ready!

Name	Link
🔨 Latest commit	c9386bf
🔍 Latest deploy log	https://app.netlify.com/sites/kuma-gui/deploys/67e68446d417d30008049a35
😎 Deploy Preview	https://deploy-preview-3714--kuma-gui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[schogges]

Nice finding 👍
I'll approve this already, so you can decide when to merge it.