Added single VR test script along with updated configs for betterment #546

KosalaiV · 2024-07-04T14:02:28Z

Added a single test script for VR that extracts endpoints from a .csv file.
Updated cypress.config.js with reporter configurations.
Created a config folder containing different environment base URLs for dynamic URL selection.
Enhanced support/e2e.js with after-run test actions.

hussainweb

Thanks for all the work here. I think we can rethink how to do a lot of things here to make it simpler for us and for the long term.

hussainweb · 2024-07-04T14:59:51Z

web/themes/custom/contribtracker/cypress/support/e2e.js

+  // returning false here prevents Cypress from
+  // failing the test


Why don't you want to fail the test when there is a failure?

@hussainweb That is specific for uncaught exceptions, most of the times cypress will fail due to that. Here's the reference

hussainweb · 2024-07-04T15:00:27Z

web/themes/custom/contribtracker/cypress/support/e2e.js

+Cypress.on('test:after:run', (test, runnable) => {
+  if (test.state === 'failed') {
+    let parent = runnable.parent;
+    let filename = '';
+    while (parent && parent.title) {
+      filename = `${titleToFileName(parent.title)} -- ${filename}`;
+      parent = parent.parent;
+    }
+    filename += `${titleToFileName(test.title)} (failed).png`;
+    addContext({ test }, `../screenshots/${Cypress.spec.name}/${filename}`);
+  }
+  // always add the video
+  addContext({ test }, `../videos/${Cypress.spec.name}.mp4`);
+});


What does this do? Please put in a comment.

@hussainweb This is an after run configurations where it will generate an HTML report with attached execution videos and screenshots of the tests.

hussainweb · 2024-07-04T15:01:36Z

web/themes/custom/contribtracker/cypress/support/e2e.js

+require('cypress-grep')();
+require('cypress-xpath');


Where are you using these?

hussainweb · 2024-07-04T15:03:11Z

web/themes/custom/contribtracker/cypress/support/e2e.js

+      parent = parent.parent;
+    }
+    filename += `${titleToFileName(test.title)} (failed).png`;
+    addContext({ test }, `../screenshots/${Cypress.spec.name}/${filename}`);


Isn't this something Cypress does by default?

Yes, it seems this is done to pass data to mochawesome-report-generator. I suggest not using any reporter for now. we can evaluate and consider it later.

This is for the later use to provide an HTML report.

Let's have a separate discussion and PR for reporting. In this PR, we should only focus on writing tests. We can address reporting later on.

hussainweb · 2024-07-04T15:06:23Z

web/themes/custom/contribtracker/package.json

@@ -20,6 +22,14 @@
    "breakpoint-sass": "^3.0.0",
    "browser-sync": "^3.0.2",
    "cypress": "13.12.0",
+    "cypress-grep": "^2.0.1",


This is deprecated. https://www.npmjs.com/package/cypress-grep

Will update it.

hussainweb · 2024-07-04T15:08:19Z

web/themes/custom/contribtracker/cypress.config.js

+  pageLoadTimeout: 500000,
+  requestTimeout: 90000,
+  responseTimeout: 90000,


These timeouts are really long for CI. Is there a reason for such long timeouts?

It is simply a maximum limit; we likely don't want any tests to fail due to exceeding the page load time.

hussainweb · 2024-07-04T15:09:11Z

web/themes/custom/contribtracker/cypress.config.js

+  experimentalStudio: true,
+  trashAssetsBeforeRuns: false,
+  experimentalShadowDomSupport: true,
+  reporter: 'mochawesome',


How does this reporter help? I'd like to keep dependencies to a minimum if this is not important.

I don't think we need a reporter right now. Our focus should be on writing some test cases. We can worry about using a reporter later. Anyway, we have enough visibility of running tests in CI, and locally we can run the tests in interactive mode.

hussainweb · 2024-07-04T15:16:16Z

web/themes/custom/contribtracker/package.json

+    "mochawesome": "7.1.3",
+    "mochawesome-merge": "4.2.2",
+    "mochawesome-report-generator": "6.2.0",
+    "neat-csv": "5.1.0",


This package has not seen an update in 3 years. I am not comfortable with introducing this as a dependency. There is https://www.npmjs.com/package/csv-parse instead but I think we can avoid using CSV entirely. How about storing the URL's as JSON instead?

hussainweb · 2024-07-04T15:18:40Z

