E2e test deregistration cascading #1634

rtorrero · 2023-07-14T13:41:04Z

Description

This PR adds tests to confirm that a deregistration of a host also removes the affected data from other views. Included in this PR are tests for:

clusters overview: a cluster should be removed when all its hosts are removed
databases overview: deregistering a primary instance should remove the database
hana cluster details: linking to a unregistered host should be disabled
hana db details: a deregistered host should not be present in the list of hosts
sap system details: a deregistered host should not be present in the list of hosts
sap system overview: deregistering the primary instance of the db should remove the sap system

arbulu89

@rtorrero The tests look good in general.
I think that we should apply the deregisterHost command in the test itself, and not in the before, as the deregistration is the most important part of the test itself, and not a "setup` thing.

arbulu89 · 2023-07-19T11:51:42Z

test/e2e/cypress/support/commands.js

+    Cypress.env('web_api_port'),
+  ];
+
+  const headers = {


We could have this headers in a default constant.
Maybe food for a tech debt

arbulu89 · 2023-07-19T11:54:25Z

test/e2e/cypress/e2e/clusters_overview.cy.js

+      ],
+    };
+
+    before(() => {


Why this in a before?
This is the most important part of the test and it should be done in the test itself, right?

arbulu89 · 2023-07-19T11:56:17Z

test/e2e/cypress/e2e/clusters_overview.cy.js

+      hosts: [
+        {
+          id: '13e8c25c-3180-5a9a-95c8-51ec38e50cfc',
+          name: 'vmhdbdev01',


These names are not used, maybe we could remove them?
And have a list of ids

I kept them there for reference, so that when reading the test you at least know which host is being deregistered, what do you think? maybe better a comment or nothing at all?

Up to you. If it was me I wouldn't have it, but if it helps you, it is not harmful at least

arbulu89 · 2023-07-19T11:57:05Z

test/e2e/cypress/e2e/databases_overview.cy.js

+      cy.deregisterHost(hdq_database.hana_primary.id);
+    });
+
+    it(`should not display DB ${hdq_database.hana_primary.name} after deregistering the primary instance`, () => {


This interpolation doesn't make much sense. This should be hdq_database.sid I guess

arbulu89 · 2023-07-19T11:57:43Z

test/e2e/cypress/e2e/databases_overview.cy.js

+  });
+
+  describe('Deregistration', () => {
+    const hdq_database = {


hdq_database -> hdqDatabase

arbulu89 · 2023-07-19T12:00:17Z

test/e2e/cypress/e2e/sap_systems_overview.cy.js

@@ -291,4 +291,22 @@ context('SAP Systems Overview', () => {
      cy.get('table.table-fixed').should('not.contain', 'DAA');
    });
  });
+
+  describe('Deregistration', () => {
+    const sap_system_nwp = {


sapSystemNwp
hanaPrimary

arbulu89 · 2023-07-19T12:01:28Z

test/e2e/cypress/e2e/sap_systems_overview.cy.js

+        id: '9cd46919-5f19-59aa-993e-cf3736c71053',
+      },
+    };
+


I miss a test here where we check that the SAP system is deregistered when the ASCS or PAS instances are deregistered

arbulu89 · 2023-07-19T12:03:57Z

test/e2e/cypress/e2e/clusters_overview.cy.js

      });
    });
  });
+
+  describe('Deregistration', () => {
+    const hana_cluster_1 = {


hanaCluster1

jamie-suse · 2023-07-19T15:25:34Z

test/e2e/cypress/e2e/clusters_overview.cy.js

-      cy.deregisterHost(hana_cluster_1.hosts[0].id);
-      cy.deregisterHost(hana_cluster_1.hosts[1].id);
-    });
+    before(() => {});


Is this required?

Nope, @rtorrero , you can remove it

test/e2e/cypress/e2e/clusters_overview.cy.js

arbulu89

Good job @rtorrero
Some last nitpicks. Specially, move the deregisterHost to the test, and we are ready to merge

arbulu89 · 2023-07-19T15:43:32Z

test/e2e/cypress/e2e/clusters_overview.cy.js

-      cy.deregisterHost(hana_cluster_1.hosts[0].id);
-      cy.deregisterHost(hana_cluster_1.hosts[1].id);
-    });
+    before(() => {});


Nope, @rtorrero , you can remove it

arbulu89 · 2023-07-19T15:44:02Z

test/e2e/cypress/e2e/clusters_overview.cy.js

