test: [M3-7955] - Cypress integration test to add SSH key via Linode Create #10448
Conversation
|
Coverage Report: ✅ |
jdamore-linode
changed the title
There was a problem hiding this comment.
Nice work @cliu-akamai! Left some comments for potential clean up, but nothing that impacts the quality or effectiveness of your test (which is great, by the way!)
You have my approval pending yarn changeset. Thanks Cassie!
| @@ -76,6 +76,7 @@ authenticate(); | |||
| describe('create linode', () => { | |||
| before(() => { | |||
| cleanUp('linodes'); | |||
| cleanUp('ssh-keys'); | |||
There was a problem hiding this comment.
| cleanUp('ssh-keys'); |
Very minor nit: This call to cleanUp('ssh-keys') isn't really necessary since your test mocks the SSH key (no real SSH key is being created, so nothing will need to be cleaned up). However, I really appreciate you setting up the deleteAllTestSSHKeys util and hooking it up to the cleanUp function since this will be needed for M3-7956!
Oops, never mind! Should have looked closer, realizing now that this test does actually create an SSH key! The ticket technically called for an integration test but I'm thinking leaving this as-is might be fine
| const sshPublicKey = `ssh-rsa e2etestkey${randomKey} e2etest@linode`; | ||
| const linodeLabel = randomLabel(); | ||
| const region: Region = getRegionById('us-southeast'); | ||
| const diskLabel: string = 'Debian 10 Disk'; |
There was a problem hiding this comment.
I think a lot of this mock data is held over from other tests, and we can get rid of them for this test just to make things a little cleaner! Locally, I was able to get rid of all the mock data and setup for disks, VLANs, VPCs & subnets, configs, and the mocks for the Linode DC-specific pricing types and your test still ran really nicely and passed!
Not a super big deal since your test is still testing all of the important things we need to cover for SSH keys 👍
Description 📝
Add integration test to add SSH key via Linode create.
Major Changes 🔄
How to test 🧪