Skip to content

test: [M3-7768] - Add VM Placement Group landing page empty state test - #10350

Merged
abailly-akamai merged 6 commits into
linode:developfrom
jdamore-linode:M3-7768-vm-placement-landing-page-empty
Apr 11, 2024
Merged

abailly-akamai merged 6 commits into
linode:developfrom
jdamore-linode:M3-7768-vm-placement-landing-page-empty

Conversation

@jdamore-linode

Copy link
Copy Markdown
Contributor

Description 📝

Adds a quick integration test to confirm the VM Placement Groups landing page empty state. The real purpose of this PR is to provide a bare minimum example showing how to mock one of our new object-based feature flags, but it's not much different from mocking our boolean flags.

Changes 🔄

  • Add integration test to confirm VMPG landing page empty state
  • Adds a mock util for Placement Group fetching

How to test 🧪

yarn cy:run -s "cypress/e2e/core/vmPlacement/vm-placement-landing-page.spec.ts"

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

@jdamore-linode jdamore-linode self-assigned this Apr 3, 2024
@jdamore-linode
jdamore-linode requested a review from a team as a code owner April 3, 2024 21:49
@jdamore-linode
jdamore-linode requested review from cliu-akamai and removed request for a team April 3, 2024 21:49
Comment on lines 12 to 18

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The only real difference here is we're passing an object in the same shape as the feature flag rather than a true or false (however, there's no type safety here that'll catch us if we make a mistake).

@jdamore-linode
jdamore-linode requested a review from a team as a code owner April 3, 2024 21:55
@jdamore-linode
jdamore-linode requested review from bnussman-akamai and jaalah-akamai and removed request for a team April 3, 2024 21:55
@github-actions

github-actions Bot commented Apr 3, 2024 •

Copy link
Copy Markdown

Coverage Report: ✅
Base Coverage: 81.79%
Current Coverage: 81.79%

@abailly-akamai abailly-akamai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great! thanks for making this one, looking forward to writing more

@abailly-akamai abailly-akamai Apr 4, 2024 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jdamore-linode since it's a generic we can add type safety here. We'll just have to remember to do it and catch it at code reviews if not present.

Pushed a fix for this one

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Beautiful, wasn't aware of that Flags type. Thanks @abailly-akamai!

By the way, do you know anything about the vmPlacement LD flag? Was that a holdover from earlier in the project that isn't going to be used?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Correct, lemme delete it now so there's no confusion anymore

@carrillo-erik

carrillo-erik commented Apr 4, 2024 •

Copy link
Copy Markdown
Contributor

@jdamore-linode Looks good and thanks for getting this started. My only reservation is whether the folder and spect should be renamed to placementGroups/ and placement-group-landing-page.spec.ts. This change would reflect the feature and component names in the rest of the codebase.

@jdamore-linode

Copy link
Copy Markdown
Contributor Author

My only reservation is whether the folder and spect should be renamed to placementGroups/ and placement-group-landing-page.spec.ts. This change would reflect the feature and component names in the rest of the codebase.

Great callout, thanks @carrillo-erik! If that's more consistent and makes more sense then I absolutely agree. I'll try to take care of that before I sign out tonight!

@mjac0bs mjac0bs added Approved Multiple approvals and ready to merge! Placement Groups and removed Ready for Review labels Apr 8, 2024
@abailly-akamai
abailly-akamai force-pushed the M3-7768-vm-placement-landing-page-empty branch from 1e5690f to 49d2653 Compare April 11, 2024 18:55
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! Placement Groups

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants