Repository navigation
test: [various] - Cypress test cleanup and flake fixes - #10405
Conversation
…ateAndBootLinode`
|
Coverage Report: ✅ |
bnussman-akamai
left a comment
bnussman-akamai
left a comment
There was a problem hiding this comment.
Very nice 🔥 Thanks for getting all of these things addressed
Left one question, but it doesn't need to hold up merging
| // Fetch Linode kernel data from the API. | ||
| // We'll use this data in the tests to confirm that config labels are rendered correctly. | ||
| cy.defer(fetchAllKernels(), 'Fetching Linode kernels...').then( | ||
| (fetchedKernels) => { | ||
| kernels = fetchedKernels; | ||
| } | ||
| ); | ||
| }); |
There was a problem hiding this comment.
I'm cool with this, but are there other ways to do it?
For example, could we intercept the kernels response that Cloud Manager makes to avoid needing to defer here?
There was a problem hiding this comment.
Great question! Intercepting the request and looking at the response was my first approach, and it technically worked, but I didn't really like it because of these points:
- We don't get any type safety on the response object, so we have to explicitly check and guard against
undefinedvalues, etc.. The end result was I had all these conditionals in the middle of the tests checking the response data andthrowing if they didn't have the right shape, and it was just kind of ugly. We do do this in other places throughout the tests, though.- Long term I'd like to come up with a command that can intercept requests and validate the response so we don't have to do it manually every time, but I'm not 100% certain it's possible (haven't tried yet), and it doesn't address the next point...
- This is a paginated response, so it only works when we're certain that the data we want is included in the response that we're intercepting. The
kernelsendpoint responds with fewer than 500 items so, for now,linode/latest-64bitis guaranteed to be in the response, but that might not be the case as more and more items get added to the response (there are 328 items right now). Things start getting really tricky when we start trying to intercept multiple requests.
It is a bummer to have to fire off more API requests in the tests so definitely happy to keep discussing. I'd love a better approach and I bet we could figure one out!
There was a problem hiding this comment.
That all makes sense! Thanks for the detailed explanation 💯
mjac0bs
left a comment
mjac0bs
left a comment
There was a problem hiding this comment.
Thank you for all this clean up and the corresponding explanations! 🧼
CI looks good; I read through the PR and ran the linode-config test locally to watch it. (That form has a lot of possible situations resulting in warning notices to display, and I don't think we're testing all of them, currently, so I might add to it at the end of M3-7977.)
There was a problem hiding this comment.
Thank you! I had wondered about these.
3 participants
Description 📝
Cleans up and fixes various tests that have been especially flaky lately.
Changes 🔄
List any change relevant to the reviewer.
How to test 🧪
We can rely on CI.