web/themes/custom/contribtracker/cypress/fixtures/urls_endpoint.csv

@@ -0,0 +1,12 @@
+url,title


We don't need a CSV parser complicating our dependency tree for such a simple CSV file. In fact, we can just write a simple JS file returning a config array or a JSON file. Something like this:

{ "Home": "/", "CodeContributions", "/code-contributions", // and so on... }

Yes, let's not use CSV. We can use a JSON file or just hardcode the array in the spec file itself.

Will go with JSON file

We have switched to JSON, right? Why still keep the CSV file?

hussainweb · 2024-07-04T15:21:46Z

web/themes/custom/contribtracker/cypress.config.js

  },
-  video: true,


We don't want videos? Should we make this configurable? Anyway, it seems out of the scope for this PR.

@hussainweb This would help a non-technical user to look through why the tests are failing. We'll see the video attached to the HTML report itself. Let me know your thoughts here.

@KosalaiV, it was a question. You are removing the line video: true and I presumed that this change will disable videos from being generated.

@KosalaiV,

Let's keep video: true for now,

So you're going to revert this change?

@hussainweb Will be updating it with env variable like this:
video: process.env.CYPRESS_VIDEO !== 'false'

The plan is to not include videos for VR scripts, hence it defaults to true for the functional test cases

zeshanziya · 2024-07-05T11:32:18Z

web/themes/custom/contribtracker/package.json

@@ -45,6 +55,8 @@
    "uuid": "^10.0.0"
  },
  "dependencies": {
+    "cypress-split": "^1.24.0",
+    "@slack/web-api": "^6.11.2",


Why are we using @slack/web-api package? Are we planning to send a notification to slack?

@zeshanziya Yes, this is for functional tests notification. Likely to attach screenshots of failures

@KosalaiV Let's remove it for now. For Slack integration, we can have another PR. Our intention is to keep it simple, so only changes related to this PR should be included. Any additional changes can be added later. Let's work with small PRs so it's easier to review and merge faster.

zeshanziya · 2024-07-05T11:37:05Z

@KosalaiV, the CI checks are failing. Are you using the latest version of npm? Instead of using your local npm, have you tried setting up contrib-tracker on your local machine? Try using the command specified in the documentation; it will use the npm from the container, and you won't have to worry about updating your local npm. The documentation contains all the steps to run Cypress on your local machine.

hussainweb · 2024-07-29T16:02:51Z

web/themes/custom/contribtracker/cypress.config.js

  },
-  video: true,


@KosalaiV, it was a question. You are removing the line video: true and I presumed that this change will disable videos from being generated.

hussainweb · 2024-07-29T16:03:47Z

web/themes/custom/contribtracker/cypress/e2e/Functional_tasks/hover_checks.cy.js

+// describe('Check for .aspx links', { tags: '@functional' }, () => {
+//   it('should not contain any .aspx links', () => {
+//     cy.visit('https://waps-php.test.ohchr.org/',{failOnStatusCode: false}); // Replace with your actual URL
+
+//     cy.get('a[href*=".aspx"]').then($els => {
+//       if ($els.length > 0) {
+//         // Collect all .aspx links
+//         const aspxLinks = [];
+//         $els.each((index, el) => {
+//           aspxLinks.push(el.getAttribute('href'));
+//         });
+
+//         // Throw an error with all the found .aspx links
+//         throw new Error(`Found .aspx links on the page: ${aspxLinks.join(', ')}`);
+//       }
+//     });
+//   });
+// });


This should be removed, right?

Ahh, Let me remove the commented code.

Let's keep video: true for now, @hussainweb. The intention is to identify any breakages and allow for review of what the test does. Videos make this process easier, and we'll attach the videos to the report.

However, for visual testing with Percy, the video configuration isn't necessary. But for others, it is a good to have one.
I'll add an environment variable setup to accept a boolean value for video.

@KosalaiV, please comment on the relevant line. The video: true line is elsewhere.

hussainweb · 2024-07-29T16:04:38Z

web/themes/custom/contribtracker/cypress/fixtures/urls_endpoint.csv

@@ -0,0 +1,12 @@
+url,title


We have switched to JSON, right? Why still keep the CSV file?

hussainweb · 2024-07-29T16:06:01Z

web/themes/custom/contribtracker/package.json

