kumahq / kuma-gui

🐻 A GUI built on Vue.js for use with Kuma.
https://kuma.io/
Apache License 2.0
39 stars 22 forks source link

tests: create a double assertion non-existence step #2350

Open johncowen opened 7 months ago

johncowen commented 7 months ago

Description

We have a e2e assertion step that checks for the non-existence of things. But this can very easily be accidentally pass if things haven't loaded yet, then pass and then the thing we are trying to assert never exists, then appears once the load has finished.

We should make a step that firstly asserts for the existence of something that you know will not be there until you can properly assert that something else doesn't exist.

Maybe something like:

And the "$something" element exists but the "$somethingelse" element doesn't

We may then be able to remove the ability to assert for something without checking for an existing element first. Not totally sure right now whether we'd want to do that tho. We can see once we have this new step.

github-actions[bot] commented 4 months ago

This issue was inactive for 90 days. It will be reviewed in the next triage meeting and might be closed. If you think this issue is still relevant, please comment on it or attend the next triage meeting.

github-actions[bot] commented 1 month ago

This issue was inactive for 90 days. It will be reviewed in the next triage meeting and might be closed. If you think this issue is still relevant, please comment on it or attend the next triage meeting.

slonka commented 1 month ago

Could you maybe link to the code where this happens?

johncowen commented 1 month ago

Here's a place where theoretically the element we assert doesn't exist might then exist a split second after we've asserted:

https://github.com/kumahq/kuma-gui/blob/d3a8553c58587d47f9a3a44bf7e5f8bc1db7f37d/features/application/Warnings.feature#L16-L22

Here's one of the may places where we assert for existence of something that should exist first before asserting that something else doesn't exist:

https://github.com/kumahq/kuma-gui/blob/d3a8553c58587d47f9a3a44bf7e5f8bc1db7f37d/features/mesh/services/Item.feature#L15-L23

johncowen commented 2 weeks ago

I just came across a place in our tests where there is a test that passes that really shouldn't be because we don't have this double assertion step:

https://github.com/kumahq/kuma-gui/blob/e836d926f8ab0d60b9602b729ebbde5dbda99ca7/features/zones/zone-cps/Warnings.feature#L67-L80

The above tests mocks out /config to mock the storeType, which we no longer request 😱 , so the test should break, but it doesn't. I'm pretty sure it doesn't because the "check for none existence of a notification" assertion passes before the page loads up.

Here's a grab of the passing test clearly showing the notification we don't want to see.

Screenshot 2024-10-28 at 17 48 50

Just to note, the application functionality works, just our test is doing absolutely nothing because it has a bad assertion and we mock things out incorrectly.

I caught this whilst working on something else, and I'll probably use this comment in the future as a argument to say the approach I am working on is better, as this new approach failed these tests correctly. Cypress happily lets them pass.