Open
Description
It seems like the only problem with #1094 is providing detailed error output when a custom assertion fails. Everything else is straightforward. What if t.fail()
could accept an object to use for creating an AssertionError? The fields could be message
, actual
, expected
, and format
that will specifying which formatting function to use for setting the values
field on the error object. Could also add an optional assertion
field for a custom assertion name.
Would this be compatible with #2435?
If there's interest, I can submit a PR for this.
Metadata
Metadata
Assignees
Labels
Type
Projects
Milestone
Relationships
Development
No branches or pull requests
Activity
[-]Make t.fail() except an object[/-][+]Make t.fail() accept an object[/+]novemberborn commentedon Nov 26, 2020
It's worth exploring, sure.
I'm concerned that properly integrating this with the reporters will be harder than expected. You'd also want to customize the stack trace to not start at the
t.fail()
call site.How would you set up custom assertions to use this?
dolsem commentedon Dec 9, 2020
Here's an example of how a custom assertion might be defined:
This works but doesn't let us customize the actual error output from AVA. What if we could just modify the fail assertion to pass through an instance of
AssertionError
toaddFailedAssertion()
? This way the user will be able to modify the stack appropriately. Or alternatively just save the fail method in a property on theAssertions
instance in the constructor, so that people who write custom assertions have access to it?I haven't seen most of the codebase, but wouldn't reporting work without modifications as long as AssertionError uses the same API?
novemberborn commentedon Dec 13, 2020
@dolsem my hunch is that exposes AVA internals more than we'd like. I like an option object better.
dolsem commentedon Dec 14, 2020
@novemberborn well, if you don't want to expose
AssertionError
, the aforementioned object could have an optionalerror
field with an instance of a custom error. ThenAssertionError
would reuse its stack trace. It just feels a bit more convoluted than necessary to me. Any particular reason to not expose AssertionError to the outside world? Backwards compatibility considerations? It could be exposed via something likerequire('ava/internals')
as opposed to being available on the main module entrypoint. This way it's more of a "use at your own risk" type of deal.novemberborn commentedon Dec 14, 2020
I want to avoid accidental APIs that we then cannot change. I think there are too many internal considerations for how
AssertionError
works that are hard to safely expose, hence a "facade" object would be a safer long-term strategy.