Skip to content
This repository has been archived by the owner on Aug 20, 2024. It is now read-only.

Improve toEqualText error message #73

Merged
merged 17 commits into from
May 23, 2021
4 changes: 4 additions & 0 deletions .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ jobs:
npx prettier --check .
- run: npm run build
- run: npm test
env:
# Used by chalk. Ensures output from Jest includes ANSI escape
# characters that are needed to match test snapshots.
FORCE_COLOR: true
- run: npx codecov
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
4 changes: 4 additions & 0 deletions src/matchers/tests/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@ export const testWrapper = (result: SyncExpectationResult) => {
throw new Error(result.message())
}
}

export const assertSnapshot = async (fn: () => Promise<void>) => {
await expect(fn).rejects.toThrowErrorMatchingSnapshot()
}
27 changes: 24 additions & 3 deletions src/matchers/toEqualText/__snapshots__/index.test.ts.snap
Original file line number Diff line number Diff line change
@@ -1,9 +1,30 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`toEqualText selector negative 1`] = `"'not-existing' does not equal 'zzzBarzzz' of '#foobar'."`;
exports[`toEqualText element failure 1`] = `
"expect(received).toEqualText(expected)

exports[`toEqualText selector positive 1`] = `"'Bar' does equal 'Bar'."`;
Expected: \\"not-existing\\"
Received: \\"zzzBarzzz\\""
`;

exports[`toEqualText selector positive frame 1`] = `"'Example Domain' does equal 'Example Domain'."`;
exports[`toEqualText page failure 1`] = `
"expect(received).toEqualText(expected)

Expected: \\"not-existing\\"
Received: \\"zzzBarzzz\\""
`;

exports[`toEqualText selector negative 1`] = `
"expect(received).toEqualText(expected)

Expected: \\"Bar\\"
Received: \\"zzzBarzzz\\""
`;

exports[`toEqualText selector not failure 1`] = `
"expect(received).not.toEqualText(expected)

Expected: not \\"Bar\\""
`;

exports[`toEqualText selector timeout should throw an error after the timeout exceeds 1`] = `"Error: Timeout exceed for element '#foobar'"`;
74 changes: 38 additions & 36 deletions src/matchers/toEqualText/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { testWrapper } from "../tests/utils"

import toEqualText from "."
import { assertSnapshot } from "../tests/utils"

expect.extend({ toEqualText })

