Refactor assertions #1302
Refactor assertions #1302
Conversation
|
Tests are red across the board, but it works on my machine… I'll look into that tomorrow. Please hit me with feedback regardless 😄 |
novemberborn
force-pushed
the
refactor-assertions
branch
2 times, most recently
from
f2639b9 to
4cda666
Compare
Clearing CI caches helped. Pushed more changes too. |
novemberborn
force-pushed
the
refactor-assertions
branch
2 times, most recently
from
8dbeda6 to
f90a9ef
Compare
It's undocumented and unused.
This matches the terminology used in the README.
* Use our own AssertionError class. Now we can track the assertion rather than abusing the operator field. We can also stop depending on `core-assert` once we implement `t.throws()` ourselves. Reserve the `statements` property for power-assert output * Directly create `AssertionError`s, making it easier to specify the varying options * Default the assertion message to an empty string, removing workarounds in the mini and verbose reporters * Improve `t.plan()` assertion errors. Specify 'plan' as the assertion, and use `===` as the operator * Refactor `t.throws()` and `t.notThrows()`, improving readability and fixing #1227 * Move knowledge of when to show output into `serialize-error.js`. It sat kind of awkwardly in `assert.js` * Always try to format errors in mini and verbose reporters, check if actual / expected values are available when formatting * Wrap assertions in `assert.js` for use with the `ExecutionContext` * Only enhance the power-assert assertions, don't use `empower-core` to wrap the others * Remove superfluous `empower-core` 'originalMessage' handling. Our assertions create `AssertionError`s with the correct message * Refactor overloaded `Test#_assert()` method and assertion tracking internals. Note that only `t.throws()` and `notThrows()` are (possibly) asynchronous
Avoid having to normalize errors in `run-status.js`.
* Retain non-standard error properties in a cleaned `object` property * Make AssertionError serialization more explicit * Flag whether the serialized error came from AVA's AssertionError
This is especially useful for `t.notThrows()`, which includes the thrown exception as the `actual` value for the assertion error. Fixes #1150.
Wrap callback errors, rejected promises or synchronously thrown values in an `AssertionError`. Rely on the `actual` value being printed by the reporters, which means it's still shown to the user. Remove now obsolete `Error: ` prefix added to non-assertion errors in `run-status.js`. Instead more helpful messages from `test.js` can be shown. Try to reuse the stack trace from any original error, so magic assert can show context. Fail `t.throws()` / `t.notThrows()` with an AssertionError if called with an improper argument, rather than throwing a TypeError.
novemberborn
force-pushed
the
refactor-assertions
branch
from
f90a9ef to
42b117a
Compare
There was a problem hiding this comment.
This is fantastic, Mark! It took me a while to read through and understand it all. I agree with pretty much all the changes. Lots of great cleanups too 👌😄.
I ran this on a bunch of projects and tried failing tests in different ways, and it seems to work just fine.
lib/assert.js
Outdated
| fail(this, new AssertionError({ | ||
| actual: fn, | ||
| message: '`t.throws()` must be called with a function, Promise, or Observable' | ||
| })); |
There was a problem hiding this comment.
Should it really wait until all tests are finished before warning about this? I would think would call for an immediate error even if the user didn't pass --fail-fast.
There was a problem hiding this comment.
--fail-fast implies all tests stop. This still immediately fails the test. The behavior should be similar to before this refactor, except that the error can no longer be caught by calling code.
There was a problem hiding this comment.
I just tried and you're right. It is the existing behavior. I still think user mistakes like this should fail the test right away no matter what flags the user used though, but that's a separate discussion.
|
🎉🎉🎉 |
See commits. Behavior should be the same, aside from these changes:
undefinedrather thannullt.assertCountgettert.end(error)) are now wrapped in an assertion error, but we'll still print the actual errort.throws()/t.notThrows()are called with an improper argument. We can now even print the actual argument passed!operatoror evenassertion)