Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Customize assertions #1094

Open
yamsellem opened this issue Oct 26, 2016 · 30 comments
Open

Customize assertions #1094

yamsellem opened this issue Oct 26, 2016 · 30 comments

Comments

@yamsellem
Copy link

yamsellem commented Oct 26, 2016

Is there a way to add some custom assertions to t existing ones?

I would like to add those basic ones:
t.instanceof(object, clazz)
t.length(array, length)
t.members(array, item, item)
t.includes(string, substring)

I guess t.context can be used, but feels cumbersome.
And maybe, those assertions can be considered to be added to ava core.

@yamsellem yamsellem changed the title Custom assertions Customize assertions Oct 26, 2016
@sindresorhus
Copy link
Member

Not at the moment, but your needs can be solved with the existing methods:

t.true(object instanceof clazz);
t.is(array.length, length);
t.true(array.every(x => [item, item].includes(x)));
t.true(string.includes(substring));

You can already use any external assertion library. The only caveat is that t.plan() doesn't work.

@alathon
Copy link
Contributor

alathon commented Oct 26, 2016

I think a very strong point of AVA (and tape) is precisely that there are few assertions by default - this reduces the amount of 'magic' that occurs, and reduces bugs in testing code that occurs because you aren't 100% sure how a particular assertion works. Given that bugs occur almost as frequently in test code as they do in the code they test, I think it would be a mistake to change this.

@yamsellem
Copy link
Author

@alathon Being small does not necessarily means being ascetic.

t.is(array.length, length); // is better with t.truthy(array) before
t.true(array.every(x => [item, item].includes(x))); // seems cumbersome
t.true(string.includes(substring)); // could be simpler

Some basic assertion on strings and arrays seems fair to me — and don't break the 'no magic' of the thing, IMO.

@sotojuan
Copy link
Contributor

sotojuan commented Nov 3, 2016

I guess the problem is that it's a slippery slope. These kind of extra assertions are better in userland as separate modules, functions, or snippets. I personally don't see anything wrong with the code block in the last comment but you could make it look "prettier" by abstracting those functions away. That way you can use your own custom logic as well.

The smaller core AVA is, the easier it is to use.

@vadimdemedes
Copy link
Contributor

@sotojuan Good point, totally agree.

I think we should make t extendable with custom methods (assertions). Was thinking to create a separate module for t.jsxEqual(), would be nice to make this possible.

@odigity
Copy link

odigity commented May 21, 2017

I'm in the same boat. I would like to write some custom assertions for convenience and add them to t in my project.

The docs say "you can use custom assertions", but provide no information on how.

@vadimdemedes
Copy link
Contributor

@odigity section "Custom Assertions" in readme is actually related to using 3rd-party assertion libraries, like expect or chai, not extending t with custom assertions. I understand the confusion, I think we have to rename the title to something like "Using 3rd-party assertion libraries".

@odigity
Copy link

odigity commented May 21, 2017

So... what's the easiest way to extend t with custom assertions? :)

@novemberborn
Copy link
Member

@odigity There is no way to do this at the moment. To start we'd need a proposal on a) extending AVA, b) how to register new assertions, and c) how this would integrate with power-assert and our assertion output.

@odigity
Copy link

odigity commented May 24, 2017

So how do you recommend approaching this particular use case?

I need to compare two floats with some margin of error. Because there's no support built-in, and no documented way to add custom assertions, I'm currently doing this:

t.is( Math.round(actual_duration), Math.round(expected_duration) )

(It's a video processing engine.)

It works, but it's ugly compared to, say...

t.almostEquals( actual_duration, expected_duration )

Perhaps with an options arg to control the error margin.

almostEquals is a common term used when dealing with floats in JavaScript.

  • Exhibit A (Yes, I'm thinking about pulling this in to my project.)

Note: I'm not asking you to implement this, only if there exists a sane way by which one such as myself could do so.

@novemberborn
Copy link
Member

@odigity I'd do t.true(almostEquals(actual, expected)).

@alathon
Copy link
Contributor

alathon commented May 25, 2017

@novemberborn But then you don't get the values for actual and expected, just a true or false value. That makes it (potentially) harder to debug. That depends on what you're testing, though, I suppose -- are you testing an almostEquals function, or that actual and expected are close enough; if its the function, then yes that'd be the way to go.

I don't see what's wrong with t.is( Math.round(actual_duration), Math.round(expected_duration) ) though, if thats what you're using (Math.round). That seems clear and declarative to me. Anyone looking at it will know exactly what's going on - as opposed to t.almostEquals, which beckons the questions of: Almost equals according to which precision/function? Can it be used on objects/strings? etc..

@novemberborn
Copy link
Member

But then you don't get the values for actual and expected, just a true or false value.

For t.true() we also print the values of the expressions in the argument, so you should see those values.

@alathon
Copy link
Contributor

alathon commented May 25, 2017

Ah, my bad -- was thinking of tape when it comes to that.

@epeterson320
Copy link

+1 to being able to extend t since we users can get the features we want and the core API can remain small. Jest has a nice API to extend their matchers. Perhaps a similar method would work in AVA.

For what it's worth, though, I'd like to see an almostEquals, closeTo, or inDelta matcher in core. Missing instanceOf is easy enough to work around, but comparing floating point numbers with a certain precision gets either verbose or hacky when testing compute-focused code.

@lewisdiamond
Copy link

lewisdiamond commented Mar 23, 2018

It's really worth making it easy to provide an implementation of custom tests. Right now using t.true(_.isMatch(... ...)) outputs almost pure gibberish. It should only print the difference so it's really easy to spot.

@novemberborn
Copy link
Member

@lewisdiamond would you want to help us implement this?

@chocolateboy
Copy link

chocolateboy commented Jun 16, 2019

In the meantime, the following hack works for me currently (AVA 2.1.0, 3.15.0):

import test from 'ava'
import { Assertions } from 'ava/lib/assert.js'

Object.assign(Assertions.prototype, {
    isFoo (value) { this.assert(value === 'foo') },
    isBar (value) { this.is(value, 'bar') }
})

test('foobar', t => {
    t.isFoo('foo')
    t.isBar('bar')
})

The only nits I've spotted are that custom assertions don't support the skip modifier, and the line number for assertion failures points to the assertion call inside the custom assertion rather than its caller (the correct line can be gleaned from the stack trace).

@novemberborn novemberborn self-assigned this May 25, 2020
@gajus
Copy link

gajus commented Sep 17, 2020

I've used the latter approach to implement https://github.com/gajus/ava-dom

As already mentioned by @chocolateboy, the biggest problem is that the error points to the location of t.true not the assertion.

Is there a way to workaround this limitation?

@novemberborn
Copy link
Member

If we could somehow expose a way to report an assertion error I wouldn't be opposed to that. Maybe as a symbol property on t. And then if other stuff is a bit hacky or just a helper method you pass t to then maybe that's enough for now?

#2435 suggests an API to construct custom test() functions, with full TypeScript support, and that could be a way to register additional assertions I think. But we haven't worked out at all how that would be forward compatible with other changes AVA core would like to make to the t object or how to best distribute those implementations.

@danon
Copy link

danon commented Mar 1, 2021

@novemberborn Where do we stand on adding custom assertions to t, or making t.true(almostEqual() point to the assertion, not t.true? Both of these are fine for me, but does any of it work now? :)

@novemberborn
Copy link
Member

Where do we stand on adding custom assertions to t

Nowhere near starting, let alone shipping.

or making t.true(almostEqual()) point to the assertion, not t.true?

I don't see how that would be possible?

@tommy-mitchell
Copy link
Contributor

Is there anything more to the built-in assertions than being a property of the Assertions class? Could custom assertions be registered there?

@novemberborn
Copy link
Member

@tommy-mitchell yep! Maybe not top-level but under a namespace, e.g. t.sinon.assert() or something that follows their assertion API, but then better integrates with AVA. We'd have to figure out how to register these, and for TypeScript users how to type them.

AVA no longer has power-assert which the start of this thread was concerned with.

@tommy-mitchell
Copy link
Contributor

I’ve played around a bit before with adding type definitions to another library that supported custom assertions, using a mapped type with generics can preserve the added assertions and any JSDoc comments on them.

@tommy-mitchell
Copy link
Contributor

Maybe not top-level but under a namespace, e.g. t.sinon.assert() or something that follows their assertion API, but then better integrates with AVA.

It might make sense for custom assertion namespaces to begin with an identifier (e.g. _) so that they don't overlap with an future additions to AVA's built-in assertions.

For example, say someone publishes some stubbing assertions, and they put them under t.stub (so t.stub.assert() or whatever). Then, later, AVA adds t.stub(/* ... */), and now the names collide.

On a related note, I think user's custom assertions should have a "best practice"/idiomatic namespace, so libraries don't publish their assertions under it. I think t._custom would suffice, or maybe even t._ for brevity:

t._custom.instanceof(object, class);

t._.includes(string, substring);

and then we get cute ._. faces!

@novemberborn
Copy link
Member

I was thinking about this some more. I reckon if we detect that a built-in assertion is being overwritten, and we log a warning at the end of the CLI output, that'll be enough. Accidental collisions are unlikely and we probably won't add all that many assertions anyhow. Why restrict things.

@tommy-mitchell
Copy link
Contributor

That’s fair. I still think having a “custom” namespace be reserved for users is a good idea, though.

@tommy-mitchell
Copy link
Contributor

I think, at minimum, we'd need to expose:

  • AssertionError (assert.js), so AVA can recognize when a custom assertion fails
  • Assertions.withSkip, Assertions.checkMessage, the formatting functions, etc. (assert.js), so custom assertions can be validated correctly
    • Perhaps routine portions of custom assertions could be done on AVA's side, such as the if (!checkMessage('[assertion_name]', message)) check
  • A test.extend function, which would consist of:
    • A property (and associated type) on TestFn (test-fn.d.cts)
    • A chain implementation. Perhaps extend can only be called on test.before.extend (create-chain.js)?

Example API:

import test as anyTest, {AssertionError} from 'ava';

const test = anyTest.extend(t => ({
	includes(string, substring, message) {
		// Could also check individually for more precise error messages
		if (typeof string !== 'string' || typeof substring !== 'string') {
			t.fail(new AssertionError({
				assertion: 'includes',
				improperUsage: true,
				message: '`t.includes()` only accepts strings`',
				values: /* concordance stuff */
			}));
			return false;
		}

		if (string.includes(substring)) {
			t.pass();
			return true;
		}

		t.fail(new AssertionError({
			assertion: 'includes',
			message,
			values: /* concordance stuff */
		}));
		return false;
	},
	// ...
}));

test('substring in string', t => {
	t.includes('hello world', 'hello', 'Substring not in string!');
});

The custom assertions would be mapped onto the new test variable for TypeScript users.

@novemberborn
Copy link
Member

test.extend() would be the most powerful piece. I've previously made inroads towards that but it's just a bulk of work.

Alternatively we already have the ava/plugin export from which you can register shared workers. We could add a "register assertion" there. We could then make the t type (ExecutionContext) take an extension point for the assertions so they can be woven into the type definitions.

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

No branches or pull requests