Allow new assertions while waiting for pending ones #1331
Conversation
Whilst not a great way to write tests, users may end up adding more assertions from inside a promise chain that's passed to the `t.throws()` or `t.notThrows()` assertions. These should be counted for the plan, and not fail the test as being extraneous. Fixes #1327.
| this.saveFirstError(new Error('Assertion passed, but test has already ended')); | ||
| } | ||
|
|
||
| this.assertCount++; | ||
| } | ||
|
|
||
| addPendingAssertion(promise) { | ||
| if (this.finishing) { | ||
| if (this.finished) { | ||
| this.saveFirstError(new Error('Assertion passed, but test has already ended')); |
There was a problem hiding this comment.
I realize this is not from this PR, but a pending assertion technically hasn't passed yet. This message should be different.
There was a problem hiding this comment.
I considered that when I first wrote it, but the nuance seemed superfluous. And since #1330 it's not even shown anymore.
| // Retry finishing after potential errors from pending assertions have been consumed. | ||
| const consumedErrors = Promise.all(this.pendingAssertions); | ||
| // Note that more assertions may be added in the meantime. | ||
| this.pendingAssertions = []; |
There was a problem hiding this comment.
Maybe add a test to validate the works it happens twice (pending resolve, but more pending have been added, and yet another sync will be added while you wait for those to resolve).
|
|
||
| ava(a => { | ||
| a.pass(); | ||
| setTimeout(() => { |
There was a problem hiding this comment.
I really don't think this should be ok. Many people do the mistake of using setTimeout inside a sync test and it silently passing. If users need to do this they should either use an async function or test.cb().
There was a problem hiding this comment.
@sindresorhus It's not OK, that's what #1330 is about. This merely verifies that the internal handling doesn't cause a crash, since the error cannot currently be communicated.
Whilst not a great way to write tests, users may end up adding more
assertions from inside a promise chain that's passed to the
t.throws()or
t.notThrows()assertions. These should be counted for the plan, andnot fail the test as being extraneous.
Fixes #1327.