Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

EVG-19948: Support EC2 On-Demand on provider settings page #2071

Merged
merged 6 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions cypress/integration/distroSettings/provider_section.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,4 +165,81 @@ describe("provider section", () => {
cy.contains("button", "Add region settings").should("exist");
});
});

describe("ec2 on-demand", () => {
beforeEach(() => {
cy.visit("/distro/ubuntu1604-parent/settings/provider");
});

it("shows and hides fields correctly", () => {
// VPC options.
cy.dataCy("use-vpc").should("be.checked");
cy.contains("Default VPC Subnet ID").should("exist");
cy.contains("VPC Subnet Prefix").should("exist");

cy.dataCy("use-vpc").uncheck({ force: true });
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's reliable to use force when making assertions on components related to the test description. Using force prevents testing if the use-vpc button is actually available to click. If use-vpc is disabled or hidden, this code will pass through. I think force usage is okay when setting the page up in a particular state to begin testing because those assertions are less important then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using force is pretty much required for LG checkboxes since they have a styling that makes them hidden and unclickable

cy.contains("Default VPC Subnet ID").should("not.exist");
cy.contains("VPC Subnet Prefix").should("not.exist");
Comment on lines +181 to +182
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to take advantage asserting visibility if showing/hiding is a theme of the test.

});

it("successfully updates ec2 on-demand provider fields", () => {
cy.dataCy("provider-select").contains("EC2 On Demand");

// Correct section is displayed.
cy.dataCy("ec2-on-demand-provider-settings").should("exist");
cy.dataCy("region-select").contains("us-east-1");

// Change field values.
cy.selectLGOption("Region", "us-west-1");
cy.getInputByLabel("EC2 AMI ID").as("amiInput");
cy.get("@amiInput").clear();
cy.get("@amiInput").type("ami-1234560");
cy.getInputByLabel("SSH Key Name").as("keyNameInput");
cy.get("@keyNameInput").clear();
cy.get("@keyNameInput").type("my ssh key");
cy.getInputByLabel("User Data").type("<powershell></powershell>");
cy.getInputByLabel("Merge with existing user data").check({
force: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably better to not use force here since this test is about form interaction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same comment as above

});
save();
cy.validateToast("success");
Copy link
Contributor

Choose a reason for hiding this comment

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

It's helpful to include the toast copy because it helps document the test and leaves a breadcrumb to the hook.


// Revert fields to original values.
cy.selectLGOption("Region", "us-east-1");
cy.get("@amiInput").clear();
cy.get("@amiInput").type("ami-0000");
cy.get("@keyNameInput").clear();
cy.get("@keyNameInput").type("mci");
cy.getInputByLabel("User Data").clear();
cy.getInputByLabel("Merge with existing user data").uncheck({
force: true,
});
save();
cy.validateToast("success");
});

it("can add and delete region settings", () => {
cy.dataCy("ec2-on-demand-provider-settings").should("exist");

// Add item for new region.
cy.contains("button", "Add region settings").click();
cy.contains("button", "Add region settings").should("not.exist");

// Save new region.
cy.selectLGOption("Region", "us-west-1");
cy.getInputByLabel("EC2 AMI ID").type("ami-1234");
cy.getInputByLabel("Instance Type").type("m5.xlarge");
cy.contains("button", "Add security group").click();
cy.getInputByLabel("Security Group ID").type("security-group-1234");
save();
cy.validateToast("success");

// Revert to original state by deleting the new region.
cy.dataCy("delete-item-button").first().click();
save();
cy.validateToast("success");

cy.contains("button", "Add region settings").should("exist");
});
});
});
21 changes: 16 additions & 5 deletions src/pages/distroSettings/tabs/ProviderTab/ProviderTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ export const ProviderTab: React.FC<TabProps> = ({ distro, distroData }) => {
AWS_REGIONS
);
const { awsRegions } = awsData || {};
const configuredRegions = formData?.ec2FleetProviderSettings?.map(
(p) => p.region
);

const { containerPools } = useSpruceConfig();
const { pools } = containerPools || {};
Expand All @@ -44,15 +41,29 @@ export const ProviderTab: React.FC<TabProps> = ({ distro, distroData }) => {
? JSON.stringify(omitTypename(selectedPool), null, 4)
: "";

const fleetRegionsInUse = formData?.ec2FleetProviderSettings?.map(
(p) => p.region
);
const onDemandRegionsInUse = formData?.ec2OnDemandProviderSettings?.map(
(p) => p.region
);

const formSchema = useMemo(
() =>
getFormSchema({
awsRegions: awsRegions || [],
configuredRegions: configuredRegions || [],
fleetRegionsInUse: fleetRegionsInUse || [],
onDemandRegionsInUse: onDemandRegionsInUse || [],
pools: pools || [],
poolMappingInfo,
}),
[awsRegions, configuredRegions, pools, poolMappingInfo]
[
awsRegions,
fleetRegionsInUse,
onDemandRegionsInUse,
pools,
poolMappingInfo,
]
);

return (
Expand Down
Loading