Add 404 assertion to e2e error-boundary

[SanderKoopmans]

I noticed a // todo: figure out which seemed a nice little challenge. Add a listener to verify the 404

Test Plan

• Checkout branch
• Run e2e tests

Checklist

• [ x ] Tests updated
• [ n/a ] Docs updated

[swalker326]

Thats clever, well done.

[kentcdodds]

Thanks! Just one thing.

[kentcdodds]

With assertions that happen asynchronously like this I always like to make certain that they can fail. I'm concerned that as it's currently written, playwright will think the test is over before this assertion runs. Can you verify that the test can fail? https://kentcdodds.com/blog/make-your-test-fail

[SanderKoopmans]

Ha excellent, I'll see if I can break the test tomorrow.

…

On Mon, 30 Oct 2023, 22:26 Kent C. Dodds, ***@***.***> wrote: ***@***.**** commented on this pull request. Thanks! Just one thing. ------------------------------ In tests/e2e/error-boundary.test.ts <#506 (comment)> : > await expect(page.getByText(/We can't find this page/i)).toBeVisible() - // TODO: figure out how to assert the 404 status code + + const listener = (request: Request) => { + if (request.url().includes(pageUrl)) { + request.response().then(response => expect(response?.status()).toBe(404)) With assertions that happen asynchronously like this I always like to make certain that they can fail. I'm concerned that as it's currently written, playwright will think the test is over before this assertion runs. Can you verify that the test can fail? https://kentcdodds.com/blog/make-your-test-fail — Reply to this email directly, view it on GitHub <#506 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKELXJ6HXC753KUPT7HFYIDYCALQXAVCNFSM6AAAAAA6WVSVYWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMBVGEYDENRYGA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[caprolactam]

I write like this in my code. Please share what is the problem here.

test('Test root error boundary caught', async ({ page }) => {
  const res = await page.goto('/does-not-exist')

  expect(res?.status()).toBe(404)
  await expect(page.getByText(/We can't find this page/i)).toBeVisible()
})

ref

[kentcdodds]

Ah, yes I definitely prefer that!

[SanderKoopmans]

Not sure if/why I didn't try that, suggestion applied. Another todo done ✅

[kentcdodds]

Super! And have you verified that it can fail?

[SanderKoopmans]

It fails as soon as someone changes the return status in the loader from routes/$.tsx. Second part fails as soon as someone changes the statusHandlers from the error boundary.

Is there anything else I should verify? ☺️

[kentcdodds]

Great! That's enough. Thank you!