Add visual regression testing with cypress #1339

raimund-schluessler · 2020-08-23T20:28:05Z

This PR tries to add visual regression testing with cypress. I think we really need this, since some components start to be that complex, that we cannot change them without automatically testing for visual regressions. E.g. the AppSidebar component has at least 64 valid combinations of props and slots only for the header, which all change the visual appearance of the component. Testing this manually is basically impossible, since every small change needs to be tested with every valid combination of options. So visual regression testing would help a lot to resolve #1338 and prevent similar problems in the future.

~~However, the cypress setup does not work at the moment, I get the multiple similar errors like this:~~ Fixed with a90e2b4

Error: Webpack Compilation Error
./src/components/AppSidebar/AppSidebar.vue?vue&type=style&index=0&id=4a2a5f2f&lang=scss&scoped=true& (./node_modules/css-loader/dist/cjs.js!./node_modules/vue-loader/lib/loaders/stylePostLoader.js!./node_modules/resolve-url-loader!./node_modules/sass-loader/dist/cjs.js??ref--1-3!./node_modules/vue-loader/lib??vue-loader-options!./src/components/AppSidebar/AppSidebar.vue?vue&type=style&index=0&id=4a2a5f2f&lang=scss&scoped=true&)
Module build failed (from ./node_modules/sass-loader/dist/cjs.js):
Error: Node Sass does not yet support your current environment: Linux 64-bit with Unsupported runtime (80)
For more information on which environments are supported please see:
https://github.com/sass/node-sass/releases/tag/v4.14.1
 @ ./src/components/AppSidebar/AppSidebar.vue?vue&type=style&index=0&id=4a2a5f2f&lang=scss&scoped=true& (./node_modules/vue-style-loader!./node_modules/css-loader/dist/cjs.js!./node_modules/vue-loader/lib/loaders/stylePostLoader.js!./node_modules/resolve-url-loader!./node_modules/sass-loader/dist/cjs.js??ref--1-3!./node_modules/vue-loader/lib??vue-loader-options!./src/components/AppSidebar/AppSidebar.vue?vue&type=style&index=0&id=4a2a5f2f&lang=scss&scoped=true&) 4:14-382
 @ ./src/components/AppSidebar/AppSidebar.vue?vue&type=style&index=0&id=4a2a5f2f&lang=scss&scoped=true&
 @ ./src/components/AppSidebar/AppSidebar.vue
 @ ./tests/visual/components/AppSidebar/AppSidebar.spec.js
 @ multi ./tests/visual/components/AppSidebar/AppSidebar.spec.js

Now I get

Running:  AppSidebar/AppSidebar.spec.js                                                   (1 of 1)


  1) An uncaught error was detected outside of a test

  0 passing (862ms)
  1 failing

  1) An uncaught error was detected outside of a test:
     ReferenceError: The following error originated from your test code, not from Cypress.

  > require is not defined

When Cypress detects uncaught errors originating from your test code it will automatically fail the current test.

Will look into this tomorrow.

Any feedback is very welcome, since I am new to cypress. @skjnldsv I saw that you use cypress for the viewer app, maybe you have an idea what is wrong here.

cypress.json

cypress/start.sh

cypress/stop.sh

package.json

tests/visual/components/AppSidebar/AppSidebar.spec.js

raimund-schluessler · 2020-08-24T12:12:44Z

I think it basically works now. This is the screentshot cypress produces on my system for a simple AppSidebar:

Things to do:

Fix jest to not run the visual tests
Take screenshots for all valid sidebar options (with and without figure, with and without star, with and without actions, etc. and all combinations)
Cleanup

skjnldsv · 2020-08-24T12:15:05Z

Awesome work @raimund-schluessler 🤗

raimund-schluessler · 2020-08-24T21:25:53Z

I added a visual regression test for 24 combinations of figure, secondary-actions, starred and compact. Here is an example image with all options so far:

AppSidebar.vue - starred_true - compact_true - header_image - secondary_button:

Please have a look at them in cypress\snapshots\AppSidebar\AppSidebar.spec.js. So far I think they look good, but I haven't tested with long title and subtitle, without subtitle, with primary-actions and in editable mode. So I guess there are more combinations to come 🙈

skjnldsv

Please add cypress/videos to gitignore :)
Maybe also play with the background instead of the header?
Shall we add a default SdidebarContent?
- If not maybe we can only capture the header?
Add Cypress action

raimund-schluessler · 2020-08-25T11:22:08Z

Looks like we have to run cypress in a Docker container as the images produced by the Github action differ from the ones on my machine. @skjnldsv Do you have an idea how to do that?

skjnldsv · 2020-08-25T16:14:41Z

Mind pushing your adjusted branch to the repo? Then I would play around with it a bit.

DOne, sorry! :)

raimund-schluessler · 2020-08-25T17:36:02Z

Seems to work with another plugin that seems supported

Hum, not really. mjhea0/cypress-visual-regression#50 :(

Hm, no idea. I get the same error no matter what.

skjnldsv · 2020-08-25T18:25:16Z

Fixed @raimund-schluessler
The strings contained new lines :)

raimund-schluessler · 2020-08-25T18:37:26Z

Nice, it runs through on my machine now. But the screenshots taken are different from the previous ones. I will try to update them and push them here. Let's see.

skjnldsv · 2020-08-25T18:39:11Z

Nice, it runs through on my machine now. But the screenshots taken are different from the previous ones. I will try to update them and push them here. Let's see.

The screenshots seems to actually be the same :)
It currently fails because there is a tiny diff