@@ -45,6 +51,7 @@
    "uuid": "^10.0.0"
  },
  "dependencies": {
+    "cypress-real-events": "^1.13.0",


Why is this not a dev dependency?

…fig setup for cypress

hussainweb · 2024-08-01T08:02:04Z

web/themes/custom/contribtracker/package.json

-    "postcss": "^8.4.40",
+    "postcss": "^8.4.38",


@KosalaiV, why are you changing these dependencies?

@hussainweb I haven't changed this, but we did rebase with main and got some conflicts in package.json. That could be the cause.
cc: @zeshanziya

@KosalaiV It seems we accepted the current change instead of the incoming changes while resolving the conflict. Please revert it to its original version.

…se code script

hussainweb · 2024-10-24T17:52:58Z

@zeshanziya, can you help rebasing this one? I thought this change was combined into the other PR but I guess I was wrong.

zeshanziya · 2024-10-25T04:44:44Z

@hussainweb I believe this PR is old and includes changes that were split into smaller PRs, which have already been merged. Should I still attempt rebasing to check if any changes are pending? We removed the folder structure for test cases, so all test cases are now directly under the e2e folder. In this PR, they're still under e2e/Functional_tasks, which is why these tests are appearing as new additions.

hussainweb · 2024-10-25T11:08:13Z

@zeshanziya, I agree about the tests. They will get rebased, of course, but we will end up with duplicate tests. Is there anything else in this PR that we should adopt? If not, let's close this PR.

dependency-updates-pr-bot · 2024-10-25T13:26:38Z

Npm packages

Package	From	To
prettier	3.3.2	3.3.3
@cypress/xpath	NEW	2.0.3

hussainweb · 2024-10-25T13:41:06Z

@zeshanziya, I rebased this and didn't see any changes left over. I am inclined to close this PR without merging but I wanted you to review once. cc @KosalaiV

zeshanziya · 2024-10-28T05:51:56Z

@hussainweb Yes, we can close this.

hussainweb requested changes Jul 4, 2024

View reviewed changes

hussainweb requested a review from zeshanziya July 4, 2024 15:22

zeshanziya reviewed Jul 5, 2024

View reviewed changes

hussainweb requested changes Jul 29, 2024

View reviewed changes

KOSALAI V added 5 commits August 1, 2024 12:36

Added single VR test script along with usable packages and report con…

819192f

…fig setup for cypress

addressed PR comments

848f3da

worked on axp 172 and 173 along with code clean up

ad10d35

updated tests and video config for axp-172

85f93f8

rebased with main

38f039b

KosalaiV force-pushed the axp-172-cypress branch from 33f9d54 to 38f039b Compare August 1, 2024 07:18

updated lock file with ddev install

4b4d218

hussainweb reviewed Aug 1, 2024

View reviewed changes

KOSALAI V added 2 commits August 1, 2024 15:55

reverted to old package.json

b7741c4

updated ddev commands to execute functional tests and modified respon…

765e45b

…se code script

hussainweb mentioned this pull request Aug 5, 2024

refactor: update Cypress config and npm commands #571

Merged

Merge branch 'main' into axp-172-cypress

e511880

hussainweb mentioned this pull request Aug 5, 2024

test: refactor all the tests into a single file #574

Merged

hussainweb added 2 commits August 5, 2024 22:42

Merge branch 'main' into axp-172-cypress

8c9721c

test: remove old files

c1699e5

hussainweb added 2 commits October 25, 2024 09:13

Merge branch 'main' into axp-172-cypress

a2666fb

rebase: continue combining files

fedda70

hussainweb had a problem deploying to feature October 25, 2024 13:33 — with GitHub Actions Error

hussainweb requested a review from zeshanziya October 25, 2024 13:40

zeshanziya approved these changes Oct 28, 2024

View reviewed changes

		// returning false here prevents Cypress from
		// failing the test

		require('cypress-grep')();
		require('cypress-xpath');

Added single VR test script along with updated configs for betterment #546

Are you sure you want to change the base?

Added single VR test script along with updated configs for betterment #546

Conversation

KosalaiV commented Jul 4, 2024

hussainweb 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

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

zeshanziya commented Jul 5, 2024

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

hussainweb commented Oct 24, 2024 • edited Loading

zeshanziya commented Oct 25, 2024

hussainweb commented Oct 25, 2024

dependency-updates-pr-bot bot commented Oct 25, 2024

Npm packages

hussainweb commented Oct 25, 2024

zeshanziya commented Oct 28, 2024

hussainweb commented Oct 24, 2024 •

edited

Loading