[terra-functional-testing] Fixing Nexus screenshot bugs #839
Conversation
| const outputDir = getOutputDir(); | ||
| const fileName = path.join(outputDir, 'ignored-mismatch.json'); | ||
| // Create the output directory if it does not already exist. | ||
| if (!fs.existsSync(outputDir)) { |
There was a problem hiding this comment.
nit: Is it possible to turn this into an async function? the Sync API from node is infamous for being a blocking API (meaning that it needs to wait for this to complete before doing anything else in node). This will affect performance down the road, especially when we change this to run multiple suites at a time. I might suggest using the equivalent promises API's from node: https://nodejs.org/docs/latest-v14.x/api/fs.html#fs_promises_api
There was a problem hiding this comment.
I don't know if it is possible, at the very least I have a suspicion that would be a major version bump since we would be changing a main testing function to be async when it wasn't before.
| static makeReferenceName(locale, theme, formFactor, browser) { | ||
| const referenceName = [theme, locale, browser, formFactor].filter( | ||
| (str) => { | ||
| if (str !== '' || str !== undefined) { |
There was a problem hiding this comment.
nit: Are there any other types of falsey's we have to worry about? What would happen here with null?
There was a problem hiding this comment.
There shouldn't be but reconsidering this, we're just checking for falsy values so might as well filter them all out for an easy code run so changed here
0f73f13
| try { | ||
| if (process.env.SCREENSHOT_MISMATCH_CHECK === 'true' && buildBranch.match(BUILD_BRANCH.pullRequest)) { | ||
| if (fs.existsSync(fileName) && buildBranch.match(BUILD_BRANCH.pullRequest)) { |
There was a problem hiding this comment.
Like the above comment: I'd try to avoid the sync api as much as possible. This is also already an async function so we should be using the async (Promises) api as much as possible.
There was a problem hiding this comment.
alright finally got a moment to updated this, moved off the sync api and onto the async ones
3b352ec
…olkit into functional-testing-debug
packages/terra-functional-testing/tests/jest/commands/expect/toMatchReference.test.js
Outdated
Show resolved
Hide resolved
packages/terra-functional-testing/tests/jest/commands/expect/toMatchReference.test.js
Outdated
Show resolved
Hide resolved
Summary
Fixed Nexus upload/download of screenshots for multiple nodes.Fixed PR commenting when mismatch was detected.
Fixed logging for when doing a PR build or Screenshot Branch commit build not showing which tests had mismatches.
What was changed:
Nexus screenshots are now stored as multiple directories for each node being run. Directories are named as following with undefined values not being present '$THEME-$LOCALE-$BROWSER-$FORMFACTOR$' I.E 'terra-default-theme-en-chrome-large'
PR commenting is now based on the existence of a file that is made during
toMatchReferencecalls instead of via process.env variables. This file is deleted on test suite completion.Validates.screenshot now passes in the test name to its 'toMatchReference' calls, so that when 'toMatchReference' is doing Nexus mismatch logic it can now output a log message of the test that fails due to mismatch so that in can be found by Nexus users.
Why it was changed:
Testing
This change was tested using:
Reviews
In addition to engineering reviews, this PR needs:
Additional Details
This PR resolves:
UXPLATFORM-9299
Thank you for contributing to Terra.
@cerner/terra