Skip to content

[Base-66] Visual regression#6

Merged
bytasv merged 2 commits into
masterfrom
base-66-visual-regression
Dec 14, 2018
Merged

[Base-66] Visual regression#6
bytasv merged 2 commits into
masterfrom
base-66-visual-regression

Conversation

@bytasv
Copy link
Copy Markdown
Contributor

@bytasv bytasv commented Dec 11, 2018

Added setup, helpers and instructions for running visual regression tests on our components. For now, it's tightly coupled with react storybook, in the future we should make these tests automatic from our documentation or something like that.

I change initial approach a little, decided to use pure puppeteer (with jest preset) with snapshot plugin - works like a charm now and specs are much faster.

@bytasv bytasv added the Specs Changes regarding the spec suite label Dec 11, 2018
@bytasv bytasv self-assigned this Dec 11, 2018
@bytasv bytasv requested a review from MilosMosovsky December 11, 2018 12:58
@bytasv bytasv force-pushed the base-66-visual-regression branch from 9c4b963 to 1877b7e Compare December 13, 2018 12:50
@toptal-bot
Copy link
Copy Markdown
Contributor

This is just a test comment. You can ignore it. @infrastructure_team will delete it soon.

@bytasv bytasv requested a review from a team December 14, 2018 09:48
havenchyk
havenchyk previously approved these changes Dec 14, 2018
Comment thread README.md Outdated

## Installation instructions
TODO
`yarn install` or `npm install`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't we stick to one?

Comment thread bin/build.config.js Outdated
module.exports = {
sourceFiles: path.resolve(__dirname, '../components'),
ignoredFiles: ['components/**/test.js', 'components/**/test.jsx'],
ignoredFiles: ['components/**/test.js', 'components/**/test.jsx', 'components/**/visual.test.jsx', 'components/**/visual.test.js'],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wouldn't components/**/test.js?(x) work? same for visualt.test.js?(x)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, it works, slightly different though: components/**/test.jsx?

Copy link
Copy Markdown
Contributor

@havenchyk havenchyk Dec 14, 2018

Choose a reason for hiding this comment

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

as with webpack, right ;)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We don't have webpack, if I'm not mistaken, this is just node glob syntax. Good catch

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I see that for jest configuration and ignored patterns, same syntax didn't work 🤔 so there is some difference between the two

Comment thread package.json
"rimraf": "^2.6.2",
"yargs": "^12.0.5"
"yargs": "^12.0.5",
"puppeteer": "^1.11.0",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what do you think about adding a separate package.json for tests? e.g. to skip installing puppeteer - pretty heavy dep

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What could be the use case that you would want to install everything except the puppeteer? I imagine if you are touching something in a project, you would be running visual tests as well to verify that you haven't broke anything

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

well, you're right

@bytasv bytasv force-pushed the base-66-visual-regression branch from 66b28ad to d4bceab Compare December 14, 2018 10:12
@bytasv bytasv merged commit 591d282 into master Dec 14, 2018
@bytasv bytasv deleted the base-66-visual-regression branch December 14, 2018 11:15
vedrani added a commit that referenced this pull request May 6, 2026
vedrani added a commit that referenced this pull request May 7, 2026
vedrani added a commit that referenced this pull request May 12, 2026
vedrani added a commit that referenced this pull request May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Specs Changes regarding the spec suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants