Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

e2e test that creates GKE cluster with default settings #12934

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

IsaSih
Copy link

@IsaSih IsaSih commented Dec 23, 2024

Summary

Occurred changes and/or fixed issues

This PR addresses the issue rancher/qa-tasks#1571.
This is a single e2e test that creates a GKE cluster with the default settings and verifies that the cluster is created and that the GKE default version and zone are displayed in the UI elements as expected. To support the test, were created GKE-related page objects for the GKE Create cloud credentials page, the GKE authenticate page with Project ID, and the Create GKE Cluster Page.

Some methods and variables were created but are not being used in this single test. Still, they will be used for the upcoming tests.

Storing the GKE Cloud credentials will be addressed in another PR.

Areas or cases that should be tested

Create a GKE Cluster with default settings
Other cluster provisioning-related tests
Other cloud credentials creation related tests
Other pages that contain LabeledInput or LabeledSelect elements

Areas which could experience regressions

other cluster provisioning tests

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

Signed-off-by: Isabela Guimaraes <isabela.guimaraes@suse.com>
@IsaSih IsaSih added this to the v2.11.0 milestone Dec 23, 2024
@IsaSih IsaSih requested a review from yonasberhe23 December 23, 2024 12:46
@IsaSih IsaSih self-assigned this Dec 23, 2024
cypress/e2e/po/components/labeled-select.po.ts Outdated Show resolved Hide resolved
@@ -34,4 +34,8 @@ export default class BaseCloudCredentialsPo extends PagePo {
saveButton() {
return new AsyncButtonPo('[data-testid="form-save"]', this.self());
}

authenticateButton() {
Copy link
Member

Choose a reason for hiding this comment

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

to confirm this button exists on multiple credentials pages?

Copy link
Author

@IsaSih IsaSih Jan 2, 2025

Choose a reason for hiding this comment

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

Not as far as I know. at least not for the existing test coverage nor in the default providers available in the cluster provisioning menu. Do you want me then to move it to the cloud credential gke page object?

Copy link
Member

Choose a reason for hiding this comment

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

Yes please!

<div class="col span-6">
<div
class="col span-6"
data-testid="name-ns-description-name"
Copy link
Member

Choose a reason for hiding this comment

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

name-ns-description is a component which has this testid, it's best to give this one and below unique names

Copy link
Member

Choose a reason for hiding this comment

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

though both could be avoided by using a selector something like this [data-testid="crugke-form"] .labeled-input:nth-of-type(x)

Copy link
Author

Choose a reason for hiding this comment

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

I prefer to keep this selector, as I'm utilizing the functions of this component in my tests (lines 62, 91, and 93), and these selectors are used across other tests that validate provisioning clusters with different cloud providers to reference the same fields: cluster name and cluster description. I believe this avoids duplication of code.

Copy link
Member

Choose a reason for hiding this comment

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

The shell/components/form/NameNsDescription.vue component contains the unique testid of name-ns-description-name already. It's used specifically to find the name label select within that NameNsDescription component.

In pkg/gke/components/CruGKE.vue though it doesn't use the NameNsDescription component, so testid's should not copy the already taken id's from the NameNsDescription component and e2e tests should not use the cypress/e2e/po/components/name-ns-description.po.ts PO.

if (item.googlecredentialConfig) {
const id = item.id;

cy.deleteRancherResource('v3', 'cloudcredentials', id);
Copy link
Member

Choose a reason for hiding this comment

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

This will delete all gke credentials on the target rancher, without knowing it was there developers could lose them.

Can the test be updated to be agnostic of existing credentials?

Copy link
Author

Choose a reason for hiding this comment

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

I could make this test agnostic to existing credentials, but the current cluster provisioning tests for the other cloud providers perform the same cleanup step.
cluster-provisioning-amazon-ec2-rke1.spec.ts and cluster-provisioning-amazon-ec2-rke2.spec.ts perform it before the tests whereas cluster-provisioning-azure-rke1.spec.ts and cluster-provisioning-azure-rke2.spec.ts perform it after the tests. My suggestion is to keep the cleanup step.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like the other tests have a large message at the top of the file warning of the consequences of running the test. Lo-bar solution is to ensure that's added here too.

Note - the cleanup in all the referenced files only removes the cloud cred that the test created, rather than the ideal step of returning the environment back to how it was before the tests run.

Signed-off-by: Isabela Guimaraes <isabela.guimaraes@suse.com>
@IsaSih IsaSih requested a review from richard-cox January 2, 2025 05:33
cy.intercept('GET', '/v1/management.cattle.io.users?exclude=metadata.managedFields').as('pageLoad');
cloudCredForm.saveCreateForm().cruResource().saveAndWaitForRequests('POST', '/v3/cloudcredentials').then((req) => {
expect(req.response?.statusCode).to.equal(201);
cloudcredentialId = req.response?.body.id.replace(':', '%3A');
Copy link
Member

@richard-cox richard-cox Jan 2, 2025

Choose a reason for hiding this comment

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

This should be removed post test

Edit: the cloud cred should be deleted post test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[UI] Cluster Provisioning - Provision Hosted Cluster - GKE
2 participants