Improve toEqualText error message
#73
Conversation
| const result = await toEqualText(iframe!, "h1", "Example Domain") | ||
| expect(result.pass).toBe(true) | ||
| expect(result.message()).toMatchSnapshot() | ||
| await expect(iframe!).toEqualText("h1", "Example Domain") |
There was a problem hiding this comment.
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.
Codecov Report
@@ Coverage Diff @@
## master #73 +/- ##
==========================================
- Coverage 99.37% 91.61% -7.77%
==========================================
Files 10 10
Lines 161 155 -6
Branches 27 27
==========================================
- Hits 160 142 -18
- Misses 1 13 +12
Continue to review full report at Codecov.
|
| val === null ? "null" : `'${val}'` | ||
|
|
||
| export const getMessage = ( | ||
| { isNot, promise, utils }: jest.MatcherContext, |
There was a problem hiding this comment.
We also gave the user ability to use expect-playwright without jest. And don't we get any errors in this case?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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:
The TS definitions would need updating which looks similar to jest:
declare namespace folio {
interface Matchers<R> extends PlaywrightMatchers<R> {}
}There was a problem hiding this comment.
Looks good, regrading types, in the new folio version which gets released soon it will automatically pick up our existing Jest extend types.
README.md
Outdated
| ### With Playwright test runner | ||
|
|
||
| ```javascript | ||
| import expect from "expect-playwright" | ||
| To activate with the Playwright test runner, use `expect.extend` to add the `expect-playwright` matchers. | ||
|
|
||
| await expect(page).toHaveText("#foo", "my text") | ||
| ```js | ||
| // folio.config.ts | ||
| import { expect } from "@playwright/test" | ||
| import matchers from "expect-playwright" | ||
|
|
||
| expect.extend(matchers) |
There was a problem hiding this comment.
@mxschmitt I updated the readme with a sample of what the docs could look like. I'm not real familiar with folio so this might very well need to be adjusted on how the next release will work.
There was a problem hiding this comment.
I would have waited with folio since there is a big breaking change incoming there soon, but we can also update it then.
There was a problem hiding this comment.
@mxschmitt I'm fine with waiting on the folio update before moving this forward. That way it's one change, and it will continue to work for folio users in the mean time as is. This PR isn't impeding any progress since it's just a quality of life change that doesn't impact functionality of the matchers themselves.
Do you have an estimate on when the folio breaking change will land so I can keep tabs and update this PR accordingly?
There was a problem hiding this comment.
This will probably take a few weeks and the users are afaik mostly using it with Jest. So I think we can merge the PR earlier, let me know when you feel ready, then I can merge it.
There was a problem hiding this comment.
@mxschmitt Sounds good, in that case this is ready to go. I did do one final change of exporting the matchers as a non-default export to be less confusing since the old default export was the custom expect wrapper.
|
@mskelton thanks a lot for your hard work! |
… default export `expect`
Related to #69
This PR updates the error message for
toEqualTextto have a more clean and readable output and to match better with Jest's built-in matchers. Just doing this matcher for now so we can sort out the format and everything and after this is merged I'll follow up to update the rest of the matchers.Examples
Using
.toEqualTextUsing
.not.toEqualTextWith folio