[Idea]: test macros

[jamestalmage]

From: #78 (comment)

Occasionally it's useful to create a function that calls test for you:

// fictional module that analyzes math expressions in strings:
import calculator from 'calculator-parser';
import ava from 'ava';

function test(input, expected, only) {
  const method = only ? ava.only : ava;
  method(input + ' === ' + expected, t => {
    const actual = calculator.calculate(input);
    t.is(actual, expected);
  });
}

test('3 + 4', 7);
test('5 - 2', 3);
test('6 * 3', 18, true); // causes this to be interpreted as an exclusive test
// ...

Unfortunately, such helpers create a situation where it becomes basically impossible to do static analysis on the code (static analysis may be necessary to solve #78).

I think we could solve some of this with a macro command:

import test from `ava`;

const myMacro = test.macro(
  (t, input, expected) => { // additional arguments to the macro are passed in. 
    const actual = calculator.calculate(input);
    t.is(actual, expected);
  },
  (input, expected) => input + ' === ' + expected // optional function to compute the test tile from the input.
);

myMacro('3 + 4', 7);

// macros get some of the same modifiers as a normal test function:
myMacro.skip('3 * 6', 18);
myMacro.only('9 / 3', 3);

[jamestalmage]

@jfmengels points out another potential benefit of this in our linter: avajs/eslint-plugin-ava#56 (comment)

[jfmengels]

I love the idea for test macros, but I noticed that it will also make linting some stuff harder (finding out that .only, .skip is used etc.). It's a trade-off.

[jamestalmage]

but I noticed that it will also make linting some stuff harder

I am not sure if Eslint has an equivalent to Babel's bindings. It would make linting macros pretty easy. I have not seen anything in the docs though.

[jamestalmage]

@jfmengels - What if, instead of storing a reference, you had to pass a string identifier, and it became just another macro.

test.macro('myMacro', (t, input, expected) => ...);

test.myMacro('input', 'expected');
test.only.myMacro('input', 'expected');

I think that makes static analysis a little easier (especially if you define macros in helpers).

[jfmengels]

Interesting idea, but then you'd have

1. tampering of the AVA import, which will not affect other tests but is still pretty ugly
2. to import a file who'd have a side-effect of defining a macro, which seems counter to AVA's explicit imports (compared to Mocha where describe, it that are injected for instance). I like explicit imports.
3. Odd behavior when definining a macro called only for instance.

An alternative would be

// Proposal 1
// Defining
test.defineMacro('myMacro', (t, input, expected) => ...);
// Using
test.macro('myMacro', 'input', 'expected');

But you'd still have problem 1 above.
An even better alternative:

// Proposal 2
// Defining (call it createMacro, defineMacro, whatever)
const myMacro = test.defineMacro((t, input, expected) => ...);
// Using the macro
test.macro('title', myMacro, 'input', 'expected');

// Proposal 3
// or even alternatively, by re-using `test`, that adapts its behavior 
const myMacro = test.macro((t, input, expected) => ...);
// Using
test('title', myMacro, 'input', 'expected');

I think proposal 2 is the best implementation. You get

• static analysis capability
• test creators in helpers that will not be linted as errors
• Explicit imports of macros
• No tampering of the AVA/test object
• Explicit use of a macro (which is neutral, but worth pointing out)

[jamestalmage]

My only concern with your proposals is the verbosity. I was already concerned about that with my second proposal here, your proposals just make it worse.

Ideally, we could use my first proposal, as it is the least verbose. My second one was just to address the static analysis problem.

IMO, most of your concerns aren't that big a deal:

1. tampering of the AVA import, which will not affect other tests but is still pretty ugly

Exactly - it won't affect other tests (because we fork), so no surprising behavior. You define which macros you load in each test (so no trouble figuring out what is happening).

2. to import a file who'd have a side-effect of defining a macro, which seems counter to AVA's explicit imports (compared to Mocha where describe, it that are injected for instance). I like explicit imports.

I think in the case of a helper defining macros - the user has decided to do that themselves. AVA's not doing that to them. They won't be surprised by it. In general, I agree with your sentiment, but this seems like a reasonable exception.

3. Odd behavior when definining a macro called only for instance.

We could protect against macros that try to overwrite built in properties.

[jfmengels]

I think in the case of a helper defining macros - the user has decided to do that themselves. AVA's not doing that to them

Sure, but still, I myself as a user don't like it. And if AVA doesn't allow macros to be used without a variable, AVA will kind of force me to do that, should I want to create a macro used in multiple files.

It's more verbose, but not that much, but that's just my opinion. If you're that concerned about verbosity, my proposal 3 is pretty short. It can be even be shortened by simply having macro be a simple function and injecting the arguments

// Normal tests
test(t => { var a = 2, b = 3; ... });
test(t => { var a = 2, b = 4; ... });

// Proposal 4
// Macro/reusable test
var macro = (t, a, b) => { ... };
test(macro, 2, 3);
test(macro, 2, 4);

Importing would be easy, as it's a special function. No naming, no AVA tampering. Alternatively, args could be added to the context: t.context.args === [2, 3]. I like this version even more.

[jamestalmage]

Hmm. Proposal 4 looks pretty good, and it should be really simple to implement.

I like it.

[jamestalmage]

Alternatively, args could be added to the context: t.context.args === [2, 3].

👎 You can completely replace t.context in a beforeEach. t.context = something.

[jfmengels]

👎 You can completely replace t.context in a beforeEach. t.context = something.

Proposed it just in case proposal 4 could not be done because test already uses further arguments or so. Much prefer the args in function option too.

[jamestalmage]

Rather than spreading the arguments ourselves, we should pass additional args as an array:

function macro(t, additionalArgs) {
  if (additionalArgs.length === 2) {
    // ...
  } else {
    // ...
  }
}

test(macro, 'foo', 'bar');
test(macro, 'foo');

This allows us to pass additional args in the future breaking changes. With destructuring, defining your macro isn't really any more verbose:

function macro(t, [foo, bar]) {
  // ...
}

[novemberborn]

👍 on proposal 4.

Perhaps we could expose the macro creating functionality through an ava/macro module. That way helper files won't accidentally become AVA runners.

[jfmengels]

@novemberborn With proposal 4 macros are simple functions, and there's no need for a special macro creator.

[novemberborn]

With proposal 4 macros are simple functions, and there's no need for a special macro creator.

Ah, I like it even more now!

[novemberborn]

What separates proposal 4 (test(macro, 2, 4)) from calling test with too many arguments? Can the macro define the test title? Do we want to support macros that create multiple tests?

[jfmengels]

What separates proposal 4 (test(macro, 2, 4)) from calling test with too many arguments?

Nothing, except that the arguments may be used by the macro (I might not have understood the question right).

Can the macro define the test title?

Since the macro is simply a function, no. I'm guessing it might be wanted in some cases (if you're testing plenty of cases for add(a, b), having multiple titles might get cumbersome), but then again, it might get hard to identify it otherwise. test(addMacro, 2, 3, 5); test(addMacro, 2, -1, 1); Which one failed? You could put the args in the test title, but can get ugly when given complex objects.

