Added Custom 404 Page.

[Adi-204]

Closes: #1752

---

📚 Documentation preview 📚: https://bowtie-json-schema--1753.org.readthedocs.build/en/1753/

[Julian]

Thanks. Any chance you know how to add a test for this? Something which just directly browses to a route that doesn't exist and then asserts the component being shown is the not found component?

[Adi-204]

I have not previously worked on writing test cases in React, but I am willing to study and implement them as needed. If the implementation does not go as expected, I will remove the test cases and provide a report accordingly.

[Julian]

I'm not sure I see a reason for the describe.each -- I can't think of a reason we'd need a second test here, there's one single behavior (visiting a non-existing route) that we're testing.
So I'd just inline the "array" and create a single normal test.

[Julian]

I don't think this test is good. If I understand it correctly, what you've done is set up a new router which always renders the NotFound component, and then browsed to some path (/random-path -- but even / would have the same behavior!) and asserted the 404 text is shown.

That isn't useful.

You should test our application renders the NotFound component on a route not known by our application, otherwise we're testing something completely unconnected to our application, which could be entirely broken (say if you forgot to make the change to index.tsx -- this test would still pass).

[Adi-204]

Thank you for guidance.

Should I make following changes -

Approach -
Case 1: If a NotFound route (path: "*") is defined in index.tsx, it should render correctly, and the test should pass when it finds an h1 " with "404 Not Found".
Case 2: If no NotFound route is defined, React Router’s default error boundary will handle the unknown route, but it may not include an h1. The test will fail since it expects h1 to exist.

Source - https://testing-library.com/docs/example-react-router/

[Julian]

That looks like it has the same issue as your current test, you create a new (memory) router instead of using our real one.
(I'm not familiar with that page, but it seems a bit strange as a recommendation.)
The "real" solution here is likely we should be testing through playwright as an end-to-end test.
But to keep the work minimal, I haven't tried it, but I would assume your test should work just fine if you import our real hash router which is what we care about the behavior of, and request against it? At least that should be the first thing tried.

[Adi-204]

I have imported the router from index.tsx (line no. 4) and used the spread operator to copy all paths from there, setting the entry point as /random-path. So new (memory) router is used to set initialEntry point of website when all routes are present in route.
If the following code is missing in the router, the test case will fail:
{
path: "*",
Component: NotFound,
},
However, if this code is present, the test case will pass.

Are you telling that we should test app on all other existing paths as well ?

[Julian]

I have imported the router from index.tsx (line no. 4) and used the spread operator to copy all paths from there, setting the entry point as /random-path. So new (memory) router is used to set initialEntry point of website when all routes are present in route.

I see, sorry, it's hard to review screenshots.

I don't yet understand what the point of that is though, what goes wrong if you literally pass RouterProvider router={ourHashRouter}?

[Adi-204]

If we render without specifying an initial path, the test framework will not know which route to check. To set the initial route, we need to use createMemoryRouter, which allows us to define initialEntries.
This ensures that when testing for /random-path, the router will not find a matching route, triggering the wildcard (*) path and rendering the NotFound component. The test can then verify that the expected content is present in the rendered output.

[Julian]

I don't follow why the test framework needs an initial route? The route we check should be one you browse to, i.e. /random-path. https://test-utils.vuejs.org/guide/advanced/vue-router perhaps seems like a relevant page, what I'm talking about is the "test with the real router" bits described there.

[Julian]

If after reading that page it isn't obvious how to write this test I'll make some time in the next few days to figure it out.

[Adi-204]

I explored alternative ways to test routing in React but found that createMemoryRouter is the most suitable approach. However, I couldn't find an equivalent method similar to Vue. Given this, should I remove the test implementation and proceed with committing only the addition of the 404 Not Found page? Also, could you please merge only this change?

[Julian]

I don't plan to merge this without tests (all PRs merged should have tests!). But I'm happy to try to make time to figure out how to best test this if you think what you have is the best you've seen.

[Adi-204]

Thanks, I will also try to find a better solution.

[Adi-204]

@Julian just wanted to ask the status of the tests for the above. Let me know if you need any help or input from my side.

[Julian]

I made some time to do the work to add support for Playwright tests and moved your test over to that. Thanks for the PR, appreciated.