chore: Add initial jest tests + code coverage #13
Conversation
bryphe-coder
force-pushed
the
bryphe/chore/jest-tests
branch
from
1add80d
to
89c44a5
Compare
Codecov Report
@@ Coverage Diff @@
## main #13 +/- ##
==========================================
- Coverage 70.92% 70.86% -0.07%
==========================================
Files 19 37 +18
Lines 1173 1311 +138
Branches 0 7 +7
==========================================
+ Hits 832 929 +97
- Misses 269 306 +37
- Partials 72 76 +4
Continue to review full report at Codecov.
|
| }) | ||
|
|
||
| it("is called when clicking option in pop-up", () => { | ||
| // Given |
There was a problem hiding this comment.
Are we confident all tests will follow this pattern? What if there are subsequent given after a when?
I'm hesitant to say we should have these comments in all tests, because I'm not sure how else you'd write the logic anyways.
There was a problem hiding this comment.
This is a structure we've been using on the front-end (I believe @vapurrmaid introduced it) - in fact one of my first PRs had me add this to tests: https://github.com/coder/m/pull/9720/files#r697678791
Some tests have an extra given / when / then - @vapurrmaid would know some examples of that. Might be nice to have this documented in a front-end code standards page on Notion (our current standards are pretty empty)
There was a problem hiding this comment.
^ This is a fairly standard pattern.
You're welcome to follow up whens and thens
ie:
// Given - all your givens
// When - thing
// Then - outcome
// When - other thing
// Then - other outcome
site/components/Page/index.test.tsx
Outdated
| import { render } from "../../test_helpers" | ||
| import { Page } from "./index" | ||
|
|
||
| describe("Page", () => { |
There was a problem hiding this comment.
I probably should have reviewed the other PR more thoroughly, but I'm hesitant to have a Page component, because if it were truly to be used on all pages, it would be idiomatic to put it in _app.tsx instead.
There was a problem hiding this comment.
Ah good point, that <Page /> was leftover before I moved to next. I'll port this over to our _app part of this change.
There was a problem hiding this comment.
Removed the <Page /> component here: 7b50658
I still left the Page folder to contain some helper stuff for the page (like the <Footer />) - but we can revisit that folder as we start building out the UI.
Depends on #8
jest,ts-jest,jest.config.js, etc)TODO:
codecov