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

chore: add testing for visual regression testing #996

Closed
wants to merge 8 commits into from

Conversation

awentzel
Copy link
Collaborator

@awentzel awentzel commented Oct 5, 2018

By individual choice users can not execute visual regression testing on their own branches. See Wiki for details.

build/testing/cypress/fixtures is where the examples are saved for reference.
build/testing/cypress/fixtures/sites is where the code relevant to our UI Sites is stored.

@awentzel awentzel force-pushed the users/awentzel/test-applitools-cypress-sdk branch from 14cb17e to 697f65f Compare October 11, 2018 00:39
@awentzel awentzel force-pushed the users/awentzel/test-applitools-cypress-sdk branch from 7713d64 to 3a46b24 Compare October 15, 2018 20:10
"@types/node": "^9.4.7",
"cypress": "^3.1.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is it necessary to use Cypress? It appears we are using applitools selenium package already and I believe you can take screenshots with it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cypress is the future. It's faster and non-selenium. As stated previously, they are working on full vdiff support and better browser coverage (just like everyone else that claims to be E2E) Until then this is the fastest and most efficient way to test UI changes. It also, does integrate with Applitools if someone wanted to test visual regression. Browser Stack does not do visual regression. So this is the best combination I've found that gets closest to meeting our needs based on our current architecture. These features probably are 3-6 months out. But, until then we can evolve what we have use the same code we have today without writing special for Selenium and move the needle forward.

Copy link
Collaborator

@janechu janechu Oct 19, 2018

Choose a reason for hiding this comment

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

Currently Cypress does not have E2E capabilities. It does not support Edge or IE. There is no guarantee it ever will, or the timeline. On this alone I do not agree with its inclusion at this time, I would be open to switching to it when it becomes available.

Is Cypress currently doing something that Selenium cannot and that we think is necessary?

Edit: I would be interested in a hybrid approach if we have a complete breakdown of where we would use Selenium and where we would use Cypress so that we can know what tests to write for each circumstance.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When you say "Cypress does not have E2E capabilities" I respond with nobody actually has E2E capabilities with IE/EDGE. But, Cypress has committed in the past month to delivering it and this is the best testing suite I found and has the most promise that meets the product requirements.

Are you saying you'd rather go without the ability to test in a local development environment UI changes and functionality?

If so, I disagree with because as we've already seen deployments with major visual UI bugs. For the quality of the product, I feel it's necessary to have an option to quickly and efficiently run tests to validate UI. Cypress's SDK with Applitool's allows visual regression testing for each contributor and it's not automated in to the pipeline by design.

Selenium is just the execution platform. It's not a framework. Cypress engine is 100% JavaScript so doesn't run on slow antiquated Java platform, has the best documentation, is easy to use and understand, it integrates well with any other JavaScript, doesn't require rewriting any tests, and performs tests magnitudes faster than anything Selenium. Because it's JavaScript and has it's own execution environment it's able to test any part of a browser or rendered UI in the browser.

The development process
The process would be to use Cypress API w/ Applitools for local testing. Because it can spin up and run quickly with integration into both Jest and Enzyme with little overhead. You can run as you develop and actually break on your code, throttle network settings, and use any of the available browser tools to debug, troubleshoot, and fix UI issues, directly in code and with it's live auto-refresh it will re-run tests on changes.

Sauce Labs / BrowserStack are the two biggest names for cross-browser sanity testing as Nick Rice explained in a previous PR. Which is great to run prior to merging into master for one final test across browsers. But, it's all manual review process, no visual regression either. For us, we only test browsers based on different rendering engines. But the process here does all the work on a remote server and literally is only suited for visual regression. You can't take any action to correct or develop other than go back into your code and make more changes hope for the best then rerun it all again.

summary
I believe something (which in my many hours of research looks to be the best) is better than nothing. Cypress's competitors claim E2E, but, similarly they only support E2E for limited browsers (Chromium).

As long as we have UI bugs making its way into production, we need something. This is the best starting point I've found and already with the deepest feature set and potential. They have promised cross-browser support which other's have not.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well again, committing to something is not the same as having it. If Cypress had ability to test IE, Edge, firefox and Safari today I would be all for removing Selenium. As far as I know it's still only Chromium.

My only questions:

  1. Is this going to meet our requirement to cross browser visual diff on the major browsers we are supporting?
  2. Will we be able to see the visual diffs during a Pull Request?
  3. Will we be doing integration testing on our apps and with what tools? If we use Cypress what are the potential pitfalls?
  4. Will we be able to test locally if the testing fails on the build gate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Answers:

  1. Sauce Labs would be used to cross browser test the major browsers that we are supporting.
  2. Visual Diff and Cross Browser will not be built into the PR. This is by design (@Falkicon) because we do not want to block the process waiting for manual reviews/approvals. We want individuals to have the tools and responsibility to test locally before submitting PRs.
  3. Yes, when the time is right. Integration testings is comprehensive so we'd need to discuss what exactly you mean. Integration testing could be services integration, api integration or something else. These could be different tools. As for visual integration testing, Cypress has the same two pitfalls as all other options. No built in vdiff (not sure if ever), no cross-browser yet. Applitools will be used for Vdiff with the Cypress SDK which is included in this PR. Applitools also is working on cross-browser matrix and while our strategies are in alignment, they haven't yet released their matrix options. if they release in alignment with our requirements we can deprecate Sauce Labs. Sauce Labs also can be used manually and directly from their dashboard outside local code.
  4. See feat: adds fast-react-jss-manager project #2. But, we will always are able to test locally.

Essentially, the market is migrating off Selenium and onto JavaScript. But, all the players right now are missing some key features that we require. So this just gets us ability to test today and we can upgrade as new features come. If it turns out Cypress fails to deliver or a better option turns up we can always migrate without requiring heavy refactor effort.

Demo to be scheduled to talk through any remaining concerns.

@stale
Copy link

stale bot commented Oct 28, 2018

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the warning:stale No recent activity within a reasonable amount of time label Oct 28, 2018
@stale stale bot closed this Nov 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
warning:stale No recent activity within a reasonable amount of time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants