Attempt My Gameboards VRT fix

[jacbn]

There are two fixes here that work together and do seem to be working consistently. There are 5 successful consecutive runs here.

First, this wraps the mountWithStoreAndRouter command with a cy.window().then(window => ...);. This ensures a global window is necessarily defined before createBrowserRouter gets called; this was causing the Cannot read properties of null (reading 'history') error we love. The reason the window was not defined in the first place is slightly more tricky, and indeed this fix alone does not solve the issue.

If we fix the window issue, the next error seen (from this run) helps find the root cause – this reads queue.insert must be called with a valid index - the index (1) is out of bounds. Similar stack traces occur when Cypress commands occur outside of the standard test lifecycle order. In this case, my presumption is that:

• Cypress mounts the first My Gameboards test component, a <MyGameboards user={mockUser} /> wrapped in a store <Provider /> and a <RouterProvider />;
• This test completes fine;
• Cypress unmounts this component.
• Cypress mounts the next My Gameboards test component, again the exact same thing;
• Since the underlying React has not rendered in this time, from its perspective the same component is still there so all the state on this component is reused;
• Somewhere inside this state is a reference to something from Cypress' runner – I can't tell exactly what, but it belongs to the old test and so is stale;
• Attempting to queue up operations here inserts the operation into the wrong queue, hence this error.

The fix is therefore to ensure that the component passed into the tests cannot be recognised as the same by React. We solve this with keys; my first attempt at doing this manually did not seem to work, but the mount function takes an explicit rerenderKey that seems to be for precisely this purpose, so I'm using a random string unique to a given test.

These two things together seem to fix things!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.55%. Comparing base (ce6a80e) to head (d422b7b).
⚠️ Report is 29 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2346      +/-   ##
==========================================
- Coverage   43.57%   43.55%   -0.03%     
==========================================
  Files         601      601              
  Lines       25811    25856      +45     
  Branches     8610     8588      -22     
==========================================
+ Hits        11248    11262      +14     
- Misses      14506    14537      +31     
  Partials       57       57              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[barna-isaac]

I've gotten as far as your first fix, but gave up when I saw the second problem! (Here's the PR I ultimately gave up on: #2300). Very excited to see this potentially getting fixed!

[barna-isaac]

I'd by lying if I said I understood everything that's happening, but if I correctly understood you:

• you're now waiting for the window object to become available, and explicitly passing this to createBrowserRouter
• you're now passing a unique uuid to the mount command, so react doesn't treat our components as the same. This is because you suspect some react state is seeping through these test cases.

The first change is specific to createBrowserRouter, and the second is about isolating our test cases. I'd just like to note here for our later reference that even with the (hopefully) improved isolation here, we have a lot of state bleeding through these test cases. For an example, I've worked up this proof to show that they use the same store:

import React from "react";
import { selectors, sidebarSlice, useAppDispatch, useAppSelector } from "../../app/state";


const SetStore = () => {
   const isSidebarOpen = useAppSelector(selectors.sidebar.open);
   const dispatch = useAppDispatch();
   dispatch(sidebarSlice.actions.setOpen(true));

   return <p data-testId="testPar">The sidebar is {isSidebarOpen ? "open" : "closed"}</p>;
};

const ReadStore = () => {
   const isSidebarOpen = useAppSelector(selectors.sidebar.open);
   return <p data-testId="testPar">The sidebar is {isSidebarOpen ? "open" : "closed"}</p>;
};

it('sets something on the store', () => {
   cy.mountWithStoreAndRouter(<SetStore />);    
   cy.get('[data-testId=testPar]').should('have.text', "The sidebar is open");
});

it('then reads that from the store', () => {
   cy.mountWithStoreAndRouter(<ReadStore />);
   cy.get('[data-testId=testPar]').should('have.text', "The sidebar is open");
});

[jacbn]

Yes, indeed – you can see this quite clearly from commands.tsx; store is at no point reset, and is imported via import {store} from "../../src/app/state"; at the top – outside any individual test environment. This essentially guarantees a shared store across all the tests. It does seem likely that we would want a separate store between tests, but it isn't breaking anything at the moment.