Skip to content

test: [M3-8454] - Cypress test for Secure VMs firewall generation#10802

Merged
hkhalil-akamai merged 5 commits intolinode:developfrom
hkhalil-akamai:M3-8454-secure-vm-tests
Sep 3, 2024
Merged

test: [M3-8454] - Cypress test for Secure VMs firewall generation#10802
hkhalil-akamai merged 5 commits intolinode:developfrom
hkhalil-akamai:M3-8454-secure-vm-tests

Conversation

@hkhalil-akamai
Copy link
Contributor

@hkhalil-akamai hkhalil-akamai commented Aug 19, 2024

Description 📝

Add Cypress tests to verify the flow to generate compliant firewalls in the Linode Create form.

This flow can also be initiated from the banners in FirewallLanding and FirewallDetails but since those are already covered by unit tests, and the component is shared, I decided to only write tests for generating firewalls through the Linode create flow.

Target release date 🗓️

9/3

How to test 🧪

  • Verify Cypress tests pass

As an Author I have considered 🤔

Check all that apply

  • 👀 Doing a self review
  • ❔ Our contribution guidelines
  • 🤏 Splitting feature into small PRs
  • ➕ Adding a changeset
  • 🧪 Providing/Improving test coverage
  • 🔐 Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • 👣 Providing comprehensive reproduction steps
  • 📑 Providing or updating our documentation
  • 🕛 Scheduling a pair reviewing session
  • 📱 Providing mobile support
  • ♿ Providing accessibility support

@hkhalil-akamai hkhalil-akamai added Work in Progress e2e Indicates that a PR touches Cypress tests in some way Secure VMs labels Aug 19, 2024
@hkhalil-akamai hkhalil-akamai self-assigned this Aug 19, 2024
@hkhalil-akamai hkhalil-akamai changed the title test: [M3-8454] - Secure VMs Integration Tests test: [M3-8454] - Cypress test for Secure VMs firewall generation Aug 21, 2024
Comment on lines 195 to 206
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This header is removed in a global interceptor, so we have to manually re-add it for this request.

Copy link
Contributor Author

@hkhalil-akamai hkhalil-akamai Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value of watchFirewall becomes null after clearing (create v2).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value of firewallId becomes -1 after clearing (create v1).

@hkhalil-akamai hkhalil-akamai marked this pull request as ready for review August 21, 2024 20:17
@hkhalil-akamai hkhalil-akamai requested review from a team as code owners August 21, 2024 20:17
@hkhalil-akamai hkhalil-akamai requested review from AzureLatte, cliu-akamai and harsh-akamai and removed request for a team August 21, 2024 20:17
Copy link
Contributor

@AzureLatte AzureLatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@hkhalil-akamai hkhalil-akamai force-pushed the M3-8454-secure-vm-tests branch from e9ead05 to 567d736 Compare August 26, 2024 21:07
@github-actions
Copy link

github-actions bot commented Aug 26, 2024

Coverage Report:
Base Coverage: 82.73%
Current Coverage: 82.73%

@hkhalil-akamai
Copy link
Contributor Author

@cliu-akamai @harsh-akamai this PR is ready for review whenever you are able. Thanks!


linodeCreatePage.setLabel(mockLinode.label);
linodeCreatePage.selectImage('Debian 11');
linodeCreatePage.selectRegionById(linodeRegion.id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
linodeCreatePage.selectRegionById(linodeRegion.id);
linodeCreatePage.selectRegionById(linodeRegion.label);

Not sure if this is applicable only for me, but all the tests seem to fail when I use id instead of label

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was encountering this issue too, but it only occurs when running tests locally with the Gecko v2 GA flag enabled.

getOptionLabel={(region) =>
isGeckoGAEnabled ? region.label : `${region.label} (${region.id})`
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may have to change all instances like this in the near future if GA becomes permanent and the flag is retired (cc @jdamore-linode)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @hkhalil-akamai, I can't run the tests locally right now because I'm running DC tests, but is this failing because Cypress is typing the region ID into a region select field and the desired region is not showing up? If so, I believe that's a Gecko bug (M3-8502) rather than an issue with the tests.

@mjac0bs mjac0bs added the Approved Multiple approvals and ready to merge! label Sep 3, 2024
@hkhalil-akamai hkhalil-akamai merged commit f471eb0 into linode:develop Sep 3, 2024
@hkhalil-akamai hkhalil-akamai deleted the M3-8454-secure-vm-tests branch September 3, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! e2e Indicates that a PR touches Cypress tests in some way Secure VMs

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants