-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
testing: search page component test [1393] #2189
Conversation
@@ -3,4 +3,4 @@ | |||
/// <reference types="next/navigation-types/compat/navigation" /> | |||
|
|||
// NOTE: This file should not be edited | |||
// see https://nextjs.org/docs/basic-features/typescript for more information. | |||
// see https://nextjs.org/docs/app/building-your-application/configuring/typescript for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this change occurred automatically when I started up a local server, not entirely sure why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get the same. It is a doc link update so I think we can just add it in this PR.
da43d87
to
73e7f39
Compare
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
---|---|---|---|
🟢 | Statements | 88.43% (-1.24% 🔻) |
917/1037 |
🟡 | Branches | 73.46% (+1.66% 🔼) |
238/324 |
🟢 | Functions | 84.11% (-2.46% 🔻) |
180/214 |
🟢 | Lines | 88.64% (-1.45% 🔻) |
858/968 |
Show new covered files 🐣
St.❔ |
File | Statements | Branches | Functions | Lines |
---|---|---|---|---|---|
🟢 | ... / page.tsx |
88.89% | 100% | 50% | 88.57% |
🟢 | ... / loading.tsx |
100% | 100% | 100% | 100% |
🟢 | ... / convertSearchParamsToProperTypes.ts |
88.89% | 87.5% | 100% | 87.5% |
🟢 | ... / SearchCallToAction.tsx |
100% | 100% | 100% | 100% |
🟢 | ... / SearchFilterOptions.ts |
100% | 100% | 100% | 100% |
🔴 | ... / SearchPaginationFetch.tsx |
42.86% | 100% | 0% | 42.86% |
🔴 | ... / SearchResultsHeaderFetch.tsx |
50% | 100% | 0% | 50% |
🔴 | ... / SearchResultsListFetch.tsx |
38.46% | 0% | 0% | 38.46% |
🔴 | ... / MockSearchFetcher.ts |
42.86% | 100% | 0% | 50% |
🟢 | ... / SearchFetcher.ts |
100% | 100% | 100% | 100% |
Test suite run success
188 tests passing in 59 suites.
Report generated by 🧪jest coverage report action from f9ceacc
); | ||
|
||
// test without i18n functionality, pass through translation key as text | ||
jest.mock("next-intl/server", () => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mocking these is maybe what we were missing with the page tests before! We could re-add the page tests: https://github.com/HHS/simpler-grants-gov/tree/2024.5.13-1/frontend/tests/pages if that is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the tests in /dev and /api seem to be working. Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nm, I see that the branch you linked to has more tests in it - I can take a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
status: "forecasted,posted", | ||
}; | ||
|
||
render(<Search searchParams={mockSearchParams} />); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add more scenarios to test sorting, paging, input component etc in a later ticket or PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe? the potential issue with all the mocking I've done to get this working is that the dynamic behavior we get from things like useRouter
or API requests are getting lost at the moment. I can play around with what's possible, but I am curious as to the team's philosophy regarding the purpose of component unit tests vs. route level unit (?) tests vs. end to end integration tests and what level of functionality we want to test where. In my mind it could be a better use of time to thoroughly test things in the lower level components and integration tests - which it looks like we already have - and zeroing in on what the purpose of the page level tests like this are doing that is not captured elsewhere
498465d
to
f9ceacc
Compare
Summary
Fixes #1393
Time to review: 30 mins
Changes proposed
Context for reviewers
The intention is for this work to form the basis of a conversation about testing higher level pages / routes that are or contain server components. I made a number of decisions around testing behavior here that should probably be discussed and agreed upon with the team, as examples of testing approaches in this case, before moving forward.
Work was based somewhat off of previous work here main...rylew/1393-server-component-tests
Test steps
frontend
directory of the projectnpx jest tests/pages/search/pages.test.tsx
Additional information