cdb-angular(front): Add UI tests #6895

orfeas-k · 2023-01-16T11:04:48Z

This PR is part of the CentralDashboard Rewrite effort and adds UI tests with Cypress for the CentralDashboard-angular component. The tests are about functionalities introduced in the sync browser and iframe URLs PR.

Test case scenarios

User navigates to pages inside the WA they 're already in by clicking links.
User navigates to another WA from the one they were previously in e.g. from JWA details page, they click to the corresponding PVC.
User clicks and affects only the query parameters of the current URL e.g. navigates to another tab inside a WA's details page

In all of the above, we need to make sure that the browser's URL always corresponds to the page the user currently views.

Some details on the implementation:

Selecting other WAs' DOM elements

Best practice in Cypress https://docs.cypress.io/guides/references/best-practices#How-It-Works is to add data-cy-* attributes to the DOM elements one is trying to use in tests in order to be sure tests don't break unless someone explicitly modifies these attributes. In order to avoid, though, modifying elements in other WAs just to be selectable during the CentralDashboard UI tests, we decided to go with second best practice cy.contains('text content'), even if it's a more fragile approach than data-cy-* attributes.

Running UI tests in a GH action

In order to write UI tests for the CentralDashboard that will be able to run in GH actions (since we want any test we write to run also on every PR), we will need to spin up a KinD cluster where we will apply all the necessary manifests, so we can use these WAs to test CentralDashboard's functionalities. Thus, this PR goes ahead and makes use of the testing environment introduced in this PR by @kimwnasptd.

tasos-ale

@orfeas-k Thanks for the PR. Please take a look at my comments, most of them are trivial.

tasos-ale · 2023-01-30T10:24:13Z

.github/workflows/centraldb_angular_frontend_test.yaml

+        run: |
+          cd components/centraldashboard-angular/frontend
+          npm i
+          npm run serve & npx wait-on http://localhost:4200


I don't think we need to use npx since the wait-on is installed as a dependency.

@tasos-ale I removed it during my fixups but I think after all we need it. The UI tests workflow failed with the following error message:

/home/runner/work/_temp/f28aed23-effd-4194-ab41-bea8ffef080d.sh: line 3: wait-on: command not found

OK, let's keep the npx. I also found an issue for that jeffbski/wait-on#86.

tasos-ale · 2023-01-30T10:51:33Z

components/centraldashboard-angular/frontend/src/app/shared/utils.ts

+  const href = window.location.origin + url;
+  const urlObject = new URL(href);


We can make that change in case url does not start with /.

Suggested change

const href = window.location.origin + url;

const urlObject = new URL(href);

const urlObject = new URL(url, window.location.origin);

tasos-ale · 2023-01-30T10:52:19Z

.github/workflows/centraldb_angular_frontend_test.yaml

+        run: |
+          cd components/centraldashboard-angular/frontend
+          npm i
+          npm run serve & npx wait-on http://localhost:4200


I don't think we need npx since we have wait-on in our node modules.

tasos-ale · 2023-01-30T10:57:01Z

components/centraldashboard-angular/frontend/src/app/shared/utils.ts

+  if (!firstUrl && !secondUrl) {
+    console.warn(`Got undefined URLs ${firstUrl} and ${secondUrl}`);
+    return true;
+  }


This is not this PR, but maybe we want to think if we want to consider undefined URLs as equals.

Normally, we don't stumble upon 2 undefined URLs (we did with a previous implementation of URLs syncing). Thus, I think we can go ahead and remove this if completely @tasos-ale

tasos-ale · 2023-01-30T11:13:52Z

components/centraldashboard-angular/frontend/cypress/support/commands.ts

