-
Notifications
You must be signed in to change notification settings - Fork 99
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
test(Button): improve button tests #1756
Conversation
Preview is ready. |
Visual Tests Report is ready. |
@@ -10,6 +10,7 @@ export const expectScreenshotFixture: PlaywrightFixture<ExpectScreenshotFixture> | |||
const expectScreenshot: ExpectScreenshotFixture = async ({ | |||
component, | |||
screenshotName, | |||
screenshotPostfix, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For multiple screenshots per test
test(........., () => {
await expectScreenshot();
//...
await expectScreenshot({
screenshotPostfix: 'after hover',
});
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe screenshotNameSuffix
?
options?: Options, | ||
) => { | ||
const scenarioDetails: ScenarioDetails = { | ||
tag: ['@smoke', ...(options?.additionalTags || [])], | ||
}; | ||
|
||
const scenarioName: ScenarioName = `smoke scenario${options?.scenarioName ? ` ${options?.scenarioName}` : ''}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we shorten this to smoke [default]
or smoke [view: action]
, omit scenario and props words
|
||
await expectScreenshot({ | ||
screenshotPostfix: 'after hover', | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added hover test for the button
@@ -10,6 +10,7 @@ export const expectScreenshotFixture: PlaywrightFixture<ExpectScreenshotFixture> | |||
const expectScreenshot: ExpectScreenshotFixture = async ({ | |||
component, | |||
screenshotName, | |||
screenshotPostfix, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe screenshotNameSuffix
?
await root.getByTestId(qa).hover(); | ||
|
||
await expectScreenshot({ | ||
screenshotPostfix: 'after hover', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use hovered
, thus screenshot name will be more readable: Button-smoke-hovered-...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed
options?: Options, | ||
) => { | ||
const scenarioDetails: ScenarioDetails = { | ||
tag: ['@smoke', ...(options?.additionalTags || [])], | ||
}; | ||
|
||
const scenarioName: ScenarioName = `smoke scenario${options?.scenarioName ? ` ${options?.scenarioName}` : ''}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we shorten this to smoke [default]
or smoke [view: action]
, omit scenario and props words
@@ -10,6 +10,7 @@ export const expectScreenshotFixture: PlaywrightFixture<ExpectScreenshotFixture> | |||
const expectScreenshot: ExpectScreenshotFixture = async ({ | |||
component, | |||
screenshotName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove screenshotName
(it's not used anywhere), and instead of screenshotPostfix
introduce nameSuffix
option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.