MWPW-135084 POC Visual Comparison Test #375
Conversation
TsayAdobe
requested review from
Blainegunn,
seanchoi-dev,
zagi25,
puruadobe and
JFernandezAdobe
|
Hello, I'm Franklin Bot and I will run some test suites that validate the page speed.
|
Codecov Report
@@ Coverage Diff @@
## stage #375 +/- ##
=======================================
Coverage 93.30% 93.30%
=======================================
Files 25 25
Lines 2927 2927
=======================================
Hits 2731 2731
Misses 196 196 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
eslint
test/e2e/frictionless/step-definitions/dc.steps.js|291 col 7| 'baseDir' is never reassigned. Use 'const' instead.
test/e2e/frictionless/step-definitions/dc.steps.js|293 col 9| Use object destructuring.
test/e2e/frictionless/step-definitions/dc.steps.js|296 col 51| Expected parentheses around arrow function argument.
test/e2e/frictionless/step-definitions/dc.steps.js|301 col 12| 'image' is never reassigned. Use 'const' instead.
test/e2e/frictionless/step-definitions/dc.steps.js|304 col 9| 'diffImage' is never reassigned. Use 'const' instead.
test/e2e/frictionless/step-definitions/dc.steps.js|311 col 5| Expected an error object to be thrown.
test/e2e/frictionless/step-definitions/dc.steps.js|315 col 40| Unnecessary escape character: ".
test/e2e/frictionless/step-definitions/dc.steps.js|315 col 58| Unexpected unnamed async function.
test/e2e/frictionless/step-definitions/dc.steps.js|341 col 30| Trailing spaces not allowed.
| @@ -23,8 +23,10 @@ import { CompressPdfPage } from "../page-objects/compresspdf.page"; | |||
| import { FrictionlessPage } from "../page-objects/frictionless.page"; | |||
There was a problem hiding this comment.
🚫 [eslint] <import/extensions> reported by reviewdog 🐶
Missing file extension "js" for "../page-objects/frictionless.page"
| @@ -23,8 +23,10 @@ | |||
| import { FrictionlessPage } from "../page-objects/frictionless.page"; | |||
| @@ -23,8 +23,10 @@ | |||
| import { FrictionlessPage } from "../page-objects/frictionless.page"; | |||
| import { cardinal } from "../support/cardinal"; | |||
There was a problem hiding this comment.
🚫 [eslint] <import/extensions> reported by reviewdog 🐶
Missing file extension "js" for "../support/cardinal"
| @@ -23,8 +23,10 @@ | |||
| import { FrictionlessPage } from "../page-objects/frictionless.page"; | |||
| import { cardinal } from "../support/cardinal"; | |||
| @@ -23,8 +23,10 @@ | |||
| import { FrictionlessPage } from "../page-objects/frictionless.page"; | |||
| import { cardinal } from "../support/cardinal"; | |||
| import { expect } from "@playwright/test"; | |||
There was a problem hiding this comment.
🚫 [eslint] <import/no-extraneous-dependencies> reported by reviewdog 🐶
'@playwright/test' should be listed in the project's dependencies. Run 'npm i -S @playwright/test' to add it
| await this.page.closeSubMenu(index); | ||
| await expect(this.page.fedsPopup).not.toBeVisible(); | ||
| } | ||
| }); | ||
|
|
||
| /*** | ||
| * This step is used to compare the current screenshots with the baseline | ||
| * screenshots. |
There was a problem hiding this comment.
🚫 [eslint] <no-trailing-spaces> reported by reviewdog 🐶
Trailing spaces not allowed.
| * screenshots. | |
| * screenshots. |
| /*** | ||
| * This step is used to compare the current screenshots with the baseline | ||
| * screenshots. | ||
| * |
There was a problem hiding this comment.
🚫 [eslint] <no-trailing-spaces> reported by reviewdog 🐶
Trailing spaces not allowed.
| * | |
| * |
| * screenshots. | ||
| * | ||
| * Baseline Folder: features/${feature-name}/${platform}/${browser} | ||
| * Current Folder: ${report-dir}/screenshots/${feature-name}/${platform}/${browser} |
There was a problem hiding this comment.
🚫 [eslint] <no-trailing-spaces> reported by reviewdog 🐶
Trailing spaces not allowed.
| * Current Folder: ${report-dir}/screenshots/${feature-name}/${platform}/${browser} | |
| * Current Folder: ${report-dir}/screenshots/${feature-name}/${platform}/${browser} |
| * Baseline Folder: features/${feature-name}/${platform}/${browser} | ||
| * Current Folder: ${report-dir}/screenshots/${feature-name}/${platform}/${browser} | ||
| * Diff Image: ${report-dir}/${platform}_${browser}_${image-name}.png | ||
| * |
There was a problem hiding this comment.
🚫 [eslint] <no-trailing-spaces> reported by reviewdog 🐶
Trailing spaces not allowed.
| * | |
| * |
| * Command line options: | ||
| * --baseBrowser: Use a different browser to compare with the current browser | ||
| */ | ||
| Then(/^I should see the same screenshots as baseline$/, async function () { |
There was a problem hiding this comment.
Unexpected unnamed async function.
There was a problem hiding this comment.
@TsayAdobe check if we need more override of eslint rules for automation code and update accordingly.
| */ | ||
| Then(/^I should see the same screenshots as baseline$/, async function () { | ||
| const comparator = getComparator('image/png'); | ||
| let baseDir = this.gherkinDocument.uri.replace('.feature', ''); |
There was a problem hiding this comment.
🚫 [eslint] <prefer-const> reported by reviewdog 🐶
'baseDir' is never reassigned. Use 'const' instead.
| let baseDir = this.gherkinDocument.uri.replace('.feature', ''); | |
| const baseDir = this.gherkinDocument.uri.replace('.feature', ''); |
| const comparator = getComparator('image/png'); | ||
| let baseDir = this.gherkinDocument.uri.replace('.feature', ''); | ||
|
|
||
| const profile = global.config.profile; |
There was a problem hiding this comment.
🚫 [eslint] <prefer-destructuring> reported by reviewdog 🐶
Use object destructuring.
| const profile = global.config.profile; | |
| const {profile} = global.config; |
| const profile = global.config.profile; | ||
| let outputDir = this.gherkinDocument.uri.split('/features/')[1].replace('.feature', ''); | ||
| outputDir = `${profile.reportDir}/screenshots/${outputDir}/${os.platform()}/${profile.browser}`; | ||
| const images = fs.readdirSync(outputDir).filter(x => x.endsWith('.png')); |
There was a problem hiding this comment.
🚫 [eslint] <arrow-parens> reported by reviewdog 🐶
Expected parentheses around arrow function argument.
| const images = fs.readdirSync(outputDir).filter(x => x.endsWith('.png')); | |
| const images = fs.readdirSync(outputDir).filter((x) => x.endsWith('.png')); |
| const baseBrowser = profile.baseBrowser || profile.browser; | ||
|
|
||
| const errors = []; | ||
| for (let image of images) { |
There was a problem hiding this comment.
🚫 [eslint] <prefer-const> reported by reviewdog 🐶
'image' is never reassigned. Use 'const' instead.
| for (let image of images) { | |
| for (const image of images) { |
| for (let image of images) { | ||
| const baseImage = fs.readFileSync(`${baseDir}/${os.platform()}/${baseBrowser}/${image}`); | ||
| const currImage = fs.readFileSync(`${outputDir}/${image}`); | ||
| let diffImage = comparator(baseImage, currImage); |
There was a problem hiding this comment.
🚫 [eslint] <prefer-const> reported by reviewdog 🐶
'diffImage' is never reassigned. Use 'const' instead.
| let diffImage = comparator(baseImage, currImage); | |
| const diffImage = comparator(baseImage, currImage); |
| } | ||
| } | ||
| if (errors.length > 0) { | ||
| throw `Differences found: ${errors.join(', ')}`; |
There was a problem hiding this comment.
🚫 [eslint] <no-throw-literal> reported by reviewdog 🐶
Expected an error object to be thrown.
| throw `Differences found: ${errors.join(', ')}`; | ||
| } | ||
| }); | ||
|
|
||
| Then(/^I should be able to use the "([^\"]*)" submenu$/, async function (menu) { |
There was a problem hiding this comment.
🚫 [eslint] <no-useless-escape> reported by reviewdog 🐶
Unnecessary escape character: ".
| throw `Differences found: ${errors.join(', ')}`; | ||
| } | ||
| }); | ||
|
|
||
| Then(/^I should be able to use the "([^\"]*)" submenu$/, async function (menu) { |
There was a problem hiding this comment.
Unexpected unnamed async function.
| @@ -296,14 +341,3 @@ | |||
| this.page.native = newPage; | |||
There was a problem hiding this comment.
🚫 [eslint] <no-trailing-spaces> reported by reviewdog 🐶
Trailing spaces not allowed.
| this.page.native = newPage; | |
| this.page.native = newPage; |
We should skip the test folder for now. Tune the rules for the product code first, and then add the test folder. |
MWPW-135084
This is a POC of visual comparison tests.
Features: