Refactor tests to use test cases #7
Conversation
Using test cases provides a better DX. - It allows running multiple tests - It shows which failed / succeeded - It enables test filtering - Assertion messages hid diffs in `node:test` A describe block was added for the function that’s being tested.
remcohaszing
added
🕸️ area/tests
|
Why describe? Why not nest tests?
I don’t understand this? Can you repeat in different words? |
|
Tests with nested tests are still tests. Nested tests need to finish before their parent, so the parent must await the import assert from 'node:assert/strict'
import {describe, test} from 'node:test'
import {setTimeout} from 'node:timers/promises'
describe('describe', () => {
let value
test('test 1', async () => {
value = 1
await setTimeout(1000)
assert.equal(value, 1)
})
test('tes 2', async () => {
value = 2
assert.equal(value, 2)
})
})
test('nested test', (t) => {
let value
t.test('t.test 1', async () => {
value = 1
await setTimeout(1000)
assert.equal(value, 1)
})
t.test('t.test2', async () => {
value = 2
assert.equal(value, 2)
})
})yields
This assertion without a message import assert from 'node:assert/strict'
import {test} from 'node:test'
test('test', () => {
assert.deepEqual({foo: 1}, {bar: 2})
})yields This assertion with a message import assert from 'node:assert/strict'
import {test} from 'node:test'
test('test', () => {
assert.deepEqual({foo: 1}, {bar: 2}, 'objects should be equal')
})yields (the diff is gone) IMO the former is much more useful. |
My point is that I dunno:
The output of Node’s test runner is TAP. That means that you can use formatters, such as for example 
So it’s possible to access the diff. As for moving the messages from |
|
So:
|
I don’t think I could:
Whatever you prefer.
I think it’s useful to always keep tests focused to testing one thing. This way one failure doesn’t obscure other test results.
It was TAP. Now it’s TAP for non-interactive prompts, but a more human-readable format for interactive prompts. I imagine you might have missed this, because you always pipe the output? :) |
Why did you not go with
And I think that’s very useful here, but for utilities that are simple and have 8 assertions, I feel like it doesn‘t add much, feels a bit like premature optimization, it’s nice but it’s also a lot of lines. I’m noting it more because I don’t think this style introduced here needs to be introduced everywhere. Or do you think that’s the case?
Potato potato for me, I do not always pipe, I pipe when I want to diff things! |
Going with But I’m not trying to convince you. I can switch to
I do think it’s nice to apply this pattern everywhere. It just takes 2 extra lines, one to call |
|
Those two conflict 😅 I went with no nesting.
Definitely! I only changed this, because it bugged my while implementing something else. Same thing goes for example for updating dev dependencies. |
I don‘t see how, you don‘t have describe anymore, which is what I like :) |
This comment has been minimized.
This comment has been minimized.
|
Thanks! Merged quickly because it sounds like you want to do more work on this project |
Initial checklist
Description of changes
Using test cases provides a better DX.
node:testA describe block was added for the function that’s being tested. I tend to like this if multiple functions are exported. In this case, where only one function is exported, it’s a bit less useful IMO.
No actual test logic was changed. Most changes are related to indentation.