I'd argue that it's best if the macro doesn't set a title, and that it should be given to the test function, like

function addMacro(t, a, b, expected) {
  t.is(add(a, b), expected);
}

test('add 2 numbers', addMacro, 2, 3, 5);
test('add a positive and a negative number', addMacro, 2, -3, -1);
test('add a number and NaN', addMacro, 2, NaN, NaN);

Do we want to support macros that create multiple tests?

Though I see the value in it for the user, one of the points for macros was to make static analysis easier/possible. I'd say that having macros that could create tests (potentially containing .only) makes it harder.

[jamestalmage]

What separates proposal 4 (test(macro, 2, 4)) from calling test with too many arguments?

There would be no such thing as "too many arguments". Everything past the test function would be passed as "additional arguments". The macro function itself would get an array as a second argument, containing all the "additional arguments"

Can the macro define the test title?

I was thinking about titles too.

Here was my thought:

function macroFn(t, ...additionalArgs) {
  t.is(...);
}

// optional: attach a title generator to `macroFn`
macroFn.title = function (...additionalArgs) {}

// optional: attach a title template to `macroFn` (not sure which template language to use)
macroFn.title = '$1 does thing $2'

// title is extracted from string:
test.macro('title', macroFn, ...additionalArgs);

// title generated using titleFn or template
test.macro(macroFn, ...additionalArgs);

Do we want to support macros that create multiple tests?

I think we could support it in a limited way by allowing arrays of macros:

function aPlusB(t, [a, b, expected]) {
  var result = calculator.parse(`${a} + ${b}`);
  t.is(result, expected);
}
aPlusB.title = "${1} + ${2} === ${3}";

function bPlusA(t, [a, b, expected]) {
  var result = calculator.parse(`${b} + ${a}`);
  t.is(result, expected);
}
bPlusA.title = "${2} + ${1} === ${3}";

const addition = [aPlusB, bPlusA];

// these two lines create 4 total tests
test(addition, 3, 4, 7);
test(addition, 2, 2, 4);

[novemberborn]

Can you run a macro using just test() or should we stipulate test.macro()? I'm concerned about the test() overloading, e.g. with the array proposal, but also with the additional arguments.

I like the macroFn.title suggestion.

[jamestalmage]

I think just test(). test.macro(macroFn) feels too long

[jfmengels]

If we can avoid introducing the idea of macro, and just enhance test, I think it would make things simpler, so I'm all for test().

I think the suggestion for macroFn.title, and would suggest that title in test(title, macro, ...args) should override it.

👍 For the title template, even though when given complex arguments, it will be unreadable, but not a lot of things we can do about that?

👍 on the array proposal too.

Still not sure why you think the test should be given an array of args (t, [a, b]) => ... and not simply spread args (t, a, b) => .... Sure it's not that harder with destructuring, but it might confuse users a bit. Do you expect a third argument to ever come into play?

Also, do you think test might ever be given an other arg to change its behavior? If so, we might want to have the arguments given as an array, rather than as "spread" arguments test(macro, [1, 2, 3], someFutureArg) without changing what we will decide on how arguments are passed to the macro function.

[novemberborn]

I think just test(). test.macro(macroFn) feels too long

Sure. It means that macros aren't anything specific, we're just giving the test implementation more powers.

[jamestalmage]

Still not sure why you think the test should be given an array of args ... Do you expect a third argument to ever come into play?

That's why ... allowing for the possibility of a third argument.

[novemberborn]

Macros shipped in https://github.com/avajs/ava/releases/tag/v0.15.0.