+  cy.get('iframe', { log: false })
+    .its('0.contentWindow.location', { log: false })
+    .then(iframeLocation => {
+      let iframeUrl: string = iframeLocation.pathname + iframeLocation.search;


Suggested change

let iframeUrl: string = iframeLocation.pathname + iframeLocation.search;

const iframeUrl: string = iframeLocation.pathname + iframeLocation.search;

tasos-ale · 2023-01-30T11:17:32Z

components/centraldashboard-angular/frontend/cypress/support/commands.ts

+
+  let iframeUrl: string;
+
+  cy.getIframeUrl().then(url => {
+    iframeUrl = url;
+    cy.location({ log: false }).should(browserLocation => {
+      let browserUrl = browserLocation.pathname + browserLocation.search;
+      browserUrl = removePrefixFrom(browserUrl);
+
+      expect(equalUrlPaths(browserUrl, iframeUrl)).to.be.true;


Suggested change

let iframeUrl: string;

cy.getIframeUrl().then(url => {

iframeUrl = url;

cy.location({ log: false }).should(browserLocation => {

let browserUrl = browserLocation.pathname + browserLocation.search;

browserUrl = removePrefixFrom(browserUrl);

expect(equalUrlPaths(browserUrl, iframeUrl)).to.be.true;

cy.getIframeUrl().then(url => {

cy.location({ log: false }).should(browserLocation => {

let browserUrl = browserLocation.pathname + browserLocation.search;

browserUrl = removePrefixFrom(browserUrl);\

expect(equalUrlPaths(browserUrl, url)).to.be.true;

tasos-ale · 2023-01-30T12:22:52Z

components/centraldashboard-angular/frontend/cypress/e2e/url-syncing.cy.ts

+    cy.equalUrls();
+  });
+
+  it('checks the URLs when user clicks and affects the query parameters of the current URL', () => {


Suggested change

it('checks the URLs when user clicks and affects the query parameters of the current URL', () => {

it('checks the URLs when the user triggers an update in query parameters of the current URL', () => {

tasos-ale · 2023-01-30T12:25:11Z

components/centraldashboard-angular/frontend/cypress/e2e/url-syncing.cy.ts

+    cy.equalUrls();
+  });
+
+  it('checks the URLs when user navigates to the corresponding PVC from JWA details page', () => {


Suggested change

it('checks the URLs when user navigates to the corresponding PVC from JWA details page', () => {

it('checks the URLs when the user navigates to the corresponding PVC from JWA details page', () => {

tasos-ale · 2023-01-30T12:25:18Z

components/centraldashboard-angular/frontend/cypress/e2e/url-syncing.cy.ts

+    cy.mockPodDefaultsRequest();
+  });
+
+  it('checks the URLs when user navigates to pages inside the same WA', () => {


Suggested change

it('checks the URLs when user navigates to pages inside the same WA', () => {

it('checks the URLs when the user navigates to pages inside the same WA', () => {

tasos-ale · 2023-01-30T17:00:10Z

components/centraldashboard-angular/frontend/public/library.js

-        return this.window.location !== this.parent.location;
+        return this.window?.location !== this.parent?.location;


After some digging I found that the reason this.parent.location is undefined is because Cypress changed it to this.self.location. If we change the modifyObstructiveCode configuration to false then we can remove the optional chaining. @orfeas-k wdyt?

Great find! Sounds like a much better solution than introducing optional chaining in a place we normally don't need just for the tests to pass. I 'll push some fix-ups with this included.

- Install Cypress & npm scripts for UI tests - Update frontend README.md with UI tests instructions Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

orfeas-k · 2023-01-31T14:10:01Z

Pushed fixups with all the comments included @tasos-ale, thanks for taking a look! I had to force push since I removed the commit with the optional chaining.

Add UI tests with Cypress to check that the URLs of the browser and the iframe are always synced. More specifically, we test the following scenarios: - User navigates to pages inside the same WA (JWA) - User navigates to another WA from the one they were previously in (to the corresponding PVC from JWA details page) - User clicks and affects the query parameters of the current URL (navigates to tabs inside JWA details page) Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

Modify script names in order to conform with script names in other WAs. Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

orfeas-k · 2023-02-01T10:15:00Z

cc @kimwnasptd since this PR needs approval from KF OWNERS too

kimwnasptd · 2023-02-01T12:58:43Z

@tasos-ale thanks for the review!

/lgtm
/approve

google-oss-prow · 2023-02-01T12:58:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kimwnasptd, tasos-ale

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~.github/OWNERS~~ [kimwnasptd]
~~components/centraldashboard-angular/OWNERS~~ [kimwnasptd,tasos-ale]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

google-oss-prow bot added the size/XXL label Jan 16, 2023

google-oss-prow bot requested review from kimwnasptd, tasos-ale and thesuperzapper January 16, 2023 11:05

orfeas-k force-pushed the feature-orfeas-cdb-angular-ui-tests branch 9 times, most recently from f8ccc82 to 12d5b4d Compare January 18, 2023 14:43

tasos-ale suggested changes Jan 30, 2023

View reviewed changes

centraldashboard(front): Install Cypress

222f3ea

- Install Cypress & npm scripts for UI tests - Update frontend README.md with UI tests instructions Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

orfeas-k force-pushed the feature-orfeas-cdb-angular-ui-tests branch from 12d5b4d to ef41add Compare January 31, 2023 14:09

orfeas-k requested review from tasos-ale and removed request for thesuperzapper and kimwnasptd February 1, 2023 09:04

tasos-ale approved these changes Feb 1, 2023

View reviewed changes

orfeas-k added 3 commits February 1, 2023 11:32

cdb-angular(front): Modify script names

82b4954

Modify script names in order to conform with script names in other WAs. Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

gh-actions(cdb-angular): Add UI tests to CDB's frontend workflow

38bac7f

Signed-off-by: Orfeas Kourkakis <orfeas@arrikto.com>

orfeas-k force-pushed the feature-orfeas-cdb-angular-ui-tests branch from ef41add to 38bac7f Compare February 1, 2023 09:33

orfeas-k requested a review from tasos-ale February 1, 2023 09:42

tasos-ale approved these changes Feb 1, 2023

View reviewed changes

google-oss-prow bot assigned kimwnasptd Feb 1, 2023

google-oss-prow bot added the lgtm label Feb 1, 2023

google-oss-prow bot added the approved label Feb 1, 2023

google-oss-prow bot merged commit 7db9c95 into kubeflow:master Feb 1, 2023

kimwnasptd mentioned this pull request Feb 2, 2023

Add approvers to the frontends #6929

Merged

orfeas-k deleted the feature-orfeas-cdb-angular-ui-tests branch February 15, 2023 12:26

orfeas-k mentioned this pull request Apr 22, 2024

Rewrite the Central Dashboard in Angular #6000

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cdb-angular(front): Add UI tests #6895

cdb-angular(front): Add UI tests #6895

orfeas-k commented Jan 16, 2023

tasos-ale left a comment

tasos-ale Jan 30, 2023

orfeas-k Jan 31, 2023

tasos-ale Feb 1, 2023

tasos-ale Jan 30, 2023

tasos-ale Jan 30, 2023

tasos-ale Jan 30, 2023

orfeas-k Jan 31, 2023

tasos-ale Jan 30, 2023

tasos-ale Jan 30, 2023

tasos-ale Jan 30, 2023

tasos-ale Jan 30, 2023

tasos-ale Jan 30, 2023

tasos-ale Jan 30, 2023

orfeas-k Jan 31, 2023

orfeas-k commented Jan 31, 2023

orfeas-k commented Feb 1, 2023

kimwnasptd commented Feb 1, 2023

google-oss-prow bot commented Feb 1, 2023

		const href = window.location.origin + url;
		const urlObject = new URL(href);

	const href = window.location.origin + url;
	const urlObject = new URL(href);
	const urlObject = new URL(url, window.location.origin);

	let iframeUrl: string = iframeLocation.pathname + iframeLocation.search;
	const iframeUrl: string = iframeLocation.pathname + iframeLocation.search;

	it('checks the URLs when user clicks and affects the query parameters of the current URL', () => {
	it('checks the URLs when the user triggers an update in query parameters of the current URL', () => {

	it('checks the URLs when user navigates to the corresponding PVC from JWA details page', () => {
	it('checks the URLs when the user navigates to the corresponding PVC from JWA details page', () => {

		return this.window.location !== this.parent.location;
		return this.window?.location !== this.parent?.location;

cdb-angular(front): Add UI tests #6895

cdb-angular(front): Add UI tests #6895

Conversation

orfeas-k commented Jan 16, 2023

Test case scenarios

Selecting other WAs' DOM elements

Running UI tests in a GH action

tasos-ale 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orfeas-k commented Jan 31, 2023

orfeas-k commented Feb 1, 2023

kimwnasptd commented Feb 1, 2023

google-oss-prow bot commented Feb 1, 2023