+      hosts: [
+        {
+          id: '13e8c25c-3180-5a9a-95c8-51ec38e50cfc',
+          name: 'vmhdbdev01',


Up to you. If it was me I wouldn't have it, but if it helps you, it is not harmful at least

test/e2e/cypress/e2e/clusters_overview.cy.js

arbulu89 · 2023-07-19T15:45:24Z

test/e2e/cypress/e2e/databases_overview.cy.js

+      },
+    };
+
+    before(() => {


This should go in the test as well

arbulu89 · 2023-07-19T15:45:46Z

test/e2e/cypress/e2e/databases_overview.cy.js

+  describe('Deregistration', () => {
+    const hdqDatabase = {
+      sid: 'HDQ',
+      hana_primary: {


Why this in snake_case yet?

arbulu89 · 2023-07-19T15:45:58Z

test/e2e/cypress/e2e/sap_system_details.cy.js

+    before(() => {
+      cy.visit(`/sap_systems/${selectedSystem.Id}`);
+      cy.url().should('include', `/sap_systems/${selectedSystem.Id}`);
+      cy.deregisterHost(hostToDeregister.id);


This to the test

test/e2e/cypress/e2e/sap_systems_overview.cy.js

arbulu89

More things 😅

arbulu89 · 2023-07-20T07:00:53Z

test/e2e/cypress/e2e/hana_cluster_details.cy.js

+    };
+
+    before(() => {
+      cy.deregisterHost(hostToDeregister.id);


deregisterHost move to the test

arbulu89 · 2023-07-20T07:01:23Z

test/e2e/cypress/e2e/hana_database_details.cy.js

+
+  describe('Deregistration', () => {
+    before(() => {
+      cy.deregisterHost(attachedHosts[0].AgentId);


Move to the test

arbulu89 · 2023-07-20T07:04:39Z

test/e2e/cypress/e2e/sap_system_details.cy.js

+      cy.url().should('include', `/sap_systems/${selectedSystem.Id}`);
+    });
+
+    it(`should not include ${hostToDeregister.name} in the list of hosts`, () => {


We could improve this test (or add a new) to check that the SAP instance belonging to this SAP system is not there. @abravosuse reported some potential issue on this?

arbulu89

🔝

rtorrero force-pushed the e2e-test-deregistration-cascading branch from e639652 to 25bbde9 Compare July 18, 2023 15:34

rtorrero added 4 commits July 19, 2023 10:22

Add new command to deregister host

6b80421

Add deregistration tests for clusters,databases & sapsystems overviews

4ca2be2

Add sap system details e2e test

8a257c2

Add hana cluster details e2e test

8166582

rtorrero force-pushed the e2e-test-deregistration-cascading branch from 25bbde9 to 334494e Compare July 19, 2023 11:01

Add e2e test for hana db details

334494e

rtorrero marked this pull request as ready for review July 19, 2023 11:06

rtorrero requested review from arbulu89, jamie-suse and CDimonaco July 19, 2023 11:15

arbulu89 requested changes Jul 19, 2023

View reviewed changes

arbulu89 reviewed Jul 19, 2023

View reviewed changes

rtorrero added 3 commits July 19, 2023 15:41

Move deregistration out of before() and fix style

4342832

camelCase all the things

e342c32

Add additional sap systems e2e tests

99a9b43

rtorrero requested a review from arbulu89 July 19, 2023 14:45

jamie-suse reviewed Jul 19, 2023

View reviewed changes

test/e2e/cypress/e2e/clusters_overview.cy.js Outdated Show resolved Hide resolved

arbulu89 requested changes Jul 19, 2023

View reviewed changes

Additional clean-ups

c0c7e46

rtorrero requested a review from arbulu89 July 20, 2023 06:57

arbulu89 requested changes Jul 20, 2023

View reviewed changes

rtorrero requested a review from arbulu89 July 20, 2023 08:36

rtorrero added 2 commits July 20, 2023 09:36

Move host deregistration out of the before

a7b6c77

Avoid visit and confirm instance removal on layout

fe7d617

arbulu89 approved these changes Jul 20, 2023

View reviewed changes

rtorrero merged commit 5d1295e into main Jul 20, 2023
14 of 16 checks passed

rtorrero deleted the e2e-test-deregistration-cascading branch July 20, 2023 09:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

E2e test deregistration cascading #1634

E2e test deregistration cascading #1634

rtorrero commented Jul 14, 2023 •

edited

Loading

arbulu89 left a comment

arbulu89 Jul 19, 2023

arbulu89 Jul 19, 2023

arbulu89 Jul 19, 2023

rtorrero Jul 19, 2023

arbulu89 Jul 19, 2023

arbulu89 Jul 19, 2023

arbulu89 Jul 19, 2023

arbulu89 Jul 19, 2023

arbulu89 Jul 19, 2023

arbulu89 Jul 19, 2023

jamie-suse Jul 19, 2023

arbulu89 Jul 19, 2023

arbulu89 left a comment

arbulu89 Jul 19, 2023

arbulu89 Jul 19, 2023

arbulu89 Jul 19, 2023

arbulu89 Jul 19, 2023

arbulu89 Jul 19, 2023

arbulu89 left a comment

arbulu89 Jul 20, 2023

arbulu89 Jul 20, 2023

arbulu89 Jul 20, 2023

arbulu89 left a comment

E2e test deregistration cascading #1634

E2e test deregistration cascading #1634

Conversation

rtorrero commented Jul 14, 2023 • edited Loading

Description

arbulu89 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arbulu89 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arbulu89 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arbulu89 left a comment

Choose a reason for hiding this comment

rtorrero commented Jul 14, 2023 •

edited

Loading