Skip to content

Commit

Permalink
Add eslint-plugin-playwright (#2574)
Browse files Browse the repository at this point in the history
* Add `eslint-plugin-playwright`

* Use explicit toBeChecked assertion

Co-authored-by: Olga Bulat <[email protected]>

* Fix bad find/replace update

* Replace and fix usages of `getCheckbox` abstraction with regular PLaywright utility calls

* Update frontend/test/playwright/e2e/filters.spec.ts

---------

Co-authored-by: Olga Bulat <[email protected]>
  • Loading branch information
sarayourfriend and obulat authored Jul 11, 2023
1 parent 4da9462 commit dfc9635
Show file tree
Hide file tree
Showing 30 changed files with 234 additions and 168 deletions.
9 changes: 9 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,15 @@ module.exports = {
],
},
},
{
files: ["frontend/test/{playwright,storybook}/**"],
plugins: ["playwright"],
extends: ["plugin:playwright/recommended"],
rules: {
// Enable once https://github.com/playwright-community/eslint-plugin-playwright/issues/154 is resolved
"playwright/expect-expect": ["off"],
},
},
{
files: [
"automations/js/src/**",
Expand Down
2 changes: 1 addition & 1 deletion frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"test:unit": "jest",
"test:unit:watch": "pnpm test:unit --collectCoverage=false --watch",
"test:playwright": "./bin/playwright.sh",
"test:playwright:local": "pnpm exec playwright test -c test/playwright",
"test:playwright:local": "playwright test -c test/playwright",
"test:playwright:debug": "PWDEBUG=1 pnpm test:playwright:local",
"test:playwright:recreate-tapes": "rimraf test/tapes && pnpm test:playwright:update-tapes",
"test:playwright:update-tapes": "UPDATE_TAPES=true pnpm test:playwright",
Expand Down
12 changes: 6 additions & 6 deletions frontend/test/playwright/e2e/all-results-keyboard.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ test.describe("all results grid keyboard accessibility test", () => {
test("should open image results as links", async ({ page }) => {
await walkToType("image", page)
await page.keyboard.press("Enter")
await page.waitForURL(
new RegExp(
`/image/[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}\\?q=birds$`,
"i"
)
const urlRegex = new RegExp(
`/image/[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}\\?q=birds$`,
"i"
)
await page.waitForURL(urlRegex)
expect(page.url()).toMatch(urlRegex)
})

test("should open audio results as links", async ({ page }) => {
Expand Down Expand Up @@ -123,6 +123,6 @@ test.describe("all results grid keyboard accessibility test", () => {
await audio.getActive(nextFocusedResult)

await expect(playButton).toBeVisible()
await expect(pauseButton).not.toBeVisible()
await expect(pauseButton).toBeHidden()
})
})
4 changes: 2 additions & 2 deletions frontend/test/playwright/e2e/audio-detail.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ test("sends a custom event on seek", async ({ page, context }) => {

test("shows the 404 error page when no valid id", async ({ page }) => {
await page.goto("audio/foo")
await showsErrorPage(page)
await expect(showsErrorPage(page)).resolves.toBeUndefined()
})

test("shows the 404 error page when no id", async ({ page }) => {
await page.goto("audio/")
await showsErrorPage(page)
await expect(showsErrorPage(page)).resolves.toBeUndefined()
})
4 changes: 0 additions & 4 deletions frontend/test/playwright/e2e/external-sources.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ test("sends correct analytics events", async ({ page, context }) => {
(event) => event.n === "SELECT_EXTERNAL_SOURCE"
)

if (!viewEvent || !selectEvent) {
throw new Error("Analytics events were not triggered properly.")
}

expectEventPayloadToMatch(viewEvent, {
searchType: "all",
query: "cat",
Expand Down
7 changes: 4 additions & 3 deletions frontend/test/playwright/e2e/filters-sidebar-keyboard.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ for (const dir of languageDirections) {
await walkToFilterButton(page)

// Check that the filters sidebar is open
expect(
await page.locator("#filter-button").getAttribute("aria-expanded")
).toBe("true")
await expect(page.locator("#filter-button")).toHaveAttribute(
"aria-expanded",
"true"
)

await page.keyboard.press(keycodes.Tab)

Expand Down
115 changes: 82 additions & 33 deletions frontend/test/playwright/e2e/filters.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { test, expect, Page } from "@playwright/test"

import {
assertCheckboxStatus,
changeSearchType,
goToSearchTerm,
isPageDesktop,
Expand All @@ -12,6 +11,8 @@ import { mockProviderApis } from "~~/test/playwright/utils/route"

import breakpoints from "~~/test/playwright/utils/breakpoints"

import enMessages from "~/locales/en.json"

import {
supportedSearchTypes,
ALL_MEDIA,
Expand Down Expand Up @@ -53,7 +54,9 @@ breakpoints.describeMobileAndDesktop(() => {

await filters.open(page)

await assertCheckboxCount(page, "total", FILTER_COUNTS[searchType])
await expect(
assertCheckboxCount(page, "total", FILTER_COUNTS[searchType])
).resolves.toBeUndefined()
})
}

Expand All @@ -63,10 +66,10 @@ breakpoints.describeMobileAndDesktop(() => {
)
await filters.open(page)
// Creator filter was removed from the UI
const expectedFilters = ["cc0", "commercial"]
const expectedFilters = ["Zero", "Use commercially"]

for (const checkbox of expectedFilters) {
await assertCheckboxStatus(page, checkbox)
await expect(page.getByRole("checkbox", { name: checkbox })).toBeChecked()
}
})

Expand All @@ -78,10 +81,10 @@ breakpoints.describeMobileAndDesktop(() => {
)
await filters.open(page)
// Creator filter was removed from the UI
const expectedFilters = ["cc0", "commercial"]
const expectedFilters = ["Zero", "Use commercially"]

for (const checkbox of expectedFilters) {
await assertCheckboxStatus(page, checkbox)
await expect(page.getByRole("checkbox", { name: checkbox })).toBeChecked()
}
await changeSearchType(page, IMAGE)

Expand All @@ -90,7 +93,7 @@ breakpoints.describeMobileAndDesktop(() => {
)
await filters.open(page)
for (const checkbox of expectedFilters) {
await assertCheckboxStatus(page, checkbox)
await expect(page.getByRole("checkbox", { name: checkbox })).toBeChecked()
}
})

Expand All @@ -103,8 +106,8 @@ breakpoints.describeMobileAndDesktop(() => {
await filters.open(page)

// Creator filter was removed from the UI
for (const checkbox of ["cc0", "commercial"]) {
await assertCheckboxStatus(page, checkbox)
for (const checkbox of ["Zero", "Use commercially"]) {
await expect(page.getByRole("checkbox", { name: checkbox })).toBeChecked()
}

await changeSearchType(page, ALL_MEDIA)
Expand All @@ -120,23 +123,64 @@ breakpoints.describeMobileAndDesktop(() => {
test("selecting some filters can disable dependent filters", async ({
page,
}) => {
await page.goto("/search/audio?q=cat&license_type=commercial")
// Ignore the "+" licenses which are not presented on the page
// `exact: true` is required in locators later in this test to prevent "Attribution" from matching
// all CC licenses with the BY element (all of them :P)
const allLicenses = Object.values(enMessages.licenseReadableNames).filter(
(l) => !l.includes("Plus")
)
const nonCommercialLicenses = allLicenses.filter((l) =>
l.includes("NonCommercial")
)
const commercialLicenses = allLicenses.filter(
(l) => !nonCommercialLicenses.includes(l)
)

await page.goto("/search/audio?q=cat")
await filters.open(page)

// by-nc is special because we normally test for fuzzy match, and by-nc matches 3 labels.
const byNc = page.locator('input[value="by-nc"]')
await expect(byNc).toBeDisabled()
for (const checkbox of ["by-nc-sa", "by-nc-nd"]) {
await assertCheckboxStatus(page, checkbox, "disabled")
await expect(
page.getByRole("checkbox", { name: "Use commercially" })
).not.toBeChecked()

// Use commercially is not enabled yet, so commercial licenses are still available
// Therefore, all active licenses should have enabled checkboxes
for (const checkbox of allLicenses) {
const element = page.getByRole("checkbox", {
name: checkbox,
exact: true,
})
await expect(element).toBeVisible()
await expect(element).toBeEnabled()
}
await assertCheckboxStatus(page, "commercial")

await page.click('label:has-text("commercial")')
// Enable the commercial use filter
await page.locator('label:has-text("Use commercially")').click()

await expect(
page.getByRole("checkbox", { name: "Use commercially" })
).toBeChecked()

// Because we checked "Use commercially", licenses that disallow commercial
// use will be disabled and the rest will still be enabled.
// Additionally, none of the checkboxes will be checked because we've only
// manipulated the commercial filter, not any specific license filters
for (const checkbox of nonCommercialLicenses) {
await expect(
page.getByRole("checkbox", { name: checkbox, exact: true })
).not.toBeChecked()
await expect(
page.getByRole("checkbox", { name: checkbox, exact: true })
).toBeDisabled()
}

await assertCheckboxStatus(page, "commercial", "unchecked")
await expect(byNc).not.toBeDisabled()
for (const checkbox of ["commercial", "by-nc-sa", "by-nc-nd"]) {
await assertCheckboxStatus(page, checkbox, "unchecked")
for (const checkbox of commercialLicenses) {
await expect(
page.getByRole("checkbox", { name: checkbox, exact: true })
).not.toBeChecked()
await expect(
page.getByRole("checkbox", { name: checkbox, exact: true })
).toBeEnabled()
}
})

Expand All @@ -153,23 +197,26 @@ breakpoints.describeMobileAndDesktop(() => {
await page.goto("/search/image?q=cat&aspect_ratio=tall&license=cc0")
await filters.open(page)

await assertCheckboxStatus(page, "tall")
await assertCheckboxStatus(page, "cc0")
await expect(page.getByRole("checkbox", { name: "Tall" })).toBeChecked()
await expect(page.getByRole("checkbox", { name: "Zero" })).toBeChecked()

await changeSearchType(page, AUDIO)
await filters.open(page)

// Only CC0 checkbox is checked, and the filter button label is
// '1 Filter' on `xl` or '1' on `lg` screens
await assertCheckboxStatus(page, "cc0")
await expect(page.getByRole("checkbox", { name: "Zero" })).toBeChecked()

await filters.close(page)
// eslint-disable-next-line playwright/no-conditional-in-test
if (isPageDesktop(page)) {
const filterButtonText = await page
.locator('[aria-controls="filters"] span:visible')
.textContent()
expect(filterButtonText).toContain("Filters")
} else {
const filtersAriaLabel =
// eslint-disable-next-line playwright/no-conditional-in-test
(await page
.locator('[aria-controls="content-settings-modal"]')
.getAttribute("aria-label")) ?? ""
Expand All @@ -185,18 +232,20 @@ breakpoints.describeMobileAndDesktop(() => {
await page.goto("/search/image?q=cat")
await filters.open(page)

await assertCheckboxStatus(page, "cc0", "unchecked")
await expect(page.getByRole("checkbox", { name: "Zero" })).not.toBeChecked()

const [response] = await Promise.all([
page.waitForResponse((response) => response.url().includes("cc0")),
page.click('label:has-text("CC0")'),
])
// Alternative way with a predicate. Note no await.
const responsePromise = page.waitForResponse(
(response) =>
response.url().includes("/images/") && response.status() === 200
)
await page.getByLabel("Zero").click()
const response = await responsePromise

await assertCheckboxStatus(page, "cc0")
await expect(page.getByRole("checkbox", { name: "Zero" })).toBeChecked()
// Remove the host url and path because when proxied, the 'http://localhost:49153' is used instead of the
// real API url
const queryString = response.url().split("/images/")[1]
expect(queryString).toEqual("?q=cat&license=cc0")
expect(response.url()).toContain("?q=cat&license=cc0")
})

for (const [searchType, source] of [
Expand All @@ -211,7 +260,7 @@ breakpoints.describeMobileAndDesktop(() => {
)
await filters.open(page)

await assertCheckboxStatus(page, source, "checked")
await expect(page.getByRole("checkbox", { name: source })).toBeChecked()
})
}
})
2 changes: 1 addition & 1 deletion frontend/test/playwright/e2e/global-audio.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ test.describe("Global Audio", () => {
.getByRole("button", { name: t("audioTrack.close") })
.click()
// and confirm the player is not visible
await expect(page.locator(".global-audio")).not.toBeVisible()
await expect(page.locator(".global-audio")).toBeHidden()
})

test("player does not reproduce an audio different that the current audio in the details page", async ({
Expand Down
2 changes: 1 addition & 1 deletion frontend/test/playwright/e2e/homepage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const searchTypePopover = "[aria-labelledby='search-type-button'] > div"
const popoverIsVisible = async (page: Page) =>
await expect(page.locator(searchTypePopover)).toBeVisible()
const popoverIsNotVisible = async (page: Page) =>
await expect(page.locator(searchTypePopover)).not.toBeVisible()
await expect(page.locator(searchTypePopover)).toBeHidden()
const clickPopoverButton = async (page: Page) =>
await page.getByRole("button", { name: t("searchType.all") }).click()

Expand Down
4 changes: 1 addition & 3 deletions frontend/test/playwright/e2e/image-detail.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ test("shows the main image with its title as alt text", async ({ page }) => {

test("does not show back to search results breadcrumb", async ({ page }) => {
await goToCustomImagePage(page)
await expect(
page.locator(`text="${t("singleResult.back")}"`)
).not.toBeVisible({
await expect(page.locator(`text="${t("singleResult.back")}"`)).toBeHidden({
timeout: 300,
})
})
Expand Down
Loading

0 comments on commit dfc9635

Please sign in to comment.