The "AppSidebar.vue - starred_null - compact_false - header_none - secondary_button" image is different. Threshold limit exceeded! 
Expected: 0 
Actual: 0.13896919372599592

raimund-schluessler · 2020-08-25T18:40:34Z

Nice, it runs through on my machine now. But the screenshots taken are different from the previous ones. I will try to update them and push them here. Let's see.

The screenshots seems to actually be the same :)
It currently fails because there is a tiny diff
The "AppSidebar.vue - starred_null - compact_false - header_none - secondary_button" image is different. Threshold limit exceeded! 
Expected: 0 
Actual: 0.13896919372599592

That's what I meant with "different". The letters are rendered slightly different than before.

skjnldsv · 2020-08-25T18:42:01Z

It seems it's not uploading the diff files though
Some example on how to upload the artifacts:

cypress-io/github-action@22681e3

EDIT: doesn't upload to cypress but to github 😕
Maybe it's because I added this to gitignore 🤔

EDIT2: nope! 😢

skjnldsv · 2020-08-25T18:55:12Z

We could also use https://github.com/meinaart/cypress-plugin-snapshots
Now that we have it working and we understand it better.
It seems up to date compared to https://github.com/palmerhq/cypress-image-snapshot

raimund-schluessler · 2020-08-25T18:59:23Z

We could also use https://github.com/meinaart/cypress-plugin-snapshots
Now that we have it working and we understand it better.
It seems up to date compared to https://github.com/palmerhq/cypress-image-snapshot

I don't mind. Both https://github.com/mjhea0/cypress-visual-regression and https://github.com/meinaart/cypress-plugin-snapshots seem to be fine.

The question is how to handle the small differences in the screenshots. Whether we can prevent the different rendering or increase the threshold.

raimund-schluessler · 2020-08-25T20:37:35Z

@skjnldsv I think it uploaded correctly: https://dashboard.cypress.io/projects/3paxvy/runs/8/test-results/408ee0df-0b7a-41b3-b473-433ba35020af

How we treat slightly different screenshots is still up to debate.

skjnldsv · 2020-08-26T07:10:59Z

@skjnldsv I think it uploaded correctly: dashboard.cypress.io/projects/3paxvy/runs/8/test-results/408ee0df-0b7a-41b3-b473-433ba35020af

It's missing the superposée red diffs images :)

raimund-schluessler · 2020-08-26T16:32:38Z

@skjnldsv I moved the cypress runner into a docker container 1e7d85e. This ensures that the local results and the results in CI are exactly the same, since they run in the samer docker container.

Now we only need to fix the uploading to cypress.io and cleanup a bit.

~~Move docker files to cypress folder if possible~~ Seems difficult
De-duplicate github cypress actions
Fix uploading to cypress.io (not really necessary but nice to have)

raimund-schluessler · 2020-08-26T20:38:03Z

@skjnldsv Please have a look. I think it is in a good shape now.

I would add more conditions for AppSidebar (primary-actions, long title etc.) in a follow-up PR, so we can also test if it really finds regressions.

By the way: changing something on the AppSidebar CSS is correctly reported. 👍

Signed-off-by: Raimund Schlüßler <[email protected]>

Achieves reliable results cross-platform Signed-off-by: Raimund Schlüßler <[email protected]>

skjnldsv · 2020-08-27T06:19:30Z

.github/workflows/cypress.yml

+      - name: Cypress run
+        env:
+          CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }}
+        run: npm run cypress:record


I'm not fond, I think that complicates the setup a lot.
Why did we need to load this inside a docker container?

Why did we need to load this inside a docker container?

Because the tests now pass, and failed before 😉

The tiny differences we saw with the font rendering are caused by running the tests on different machines. Between them, maybe the browser, the display settings, the available fonts or whatever else differs slightly, leading to tiny differences in the rendering which make the tests fail.

Of course, we could increase the difference threshold to make the tests pass. But this is not really an option as it would lead to false positives.

The difference in this image is 0.1623 and is not a regression (it's the difference between running locally on my machine and inside the docker container):

The difference in this image is 4 times lower with 0.0406 and it clearly is a regression (I intentionally changed the padding of the close button):

So, running the visual tests in the very same environment ensured by a docker container seems like the only option to fix this. If you have another idea, I am open to check it out.

I think that complicates the setup a lot.

It sure is complicated, but I got the impression, so is visual regression testing in general 🙈

Cypress actually have dockerized github actions as well.
We could have use those?

I'm just not fond of having to also maintain a Docker setup there.

https://github.com/cypress-io/github-action#docker-image

I am fine with using this github action docker setup, but we have to as well locally run cypress within the container. How would you do that then?

skjnldsv · 2020-08-27T06:20:02Z

.github/workflows/cypress.yml

+      matrix:
+        node-version: ['12.x']
+
+    name: Runner ${{ matrix.containers }}


No parallelism anymore (Wasn't needed because we have 1 test, but how would we add more?)

I don't think we gain much here. The whole Cypress action runs for 4:00 min. The cypress run took 35 s, of which running the actual AppSideber.visual.js test took 10 s. We would need to add a lot of tests to really save time.

Edit: But feel free to add it again, of course.

raimund-schluessler added enhancement New feature or request help wanted Extra attention is needed 2. developing Work in progress high High priority design Design, UX, interface and interaction design labels Aug 23, 2020

raimund-schluessler requested review from nickvergessen, georgehrke, juliusknorr and skjnldsv August 23, 2020 20:28