Add screenshot testing #3736
Add screenshot testing #3736
Conversation
…y/amplify-js into amplify-ui-components
…fig problem manually
| @@ -0,0 +1,3 @@ | |||
| images | |||
There was a problem hiding this comment.
This .gitignore is removing everything in the /screenshot/ folder. I can't quite decide if that's a good or a bad idea -- it will certainly decrease the size of the repo, among other things, but is it really worth the downsides? For example, if you've run tests on your computer (creating screenshots), but someone remotely updates the components, when you pull and re-run the tests they'll fail the screenshot comparison and you'll need to --update-screenshot. Or for another example, suppose you pull the repo fresh and don't run the tests before you change the components -- then the screenshots aren't doing their job at all, since they're not stopping you from accidentally breaking the styling of the components. All in all, I think there's a reason why snapshots are normally committed...
There was a problem hiding this comment.
This is a good analysis. I think the 'baseline' screenshot should be committed so that everyone is comparing against the same image. The latest images generated to compare against the baseline image should be ignored. Is this possible with Stencil screenshot testing?
There was a problem hiding this comment.
Doesn't look like it. The file names are generated via just a hash of the contents, so there's no easy way to know which one is the master. Moreover there's not even a function for deleting a file, let alone removing all of the previous versions whenever a new master is set.
tjleing
force-pushed
the
amplify-ui-components
branch
from
d218cf5 to
4c88b3a
Compare
| @@ -0,0 +1,3 @@ | |||
| images | |||
There was a problem hiding this comment.
This is a good analysis. I think the 'baseline' screenshot should be committed so that everyone is comparing against the same image. The latest images generated to compare against the baseline image should be ignored. Is this possible with Stencil screenshot testing?
packages/amplify-ui-components/src/components/amplify-text-field/amplify-text-field.e2e.ts
Show resolved
Hide resolved
|
Let's add a husky pre-push hook: |
* initial commit for amplify ui web components * Add base styling and cx to emotion css files * v0.1.31 * v0.1.30 * v0.2.0 * v0.1.30 * v0.1.99 * Tests added to amplify-text-field; prettifier run * v0.1.30 * Typo fix * (partly just to fix CircleCI): Implement solution #4 on https://github.com/aws-amplify/amplify-js/pull/3671/files#r304138732 * Cleaner code * change () -> iconChange () for CircleCI, hopefully it works * Add styling to web components * UI component build fix (#3691) * Amplify stencil components * New components * Generate component docs * Update es module to match script * Trying amplify-scene with Amplify imported * Update stencil to 1.0 * feat: amplify-scene and readme build * feat: add storybook and button story * feat: storybook integration and button component changes * Fix Vue build error and update yarn lock * Remove exports * Remove package lock from core * Initial commit to fix #3671 * Move amplify-ui-components package from npm to yarn (#3718) * Clean up linting (#3711) * Remove husky pre-hooks until linting is functioning * Clean up linter * Revert quotation change * Add screenshot testing (#3736) * Move amplify-ui-components package from npm to yarn * Add visual screenshot testing to amplify-text-field, and fix jest-config problem manually * Bump stencil version + husky pre-push testing * Bump resource class for CircleCI * fix typo in constants * Update LICENSE
|
This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs. Looking for a help forum? We recommend joining the Amplify Community Discord server |
Screenshot testing via Stencil + Jest. Also fixes the jest-config problem, although there might be a better way to do it.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.