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

Cypress visual testing integration #3136

Merged
merged 25 commits into from
May 11, 2021
Merged

Cypress visual testing integration #3136

merged 25 commits into from
May 11, 2021

Conversation

revanth0212
Copy link
Contributor

@revanth0212 revanth0212 commented Apr 21, 2021

Description

Added Cypress Visual Testing plugin. With this, we can write tests that can capture snapshots of the page visually while testing.

Related Issue

Closes PWA-1661

Verification Stakeholders

@dpatil-magento

Verification Steps

  1. Checkout the branch locally.
  2. Navigate to the integration tests package.
  3. Run yarn install
  4. Run yarn test to start the cypress UI.
  5. Run the sample visual test (pageBuilder/homePage.spec.js). Other tests you might see are dummy ones, ignore them for now.

Screenshots / Screen Captures (if appropriate)

Checklist

  • I have added tests to cover my changes, if necessary.
  • I have added translations for new strings, if necessary.
  • I have updated the documentation accordingly, if necessary.

@revanth0212 revanth0212 added the version: Minor This changeset includes functionality added in a backwards compatible manner. label Apr 21, 2021
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Apr 21, 2021

Messages
📖

Associated JIRA tickets: PWA-1661.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against e3a8e6c

@revanth0212 revanth0212 marked this pull request as ready for review April 22, 2021 18:23
@@ -0,0 +1,13 @@
/// <reference types="cypress" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not totally sure, Dev left it in the file. But good question, Ill read up on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it is needed for IntelliSense to suggest cypress auto completions.

it('should render page builder content correctly', () => {
cy.visit('/');

cy.wait(3000);
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this is too short. A no-cache, hard refresh page load on https://develop.pwa-venia.com/ took 6000ms for me.

Maybe 10000 here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you guys are right, but in the scope of this PR, I was working on introducing the plugin and integrating it not dint quite get into the test itself. In the following tickets, we can delegate this to the developer working on the ticket about what they think is the right choice of wait or use a helper as Dave suggested.

The test in this PR is a dummy one, just there to prove that this plugin works after the integration.

@supernova-at
Copy link
Contributor

I got a couple errors while validating:

Error: The plugins file is missing or invalid.

Your `pluginsFile` is set to `path/to/pwa-studio/venia-integration-tests/src/plugins/index.js`, but either the file is missing, it contains a syntax error, or threw an error when required. The `pluginsFile` must be a `.js`, `.ts`, or `.coffee` file.

Or you might have renamed the extension of your `pluginsFile`. If that's the case, restart the test runner.

Please fix this, or set `pluginsFile` to `false` if a plugins file is not necessary for your project.

The file is definitely there and doesn't contain a syntax error. I'm not sure why / how it would throw an error when required.


I also got an "update" message immediately from Cypress saying 7.1.0 is out (we're on 6.8.0). Since we're just introducing Cypress I feel like we should be on the latest major version, at least. Do we have a reason we're not?

Copy link
Contributor

@supernova-at supernova-at left a comment

Choose a reason for hiding this comment

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

Verification errors

@revanth0212
Copy link
Contributor Author

I also got an "update" message immediately from Cypress saying 7.1.0 is out (we're on 6.8.0). Since we're just introducing Cypress I feel like we should be on the latest major version, at least. Do we have a reason we're not?

I have upgraded Cypress in this PR. Can you try yarn install in the folder instead of the outside which is pwa-studio? This package is not hooked up into yarn workspaces so doing a yarn install on the mono repo is not gonna install or update this package.

This was done on purpose because this package is just like docs, has nothing to do with pwa-studio from the outside world's perspective.

@revanth0212
Copy link
Contributor Author

I have made some changes to the comparison logic and hopefully, it will work across machines this time.

image

@dpatil-magento
Copy link
Contributor

@revanth0212 It still fails when screen shot captured window size differs current running window size.

const fs = require('fs');
const { matchImageSnapshotPlugin } = require('cypress-image-snapshot/plugin');

const resizeSnapshot = imageConfig => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the function that resolves differences in terms of resolution and pixel density?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's the one.

delete global.Cypress;
});

test('sup', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this test be skipped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, done 👍

@anthoula
Copy link
Contributor

Tests pass. QA approved. Back @revanth0212 to confirm if there will be additional commits

@dpatil-magento
Copy link
Contributor

QA Approved. Current UI tests are just demo ones and will be replaced soon.

@dpatil-magento dpatil-magento self-requested a review May 11, 2021 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Progress: done version: Minor This changeset includes functionality added in a backwards compatible manner.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants