[test] Add visual regression tests #1081

oliviertassinari · 2021-02-19T22:05:05Z

I'm using the same stack @eps1lon has put in place for the main repository. It's kicking ass. It's insanely fast:

I have noticed a few polish changes we can do in the core, I will apply them in a batch, when I have enough items on my list.

For the screenshots, we use the stories and the demos of the documentation.
The whole process in the CI takes about 2 minutes. It runs in parallel to the other tasks so we won't feel it.
From a cost perspective, each build costs about: $0.01 in Circle CI (200 screenshots) resources.
It's less than 2 minutes at 10 credits per minute. On Argos-CI, the cost is so small that isn't even worth mentioning. Chromatic pricing is $0.008/image, so $1 per build, x100 more expensive. Percy, Applitools are likely in the same order.
I had to disable the random generation of data to guarantee stable diffs for each PR.
useData can be seriously slow. I had to reduce the order of magnitude of the data generated to get the test in a reasonable run duration. We timeout after 2s in dev and 4s in the CI.
The initial Argos-CI build is made comparing a run I did locally with the one of the CI: https://www.argos-ci.com/mui-org/material-ui-x/builds/4
I have removed most of our dependencies on puppeteer, we could remove it.
Cost: 3 hours.
One chunk of Continuous Integration #37, 6 months later

What's left to do later:

Give it a try. The solution is already stress-tested in the main repo, however, the demos we have aren't. We might have unstable content, or else that will require further cleaning
Consider removing screenshots. Right now, we can almost include all, as the overhead is negligible. However, we might still want to remove useless tests.
Fix the rendering of the demos. I have noticed that many demos don't render well. I didn't look into how to solve this.

packages/storybook/src/stories/grid-pagination.stories.tsx

dtassone · 2021-02-22T10:03:50Z

test/karma.conf.js

@@ -30,12 +30,17 @@ const MAX_CIRCLE_CI_CONCURRENCY = 83;
 module.exports = function setKarmaConfig(config) {
  const baseConfig = {
    basePath: '../',
-    browsers: ['ChromeHeadlessNoSandbox'],
+    browsers: ['chromeHeadless'],


It's an arbitrary name. I have replicated the name we give in the main repo.

The names are relevant to the CLI. You can do yarn test:karma --browsers browserNameA,browserNameB.

So the longer the name, the more you have to type in your terminal if you just want to test a subset of browsers. Think of the diff as

- yarn test:karma --browsers ChromeHeadlessNoSandbox + yarn test:karma --browsers chromeHeadless

And since we don't have a chromeHeadlessSandbox I don't see why we need the NoSandbox qualifier in the first place. I haven't found the information whether we run with or without a sandbox useful.

test/regressions/index.js

oliviertassinari · 2021-02-22T21:33:23Z

@mui-org/x I have pushed a few fixes. Let me know if it's OK to move forward

oliviertassinari · 2021-02-23T14:11:56Z

test/karma.conf.js

+    client: {
+      mocha: {
+        // Some BrowserStack browsers can be slow.
+        timeout: (process.env.CIRCLECI === 'true' ? 4 : 2) * 1000,


Fix timeout like https://app.circleci.com/pipelines/github/mui-org/material-ui-x/3259/workflows/2f9cd8b3-825e-4a9b-821c-0c1347a1de53/jobs/14118

dtassone · 2021-02-23T14:36:09Z

package.json

+    "test:regressions:dev": "concurrently \"yarn test:regressions:build --watch\" \"yarn test:regressions:server\"",
+    "test:regressions:run": "mocha --config test/regressions/.mocharc.js --delay 'test/regressions/**/*.test.js'",
+    "test:regressions:server": "serve test/regressions",
+    "test:argos": "node ./scripts/pushArgos.js",


Isn't there a way to reduce the number of scripts?

You only have to run one script yarn test:regressions locally. The others are here to help debug (when working on improving the regression generation tool.

dtassone · 2021-02-23T14:36:33Z

packages/demo-app/src/app/app.tsx

@@ -33,12 +33,6 @@ const GlobalStyle = createGlobalStyle`
    width: 100%;
  }

-  .main-container {


why do we need to change that?

I'm removing dead CSS code (at least, it's the objective)

well I think it should be done in the demo move to the doc PR...

I didn't intend to break the demo-app if it's what you assume. I can definitely revert.

dtassone · 2021-02-23T14:41:23Z

What would be great for this one, is to have a run through and discuss what is happening

oliviertassinari · 2021-02-23T14:54:48Z

What would be great for this one, is to have a run through and discuss what is happening

@dtassone For more context, the latest iteration comes from mui/material-ui#23500 we were using vrtest before (setup in 2017). From a high-level perspective:

We use webpack to build all the demos of the docs and the stories
We make this build available on port 5000
We use mocha to control Playwright
Playwright load the build at port 5000
Mocha go through all the demos that are available. It basically navigate to each URL, wait for the next rAF, take a screenshot and move to the next one
Once Mocha has finished running, we get a folder with all the screenshots
These folders is pushed to Argos-CI
Argos-CI compares all the screenshots with the baseline one it has (the fork commit between HEAD and the PR).
Argos-CI reports the diffs as a GitHub status. If there is a change, it needs to be manually approved
@mnajdova was wondering what approve does. It only does one thing, change the GitHub status (it doesn't replace the baseline screenshots)

dtassone · 2021-02-23T15:28:16Z

Are the rows data different?
What is the size of the screen?

oliviertassinari · 2021-02-23T16:33:23Z

Are the rows data different?

To some extent, yes. I have used DISABLE_CHANCE_RANDOM to guarantee that the generated data is always identical between two different builds. This is critical. Any flakiness kills the usefulness, our ability to depend on the tool. I know you have raised that it means that screenshot can't test the sorting feature. My counter argument is that it shouldn't be tested with a screenshot. This tool shine for catching CSS rendering regressions. For functional tests (not regressions), it's both too slow to run and too slow to iterate on, you don't get a clear signal if the logic is right or wrong, what you get is a visual difference you then need to check. It's mentally draining.

What is the size of the screen?

The viewport should be 1000x700px.

oliviertassinari · 2021-02-23T16:41:09Z

I have an idea for the rendering issue with the data grid stories. I will try to apply the same global class name we use to set the dimensions of the gri in storybook. It should do it.

eps1lon · 2021-02-24T11:41:54Z

It basically navigate to each URL, wait for the next rAF

Don't know if you do things different here but conceptually it's "wait for the demo to be rendered". How we determine when a demo is rendered is a bit more tricky but on the main repo we're just using React+events. We don't rely on any scheduling internals.

oliviertassinari · 2021-02-24T11:59:34Z

Ok, I'm merging with a follow-up to get proper spacing with the stories

oliviertassinari · 2021-02-24T20:00:39Z

Makes me think that we document the testing stack in https://github.com/mui-org/material-ui/blob/next/test/README.md#run-the-visual-regression-tests.

oliviertassinari added the test label Feb 19, 2021

oliviertassinari marked this pull request as ready for review February 20, 2021 00:12

oliviertassinari requested review from dtassone and DanailH February 20, 2021 00:13

oliviertassinari mentioned this pull request Feb 20, 2021

Continuous Integration #37

Closed

13 tasks

dtassone reviewed Feb 22, 2021

View reviewed changes

packages/storybook/src/stories/grid-pagination.stories.tsx Outdated Show resolved Hide resolved

dtassone reviewed Feb 22, 2021

View reviewed changes

oliviertassinari commented Feb 22, 2021

View reviewed changes

test/regressions/index.js Outdated Show resolved Hide resolved

oliviertassinari commented Feb 22, 2021

View reviewed changes

test/regressions/index.js Outdated Show resolved Hide resolved

oliviertassinari added 15 commits February 22, 2021 20:14

[test] Add visual regression tests

21731f3

fix CircleCI config

d651467

fix crash missing channel

a156d08

generate stable fake date

319bb2e

try to fix chromium path

6d35276

exclude Grid100000, too slow

66aa800

disable more broken demos

cb78edb

faster demos

4f961d6

ignore generated folder

f60a6a6

faster build

204ef16

help spot slow tests

e9644d6

install argos-cli

c03fba7

fix build

ebb8687

Only load the fonts we need

621b152

Damien's review

2680307

oliviertassinari force-pushed the visual-regressions branch from 2b5836b to 2680307 Compare February 22, 2021 19:15

oliviertassinari requested a review from dtassone February 22, 2021 19:15

oliviertassinari added 2 commits February 22, 2021 22:28

Merge branch 'master' into visual-regressions

40cf744

skip one more

2606b57

oliviertassinari self-assigned this Feb 22, 2021

oliviertassinari commented Feb 23, 2021

View reviewed changes

dtassone reviewed Feb 23, 2021

View reviewed changes

DanailH approved these changes Feb 24, 2021

View reviewed changes

dtassone approved these changes Feb 24, 2021

View reviewed changes

oliviertassinari merged commit 31d3633 into mui:master Feb 24, 2021

oliviertassinari deleted the visual-regressions branch February 24, 2021 11:59

oliviertassinari mentioned this pull request Feb 24, 2021

[test] Fix containers size for screenshots #1111

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[test] Add visual regression tests #1081

[test] Add visual regression tests #1081

oliviertassinari commented Feb 19, 2021 •

edited

Loading

dtassone Feb 22, 2021

oliviertassinari Feb 22, 2021

eps1lon Feb 22, 2021

oliviertassinari commented Feb 22, 2021

oliviertassinari Feb 23, 2021

dtassone Feb 23, 2021

oliviertassinari Feb 23, 2021

dtassone Feb 23, 2021

oliviertassinari Feb 23, 2021

dtassone Feb 23, 2021

oliviertassinari Feb 23, 2021 •

edited

Loading

dtassone commented Feb 23, 2021

oliviertassinari commented Feb 23, 2021 •

edited

Loading

dtassone commented Feb 23, 2021

oliviertassinari commented Feb 23, 2021 •

edited

Loading

oliviertassinari commented Feb 23, 2021 •

edited

Loading

eps1lon commented Feb 24, 2021

oliviertassinari commented Feb 24, 2021

oliviertassinari commented Feb 24, 2021

[test] Add visual regression tests #1081

[test] Add visual regression tests #1081

Conversation

oliviertassinari commented Feb 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari commented Feb 22, 2021

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

oliviertassinari Feb 23, 2021 • edited Loading

Choose a reason for hiding this comment

dtassone commented Feb 23, 2021

oliviertassinari commented Feb 23, 2021 • edited Loading

dtassone commented Feb 23, 2021

oliviertassinari commented Feb 23, 2021 • edited Loading

oliviertassinari commented Feb 23, 2021 • edited Loading

eps1lon commented Feb 24, 2021

oliviertassinari commented Feb 24, 2021

oliviertassinari commented Feb 24, 2021

oliviertassinari commented Feb 19, 2021 •

edited

Loading

oliviertassinari Feb 23, 2021 •

edited

Loading

oliviertassinari commented Feb 23, 2021 •

edited

Loading

oliviertassinari commented Feb 23, 2021 •

edited

Loading

oliviertassinari commented Feb 23, 2021 •

edited

Loading