Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DC MILL 0.2339 #388

Merged
merged 39 commits into from
Sep 26, 2023
Merged

DC MILL 0.2339 #388

merged 39 commits into from
Sep 26, 2023

Conversation

Blainegunn
Copy link
Collaborator

TsayAdobe and others added 30 commits September 8, 2023 12:58
MWPW-135084 POC Visual Comparison Test
@aem-code-sync
Copy link

aem-code-sync bot commented Sep 26, 2023

Hello, I'm Franklin Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-run PSI Checks

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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";
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [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";
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [eslint] <quotes> reported by reviewdog 🐶
Strings must use singlequote.

Suggested change
import { FrictionlessPage } from "../page-objects/frictionless.page";
import { FrictionlessPage } from '../page-objects/frictionless.page';

@@ -23,8 +23,10 @@
import { FrictionlessPage } from "../page-objects/frictionless.page";
import { cardinal } from "../support/cardinal";
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [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";
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [eslint] <quotes> reported by reviewdog 🐶
Strings must use singlequote.

Suggested change
import { cardinal } from "../support/cardinal";
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";
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [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.
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [eslint] <no-trailing-spaces> reported by reviewdog 🐶
Trailing spaces not allowed.

Suggested change
* screenshots.
* screenshots.

/***
* This step is used to compare the current screenshots with the baseline
* screenshots.
*
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [eslint] <no-trailing-spaces> reported by reviewdog 🐶
Trailing spaces not allowed.

Suggested change
*
*

* screenshots.
*
* Baseline Folder: features/${feature-name}/${platform}/${browser}
* Current Folder: ${report-dir}/screenshots/${feature-name}/${platform}/${browser}
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [eslint] <no-trailing-spaces> reported by reviewdog 🐶
Trailing spaces not allowed.

Suggested change
* 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
*
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [eslint] <no-trailing-spaces> reported by reviewdog 🐶
Trailing spaces not allowed.

Suggested change
*
*

* 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 () {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ [eslint] <func-names> reported by reviewdog 🐶
Unexpected unnamed async function.

@Blainegunn Blainegunn merged commit de8bf1e into main Sep 26, 2023
*/
Then(/^I should see the same screenshots as baseline$/, async function () {
const comparator = getComparator('image/png');
let baseDir = this.gherkinDocument.uri.replace('.feature', '');
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [eslint] <prefer-const> reported by reviewdog 🐶
'baseDir' is never reassigned. Use 'const' instead.

Suggested change
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [eslint] <prefer-destructuring> reported by reviewdog 🐶
Use object destructuring.

Suggested change
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'));
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [eslint] <arrow-parens> reported by reviewdog 🐶
Expected parentheses around arrow function argument.

Suggested change
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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [eslint] <prefer-const> reported by reviewdog 🐶
'image' is never reassigned. Use 'const' instead.

Suggested change
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);
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [eslint] <prefer-const> reported by reviewdog 🐶
'diffImage' is never reassigned. Use 'const' instead.

Suggested change
let diffImage = comparator(baseImage, currImage);
const diffImage = comparator(baseImage, currImage);

}
}
if (errors.length > 0) {
throw `Differences found: ${errors.join(', ')}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ [eslint] <func-names> reported by reviewdog 🐶
Unexpected unnamed async function.

@@ -296,14 +341,3 @@
this.page.native = newPage;
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [eslint] <no-trailing-spaces> reported by reviewdog 🐶
Trailing spaces not allowed.

Suggested change
this.page.native = newPage;
this.page.native = newPage;

@Blainegunn Blainegunn deleted the 2339 branch November 14, 2023 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants