test(navigation-bar): playwright refactor #6377
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f8c0633:
|
|
Hi @ZhuoyuJin , Navigation-bar component is missing in storybook so could you please fix that. |
| await expect(navigation).toBeVisible(); | ||
| } else { | ||
| await expect(navigation).not.toBeAttached; | ||
| } |
There was a problem hiding this comment.
Not a blocking change, but this is a bit neater.
if (boolean) {
await expect(navigation).not.toBeAttached;
} else {
await expect(navigation).toBeVisible();
}
|
|
||
| if (position === "fixed") { | ||
| await expect(navigationBar(page).nth(0)).toBeVisible(); | ||
| } |
There was a problem hiding this comment.
I might be wrong but I think the top navigation bar should not be visible if its position is set to fixed. This may be passing because toBeVisible finds elements that are not onscreen but which are in the DOM. I think toBeInViewport does what the test requires.
There should also be an else assertion to verify that the top navigation bar is visible when its position is set to sticky.
| import { | ||
| Default, | ||
| DarkTheme, | ||
| WhiteTheme, | ||
| BlackTheme, | ||
| ExampleWithMenu, | ||
| IsLoading, | ||
| WithCustomSpacing, | ||
| ContentMaxWidthBox, | ||
| Sticky, | ||
| Fixed, | ||
| } from "../../../src/components/navigation-bar/navigation-bar.stories"; |
There was a problem hiding this comment.
question(non-blocking): I thought if we were using stories/examples for Playwright tests, they had to be in the ./components.test-pw file? I could well be wrong here (and tell me if I am because it'll save me a load of time moving the examples to a different file) but that's what I've been doing 😆
| @@ -120,4 +120,4 @@ top/bottom can be offset using the `offset` prop. | |||
|
|
|||
| ### Navigation Bar | |||
|
|
|||
There was a problem hiding this comment.
question(non-blocking): There appears to be a change here but the two lines look the same?
ZhuoyuJin
force-pushed
the
playwright_refactor_navigation_bar
branch
from
c455d1b
to
67d57d6
Compare
| ); | ||
|
|
||
| test.describe("Accessibility tests for NavigationBar component", () => { | ||
| test("should pass accessibility tests Default story", async ({ |
There was a problem hiding this comment.
nitpick/suggestion(non-blocking): Sorry, I should have mentioned this yesterday when I suggested moving them to the test-pw file. As these are no longer stories it should just be:
| test("should pass accessibility tests Default story", async ({ | |
| test("should pass accessibility tests Default example", async ({ |
Same for the others below too. Sorry! My bad 😢
ZhuoyuJin
force-pushed
the
playwright_refactor_navigation_bar
branch
2 times, most recently
from
068fb93
to
90102fa
Compare
ZhuoyuJin
dismissed stale reviews from DipperTheDan and stephenogorman
via
ad423cb
ZhuoyuJin
force-pushed
the
playwright_refactor_navigation_bar
branch
from
553a335
to
ad423cb
Compare
|
🎉 This PR is included in version 123.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Proposed behaviour
Refactor Navigation Bar tests from Cypress to Playwright
Current behaviour
Tests are currently Cypress
Checklist
d.tsfile added or updated if requiredQA
Additional context
N/A
Testing instructions
Testing instructions
navigation-bar.pw.tsxfile passed*.pw.tsxfiles have regressed