Adding Visual Testing: Cypress + Percy#419
Conversation
rokotyan
left a comment
There was a problem hiding this comment.
This is great! Can you please also provide some steps how to set it up locally?
| @@ -0,0 +1,37 @@ | |||
| /// <reference types="cypress" /> | |||
There was a problem hiding this comment.
Do we need this file at all?
There was a problem hiding this comment.
It turns out we do need this file. Without it Cypress can't resolve /commands. See here
|
|
||
| it('Basic Annotation', () => { | ||
| cy.contains('Basic Annotations').click() | ||
| cy.wait(3000) |
There was a problem hiding this comment.
Is there a specific reason to wait 3000 ms?
There was a problem hiding this comment.
Is there a specific reason to wait 3000 ms?
Some page has animations. This way we wait till everything is fully rendered before taking the screenshot.
There was a problem hiding this comment.
Should we add duration as a query string argument to the pages? I think this way you can speed up the testing significantly.
There was a problem hiding this comment.
Should we add
durationas a query string argument to the pages? I think this way you can speed up the testing significantly.
What do you mean? Add something like ?duration=3000 to the url?
There was a problem hiding this comment.
More like ?duration=0 to render charts without animation
There was a problem hiding this comment.
More like
?duration=0to render charts without animation
That's a good idea.
There was a problem hiding this comment.
More like
?duration=0to render charts without animation
See a POC here
| cy.percySnapshot('Stacked Bar with Labels', { scope: '.exampleViewer' }) | ||
| }) | ||
| }) | ||
|
|
There was a problem hiding this comment.
We can try generating this file automatically based on the example files. Technically they have the required information, like the description text.
There was a problem hiding this comment.
We can try generating this file automatically based on the example files. Technically they have the required information, like the description text.
I suppose we could, but some page require additional settings. For example the API Endpoints Tree requires a bigger minHeight, we'd have to manually set things like that for specific test cases. Do you have a workaround for cases like this?
There was a problem hiding this comment.
You can have a map of additional settings in the testing file to apply on the go if needed. I can explain more if you want, just let me know.
There was a problem hiding this comment.
Do you mean to have an array or load a separate file which contains
testingArray = [
{name: 'Single Container', duration:3000, additionalParams: {width: [1300]}},
{name: 'Sankey', duration:3000}, ...
]And iterate through the array inside the unovis.cy.ts?
There was a problem hiding this comment.
Take a look at unovis/packages/dev/src/examples/index.ts, I was thinking about something similar to generate this test file automatically. You'll need to "compile" it using Webpack but I think it will be a reasonable trade-off.
So the script will:
- Batch import all the
index.tsxfiles in the examples folder; - You'll be able to get the required information from the imports (titles, ...);
- You can define additional testing configuration in the
unovis.cy.tsfile and match the corresponding examples by name or path. Or you can define it in theindex.tsxfiles of the examples themsevles.
There was a problem hiding this comment.
@rokotyan I will give it a try. Is this PR getting too big? Should we have a separate PR for this and the duration params?
There was a problem hiding this comment.
Up to you. I would do it here, but I don't see any problem with merging this PR first and having another one
Running this locally is a little tricky. You'd have to have a Percy account and a token to be able to do that. See this. It is possible to have a free personal account, which is what I did when I was testing this out. You need to create a project and it will generate a PERCY_TOKEN. You will then set it as part of your local env. |
82eeb5e to
9d567db
Compare
9d567db to
86f9ed0
Compare
6f2198b to
7780999
Compare
There was a problem hiding this comment.
Nice! I think I would try doing it in packages/dev/src/components/ExampleViewer/index.tsx and passing as a prop, or defining a dedicated React hook that can be imported to avoid code duplication.
0c8331e to
2ead8ea
Compare
There was a problem hiding this comment.
@reb-dev the latest push should fully implement the duration props for all dev files. I put the urls in a separate file, we can fully implement the autogen for testing urls in a separate PR.
29a2648 to
feddac8
Compare
|
@lee00678 dont enable percy run on merge to master. it will eat up the screenshot quota.
|
Core | Test: Add random seed generator for dev data
…ercy integration via browserstack
feddac8 to
fd7304c
Compare
It has a weekly run. Removed the trigger for merge to main. |
I'm setting the trigger for visual testing to happen when merge to main for now. I think we could also have Percy runs on a weekly basis just in case.
main