Extend testing guidelines #38
Conversation
This PR proposes a new set of guidelines for writing unit tests across our projects. Most are new, or if they are followed to some degree, they have not yet been codified.
mcmire
force-pushed
the
add-more-testing-guidelines
branch
from
9e62285
to
7015953
Compare
guides/unit-testing.md
Outdated
| }) | ||
| ``` | ||
|
|
||
| ## Use Jest's mock functions instead of Sinon |
There was a problem hiding this comment.
This is a corollary of "Use Jest", but I chose to call it out because we have a lot of older tests that were presumably written before the conversion to Jest and use Sinon.
guides/unit-testing.md
Outdated
| }); | ||
| ``` | ||
|
|
||
| ## Use `it` to specify known behavior for a unit of code |
There was a problem hiding this comment.
Originally I had a section which explained the purpose of test names and why they're arranged using describe and it. Essentially, you can draw a straight line from Jest to the birth of behavior-driven development. However, BDD is a subject unto itself, and I didn't want to overload people with information.
guides/unit-testing.md
Outdated
|
|
||
| Do not use "should" at the start of the test name. The official Jest documentation [omits this word from their examples](https://jestjs.io/docs/next/getting-started), and it creates noise when reviewing the list of tests that Jest outputs after a run. | ||
|
|
||
| Do not repeat the name of the function or method in the name of the test if it is possible to communicate what a consumer expects more clearly. |
There was a problem hiding this comment.
There's another bit of advice that could go here, which is to be aware of scenarios — changes in the context which affect the behavior of the thing under test — and call them out in the test name. That probably deserves its own guideline, however, and I've already written a lot 😅
guides/unit-testing.md
Outdated
| 2️⃣ | ||
|
|
||
| ``` typescript | ||
| describe('if "addToken" is true', () => { |
There was a problem hiding this comment.
Nit: I think it would be better to encourage people to group tests by scenario, rather than by other similar factors. The description of this guideline kinda implies that already, but this example isn't as clear because this isn't written like a scenario.
Maybe a description like "when adding a token" would be better here? That might prompt people to think more about the purpose of the tests and the real scenarios they represent, rather than just on inputs and outputs.
There was a problem hiding this comment.
Good point. I've updated this guideline here: 1decf7b
- Remove arguments for non-determinism - Make sub-guidelines more concise and hide details
@Mrtenz I'm going to be off for a while starting next week so I'm wondering if it makes sense to add this in a new PR? Plus this PR is pretty big 😅 |
|
I think I've addressed all the feedback above. I've made a few more tweaks along the way, so let me know if there's anything that doesn't make sense or could be simplified. |
This PR proposes a new set of guidelines for writing unit tests across our projects. Most are new, or if they are followed to some degree, they have not yet been codified.
Fixes #25.
👉 View rendered document 👈