describe("toEqualText", () => {
afterEach(async () => {
Expand All @@ -10,71 +11,72 @@ describe("toEqualText", () => {
it("positive frame", async () => {
await page.setContent(`<iframe src="https://example.com"></iframe>`)
const iframe = await page.$("iframe")
const result = await toEqualText(iframe!, "h1", "Example Domain")
expect(result.pass).toBe(true)
expect(result.message()).toMatchSnapshot()
await expect(iframe!).toEqualText("h1", "Example Domain")
Copy link
Member Author

Choose a reason for hiding this comment

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

I updated the tests to use the matcher directly in the test rather than testing the results of it. The reason for this is that the matcher is now using more of jest.MatcherContext such as printExpected, matcherHint, etc.

})
it("positive", async () => {
await page.setContent(`<div id="foobar">Bar</div>`)
const result = await toEqualText(page, "#foobar", "Bar")
expect(result.pass).toBe(true)
expect(result.message()).toMatchSnapshot()
await expect(page).toEqualText("#foobar", "Bar")
})
it("negative", async () => {
await page.setContent(`<div id="foobar">zzzBarzzz</div>`)
expect(
testWrapper(await toEqualText(page, "#foobar", "not-existing"))
).toThrowErrorMatchingSnapshot()
await assertSnapshot(() => expect(page).toEqualText("#foobar", "Bar"))
})
describe("not", () => {
it("success in frame", async () => {
mskelton marked this conversation as resolved.
Show resolved Hide resolved
await page.setContent(`<iframe src="https://example.com"></iframe>`)
const iframe = await page.$("iframe")
await expect(iframe!).not.toEqualText("h1", "Foo")
})
it("success", async () => {
await page.setContent(`<div id="foobar">Bar</div>`)
await expect(page).not.toEqualText("#foobar", "Foo")
})
it("failure", async () => {
await page.setContent(`<div id="foobar">Bar</div>`)
await assertSnapshot(() =>
expect(page).not.toEqualText("#foobar", "Bar")
)
})
})
describe("timeout", () => {
it("positive: should be able to use a custom timeout", async () => {
it("success with a custom timeout", async () => {
setTimeout(async () => {
await page.setContent(`<div id="foobar">Bar</div>`)
}, 500)
expect(testWrapper(await toEqualText(page, "#foobar", "Bar"))).toBe(
true
)
await expect(page).toEqualText("#foobar", "Bar")
})
it("should throw an error after the timeout exceeds", async () => {
const start = new Date().getTime()
expect(
testWrapper(
await toEqualText(page, "#foobar", "Bar", {
timeout: 1 * 1000,
})
)
).toThrowErrorMatchingSnapshot()
await assertSnapshot(() =>
expect(page).toEqualText("#foobar", "Bar", { timeout: 1 * 1000 })
)
const duration = new Date().getTime() - start
expect(duration).toBeLessThan(1500)
})
})
})
describe("element", () => {
it("positive", async () => {
it("success", async () => {
await page.setContent(`<div id="foobar">Bar</div>`)
const element = await page.$("#foobar")
expect(element).not.toBe(null)
expect(testWrapper(await toEqualText(element!, "Bar"))).toBe(true)
expect(element).not.toBeNull()
await expect(element!).toEqualText("Bar")
})
it("negative", async () => {
it("failure", async () => {
await page.setContent(`<div id="foobar">zzzBarzzz</div>`)
const element = await page.$("#foobar")
expect(element).not.toBe(null)
expect(
testWrapper(await toEqualText(element!, "not-existing"))
).toThrowError()
expect(element).not.toBeNull()
await assertSnapshot(() => expect(element!).toEqualText("not-existing"))
})
})
describe("page", () => {
it("positive", async () => {
it("success", async () => {
await page.setContent(`<body><div>Bar</div></body>`)
expect(testWrapper(await toEqualText(page, "Bar"))).toBe(true)
await expect(page).toEqualText("Bar")
})
it("negative", async () => {
it("failure", async () => {
await page.setContent(`<body><div>zzzBarzzz</div></body>`)
expect(
testWrapper(await toEqualText(page, "not-existing"))
).toThrowError()
await assertSnapshot(() => expect(page).toEqualText("not-existing"))
})
})
})
26 changes: 8 additions & 18 deletions src/matchers/toEqualText/index.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,20 @@
import { SyncExpectationResult } from "expect/build/types"
import { getElementText, quote, InputArguments } from "../utils"
import { getElementText, getMessage, InputArguments } from "../utils"

const toEqualText = async (
const toEqualText: jest.CustomMatcher = async function (
...args: InputArguments
): Promise<SyncExpectationResult> => {
): Promise<SyncExpectationResult> {
try {
const { elementHandle, selector, expectedValue } = await getElementText(
...args
)
const { elementHandle, expectedValue } = await getElementText(...args)
/* istanbul ignore next */
const actualTextContent = await elementHandle.evaluate<string | null>(
const actualTextContent = await elementHandle.evaluate(
(el) => el.textContent
)
if (actualTextContent === expectedValue) {
return {
pass: true,
message: () =>
`${quote(expectedValue)} does equal ${quote(actualTextContent)}.`,
}
}

return {
pass: false,
pass: actualTextContent === expectedValue,
message: () =>
`${quote(expectedValue)} does not equal ${quote(actualTextContent)}${
selector ? " of " + quote(selector) + "." : "."
}`,
getMessage(this, "toEqualText", expectedValue, actualTextContent),
}
} catch (err) {
return {
Expand Down
24 changes: 23 additions & 1 deletion src/matchers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,26 @@ export const getElementText = async (
throw new Error(`Invalid input length: ${args.length}`)
}

export const quote = (val: string | null) => `'${val}'`
export const quote = (val: string | null) =>
mskelton marked this conversation as resolved.
Show resolved Hide resolved
val === null ? "null" : `'${val}'`

export const getMessage = (
{ isNot, promise, utils }: jest.MatcherContext,
Copy link
Member

Choose a reason for hiding this comment

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

We also gave the user ability to use expect-playwright without jest. And don't we get any errors in this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting, I didn't realize until now that was an option (glossed over it in the readme). What are the use cases for using it without Jest?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We allow it to use it in https://github.com/microsoft/playwright-test since its based on https://github.com/microsoft/folio. So we support the use-case of using it fully standalone or with the http://npmjs.com/package/expect library which is also used in folio.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry for not responding sooner, got sidetracked with other priorities at work. My plan is to do some additional testing with folio later this week.

Copy link
Member

Choose a reason for hiding this comment

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

idk but i think that we will need the check if jest is available, and there is no jest - just return regular message, just how we do earlier

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, so I had a chance to test this out. Turns out the utils I'm using in this PR are available when using expect.extend but not the custom expect wrapper that this library is exporting. If we could update to use a strategy more like this code:

import { it, expect } from "@playwright/test";
import matchers from "expect-playwright/lib/matchers";

expect.extend(matchers);

it("is a basic test with the page", async ({ page }) => {
  await page.goto("https://playwright.dev/");
  await expect(page).toEqualText("h3", "Playwrights");
});

Then it would actually work. Here is what that looks like:

Screen Shot 2021-05-20 at 3 49 44 PM

The TS definitions would need updating which looks similar to jest:

declare namespace folio {
  interface Matchers<R> extends PlaywrightMatchers<R> {}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good, regrading types, in the new folio version which gets released soon it will automatically pick up our existing Jest extend types.

matcher: string,
expected: string | null,
received: string | null
) => {
const matcherHint =
utils.matcherHint(matcher, undefined, undefined, { isNot, promise }) +
"\n\n"

if (isNot) {
return matcherHint + `Expected: not ${utils.printExpected(expected)}`
} else {
return (
matcherHint +
`Expected: ${utils.printExpected(expected)}\n` +
`Received: ${utils.printReceived(received)}`
)
}
mskelton marked this conversation as resolved.
Show resolved Hide resolved
}