Skip to content

Make t.fail() accept an object #2615

Open
@dolsem

Description

@dolsem

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.

Activity

changed the title [-]Make t.fail() except an object[/-] [+]Make t.fail() accept an object[/+] on Nov 25, 2020
novemberborn

novemberborn commented on Nov 26, 2020

@novemberborn
Member

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

dolsem commented on Dec 9, 2020

@dolsem
Author

Here's an example of how a custom assertion might be defined:

const test = require('ava');
const { Assertions } = require('ava/lib/assert');

Object.assign(Assertions.prototype, {
  bla(actual, expected) {
    if (!blaTest(actual, expected)) {
      this.fail('error message');
    } else {
      this.pass();
    }
  }  
})

test('bla test', (t) => {
  t.bla(actual, expected);
});

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 to addFailedAssertion()? This way the user will be able to modify the stack appropriately. Or alternatively just save the fail method in a property on the Assertions 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

novemberborn commented on Dec 13, 2020

@novemberborn
Member

@dolsem my hunch is that exposes AVA internals more than we'd like. I like an option object better.

dolsem

dolsem commented on Dec 14, 2020

@dolsem
Author

@novemberborn well, if you don't want to expose AssertionError, the aforementioned object could have an optional error field with an instance of a custom error. Then AssertionError 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 like require('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

novemberborn commented on Dec 14, 2020

@novemberborn
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @novemberborn@dolsem

        Issue actions

          Make t.fail() accept an object · Issue #2615 · avajs/ava