[NextJS] Fix tests, re-enable on CI #1560
Conversation
| ) | ||
| return ( | ||
| totalSubtopics > 0 && ( | ||
| <SubTopicsContainer> |
| await waitFor(() => { | ||
| expect(document.title).toBe("About Us | MIT Learn") | ||
| }) |
There was a problem hiding this comment.
I removed this and some other metadata tests since the metadata is now handled by NextJS.
It probably is worth having metadata tests, but they should be added in https://github.com/mitodl/hq/issues/5410
| import NotFoundPage from "./NotFoundPage" | ||
|
|
||
| test("The NotFoundPage loads with meta", async () => { | ||
| test.skip("The NotFoundPage loads with meta", async () => { |
There was a problem hiding this comment.
This is the only test to which I added a skip, since it will be changed anyway in [ISSUE TO BE CREATED]
| params: Record<string, string | number>, | ||
| ): string => { | ||
| return template.replace(/:(\w+)/g, (_, key) => { | ||
| return template.replace(/\[(\w+)\]/g, (_, key) => { |
There was a problem hiding this comment.
Marking params as with [...] is much more convenient for use with next-router-mock... see https://github.com/mitodl/mit-open/pull/1560/files#diff-22d69d0f979fef7f8d9d0b0cd55dd7a00c6c3f4302ac7d542f79220acee4f905R18
|
|
||
| export const LEARNINGPATH_LISTING = "/learningpaths/" | ||
| export const LEARNINGPATH_VIEW = "/learningpaths/:id" | ||
| export const LEARNINGPATH_LISTING = "/learningpaths" |
There was a problem hiding this comment.
NextJS by default canonicalizes removes trailing / from paths.
If we prefer to keep them, we can configure that with next.config.js (which affects the behavior of NextJS server and of the Link component). But it's slightly annoying since the jest tests aren't aware of next.config.js.
|
|
||
| const { usePathname, useSearchParams } = nextNavigationMocks | ||
|
|
||
| describe("Mock Navigation", () => { |
There was a problem hiding this comment.
Added for sanity as I debugged an issue with the mock navigation stuff.
gumaerc
left a comment
gumaerc
left a comment
There was a problem hiding this comment.
👍
Looks good, just one small nitpick:
| @@ -1,16 +1,18 @@ | |||
| import { renderTestApp, waitFor } from "../../test-utils" | |||
| import React from "react" | |||
| import { renderWithProviders, waitFor } from "../../../../test-utils" | |||
There was a problem hiding this comment.
Kind of a nitpick, but can't this just be @/test-utils instead of the relative path?
ChristopherChudzicki
force-pushed
the
nextjs
branch
from
6d0fbfb to
e8d66d5
Compare
ChristopherChudzicki
force-pushed
the
cc/nextjs-remaining-tests
branch
from
e1dd9ae to
4d596ef
Compare
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/5414
Description (What does it do?)
This fixes our Jest tests for NextJS branch and re-enables them on CI
How can this be tested?
docker compose uporyarn watch. It should look good.