Silne30/visual regression rests 521 #630
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 17 commits
March 27, 2018 18:13
… and iedriver config, and saucelabs configuration.
…ation for webpack.
…r and webpack config to work together.
… to also be able to serve a static order for visual testing purposes.
…on't want included. Added readme. Added script for jenkins to run tests.
|
jenkins test this |
pugnascotia
reviewed
Apr 11, 2018
package.json
Outdated
| @@ -17,7 +18,10 @@ | |||
| "lint-fix": "eslint --fix --cache --ignore-pattern \"**/*.snap.js\" \"src/**/*.js\" \"src-docs/**/*.js\"", | |||
| "test": "npm run lint && npm run test-unit", | |||
| "test-unit": "jest --config ./scripts/jest/config.json", | |||
| "start-test-server": "./node_modules/.bin/webpack-dev-server --config src-docs/webpack.config.js --port 9999", | |||
There was a problem hiding this comment.
./node_modules/.bin is automatically added to the $PATH for entries in scripts when you run npm or yarn, so you can just write webpack-dev-server. Same for test-visual below.
There was a problem hiding this comment.
This is also very similar to the start script. Is it bad for the tests to supply --hot, as the start script does?
There was a problem hiding this comment.
@pugnascotia I am a bit new to webpack. Is there something that the hot argument provides that I am not understanding?
Fixed the node-modules path.
pugnascotia
reviewed
Apr 11, 2018
test/wdio.conf.js
Outdated
| chromeOptions: { | ||
| // to run chrome headless the following flags are required | ||
| // (see https://developers.google.com/web/updates/2017/04/headless-chrome) | ||
| // args: ['--headless', '--disable-gpu'], |
There was a problem hiding this comment.
Is it intended that headless mode be available at some point via e.g. a HEADLESS environment variable? Or is this code here so it can be commented out? FWIW Cloud's UI e2e tests respond to the CI and HEADLESS env vars.
There was a problem hiding this comment.
I think this is just code that can be deleted.
|
local test run logs on macOS High Sierra: |
|
The test run introduced a bunch of non-git-ignored screenshots at |
|
Seems pretty easy to locally run the tests and add new ones. I, personally, would like to see the "writing tests" section of the readme more fleshed out and tailored to how we should be writing them. For instance, should we be relying on classes/id's to select elements or use data attributes? Also, should we be targeting each component on the page or just select the whole page? |
4 tasks
|
@bevacqua Thanks for letting me know of your test results. Yes, the uncommitted baselines were expected but we are going to change that behavior. Instead of having baselines developed by one machine, we are going to have the test create the base line on a per user basic (against master) and then run again to check your code changes. That way, variances between systems don't cause problems. |
added 2 commits
April 16, 2018 15:17
|
jenkins test this |
added 2 commits
April 19, 2018 17:31
…ser. Also removed baseline images as the tests will run twice, the first will create the baselines.
cuff-links
added
work in progress
blocked
testing
scripts/run-visual-tests.js
Outdated
| return self.repo.getCurrentBranch() | ||
| .then(function (branch) { | ||
| console.log(displayCurrentBranch(branch)); | ||
| execSync('yarn start-test-server-and-visual-test', { stdio: [0,1,2] } ); |
There was a problem hiding this comment.
better to exec instead of execSync and return a promise that is resolved by the async execution instead of relying on this synchronous call to block before the following then block to switch branches & do the next test run.
refactor run-visual-tests.js into async/await
|
I'm having some issues running this locally, working through it. |
… source of wdio-visual-regression-service
Cleaned up the visual test code and some configs
|
LGTM; after this core functionality is in we can discuss exactly what should be covered and write some more documentation around visual regression testing @snide |
This was referenced May 3, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added visual regression suite to EUI
Fixes #521