avajs / ava

Node.js test runner that lets you develop with confidence 🚀
MIT License
20.74k stars 1.41k forks source link

Support running a test function multiple times with different data #2980

Open sindresorhus opened 2 years ago

sindresorhus commented 2 years ago

You can already do this with macros, but it's a bit verbose and complicated. It would be nice to simplify the common case of just running the same test over an array of fixtures.

Possible API:

test('validate email', t => {
    console.log(t.data);
    t.true(isString(t.data));
})
.data([
    'foo',
    'bar'
]);
//=> 'foo'
//=> 'bar'
novemberborn commented 2 years ago

The "forkable test interface" I was looking to experiment with in https://github.com/avajs/ava/pull/2435 would work for this.

I don't think we can easily add a trailing .data() multiplier.

tommy-mitchell commented 1 year ago

@novemberborn does AVA more easily support this now?

novemberborn commented 1 year ago

Not in the spirit of this issue, no.

There's other ways of setting up essentially the same test function for multiple inputs, though.

Looking at this again, the problem I see is that AVA requires titles to be unique (it's used as a key within the snapshot file, for instance) and it's not clear how we would execute the same test with the same title for different data.

tommy-mitchell commented 1 year ago

Perhaps the title could be suffixed with a hash of the data?

novemberborn commented 1 year ago

@tommy-mitchell yes that's not a bad idea, but then the question becomes how we'd reliably hash the data across platforms and Node.js versions, including for functions and whatnot. Unless we restrict the acceptable data types to something we can hash.

tommy-mitchell commented 1 year ago

Maybe a v1 of this could require data to be titled?


test('moves horizontal', t => {
  t.true(move(t.data));
}).data([
  ['left', -1],
  ['right', 1],
]);
novemberborn commented 1 year ago

Riffing on that, and going back on my earlier comment, perhaps it doesn't usually matter? We could generate titles like moves horizontal (data@{0}). That's not great when your snapshot fails because the data changed, but if you need to you can just snapshot the data.

We could further extend with a .title() modifier which takes titles in the same order as the data.

tommy-mitchell commented 1 year ago

2979 would be good to keep in mind with this too

geoffreytools commented 1 year ago

I do this kind of stuff all the time:

test('detect when the player has won', t => [
    { player: 'scissors', computer: 'paper' },
    { player: 'paper', computer: 'rock' },
    { player: 'rock', computer: 'scissors' }
].forEach(({ player, computer }) =>
    t.true(shifumi(player, computer), `with ${player} > ${computer}`)
))

I find this much more straightforward to write and to read than macros, and I put up with the disadvantages.

I "wouldn't mind" if the following was possible:

test('detect when the player has won', t => {
    const { player, computer } = t.data;
    t.true(shifumi(player, computer));
}).data(
    { player: 'scissors', computer: 'paper' },
    { player: 'paper', computer: 'rock' },
    { player: 'rock', computer: 'scissors' }
).title(
    ({ player, computer })=> `with ${player} > ${computer}`
);

The main advantages for me:

I am not a fan of the idea of having to maintain a separate array for the data and for the titles, especially if they get long. A title function is convenient and would work around the snapshot problem nicely, because the user can come up with a way to reliably convert its cases to a unique string.

boneskull commented 1 year ago

I'm looking at let myMacro = test.macro() to do something like this, but the problem is that this only seems to work if I have synchronous access to the parameters I'm trying to call myMacro() with.

I understand that the calls to test() in an Ava test file should be synchronous; does this mean it's just not possible in Ava? Think pulling some stuff out of a db, then calling myMacro with data from each row.

Another way would be something like e.g., test.init(async () => {}) where the callback would explicitly allow creating new tests (as opposed to e.g., test.before() which doesn't make sense semantically). I'd hope that doesn't smell too much like "nesting".

FWIW Mocha also has this problem, which I solved with a terrible hack (pass a --delay flag to mocha, then call run() when ready). I don't recommend that. 😝

novemberborn commented 1 year ago

@boneskull once you declare a test or hook, all others need to be declared synchronously.

However you're allowed to wait before you do that. Either in a timeout, a promise-chain, or with top-level await.