test: [M3-6505] - Add StackScript Update/Delete E2E Tests

[cliu-akamai]

Description 📝

Add new E2E tests for StackScript

Major Changes 🔄

• Add update StackScript tests
• Add delete StackScript tests

How to test 🧪

yarn cy:run -s "cypress/e2e/core/stackscripts/*.spec.ts"

[cypress]

1 flaky tests on run #4807 ↗︎

0

181

3

0

1

Details:

Fixed comments
Project: Cloud Manager E2E	Commit: bcc09a2e88
Status: Passed	Duration: 15:06 💡
Started: Jul 7, 2023 5:47 PM	Ended: Jul 7, 2023 6:02 PM

  cypress/e2e/core/objectStorage/object-storage.smoke.spec.ts • 1 flaky test

View Output Video

Test		Artifacts
object storage smoke tests > can upload, view, and delete bucket objects - smoke		Output Screenshots Video

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[jdamore-linode]

Suggested change

	interceptGetStackScripts(stackScripts).as('getStackScripts');
	mockGetStackScripts(stackScripts).as('getStackScripts');

I think this may be the cause of the test failure!

[hkhalil-akamai]

All tests passing on my end, looks good! Great work!

[jdamore-linode]

Hey Cassie! Posted a couple suggestions, mostly surrounding the JSON.parse(JSON.stringify()) stuff, but also had one quick function rename suggestion. Let me know if you have any questions! Other than that everything is running nicely so ready to approve soon :)

[jdamore-linode]

Suggested change

	const updateStackScript = JSON.parse(JSON.stringify(stackScripts[1]));
	mockGetStackScripts([updateStackScript]).as('getUpdatedStackScripts');
	mockGetStackScripts([stackScripts[1]]).as('getUpdatedStackScripts');

We can eliminate the JSON.parse(JSON.stringify(...)) pretty easily for this one!

[jdamore-linode]

This one is tricky. I think I understand why we reached for JSON.parse(JSON.stringify(...)), because I think you may have been getting bitten by some combination of Cypress's command queueing behavior and JavaScript's ambiguous reference handling, so I want to explain what's going on here.

I think the first solution that would come to mind here, but which wouldn't actually work, would be something like this:

const updatedStackScript = stackScripts;
// Or maybe `const updatedStackScript = [...stackScripts];`...
updatedStackScripts[0].label = stackscriptLabel;
updatedStackScripts[0].description = stackscriptDesc;
// ...

Not only does this not work, it also causes a pretty strange behavior if you try running the test in the debugger: when Cypress loads the page initially, and Cloud makes a GET request to fetch the StackScripts from the API, the mocked response already has its label set to stackscriptLabel even though the update operation hasn't occurred yet, but our assertions (cy.findByText(), etc.) all still expect to find the original label.

When Cypress begins running a test, it executes all of the test code right away. All of the Cypress functions like cy.findByText(), etc., don't actually do anything right away, however. Instead, they add a command to Cypress's command queue. These commands begin running after the code itself has executed, and that's what you're seeing happen when you watch the debugger.

Meanwhile, when you call updatedStackScripts[0].label = '...' and updatedStackScripts[0].description = '...' on lines 162 and 163, you're actually modifying the original StackScript objects that you created back on line 82 via stackScriptFactory.buildList(2), because JavaScript passes objects as references.

To complicate matters, primitive values like strings are not passed as references. So when you queue a Cypress command to find one of the mocked StackScripts by its label, like on line 87, the Cypress command will be queued with the original label value. However, by the time that command is actually executed, the StackScript objects themselves which are used for the GET mock have already been modified (lines 162, 163) with the updated label, and so that's what appears on the screen and the Cypress command fails to find what it's looking for.

There are a couple ways to fix this, and JSON.parse(JSON.stringify(...)) technically works because it's creating a clone of the objects and circumventing all of this reference confusion. However, I think the easiest and clearest solution is to simply set up all of your mock data (or as much as you can) at the very beginning of the test. So that would look something like this:

    const stackscriptLabel = randomLabel();
    const stackscriptDesc = randomPhrase();
    const stackscriptImage = 'Alpine 3.17';

    const stackScripts = stackScriptFactory.buildList(2);
    // Import StackScript type from Linode API package.
    const updatedStackScripts: StackScript[] = [
      // Spread operator clones an object...
      {...stackScripts[0]},
      {
        ...stackScripts[1],
        label: stackscriptLabel,
        description: stackscriptDesc,
      }
    ];

[jdamore-linode]

Suggested change

	const updatedStackScript = JSON.parse(JSON.stringify(stackScripts[0]));
	updatedStackScript.is_public = true;
	const updatedStackScript = {...stackScripts[0]}
	updatedStackScript.is_public = true;

This is caused by the same issue I described above, and can also be solved similarly by defining the updatedStackScript object as a clone of stackScripts[0] (which is essentially what JSON.parse(JSON.stringify()) is doing anyhow, but in a more roundabout way).

(I'd still suggest moving all of this type of stuff to the top of the test, just for the sake of readability)

[jdamore-linode]

Suggested change

	export const interceptGetStackScript = (
	export const mockGetStackScript = (

(And in update-stackscript.spec.ts too)

[jdamore-linode]

Nice work @cliu-akamai !