Introduce data providers to QUnit

[ventuno]

Many test frameworks offer ways to avoid test duplication by allowing parameters to be passed into a test. For example, Jest offers test.each:

test.each([
  [1, 1, 2],
  [1, 2, 3],
  [2, 1, 3],
])('.add(%i, %i)', (a, b, expected) => {
  expect(a + b).toBe(expected);
});

TestNG (a Java test framework) offers the DataProviders which use decorators to attach data sources to tests.

It makes sense for QUnit to offer the same feature.

Sample implementation

#1569

Open questions

1. Jest offers the possibility of customizing the test name by providing printf-like parameters (using Node's util.format);
2. Jest also offers a describe.each. I personally the benefit of this feature, but we should consider implementing our version of module.each.

[ventuno]

@Krinkle any thoughts on this?

[Krinkle]

test.each([
  [1, 1, 2],
  [1, 2, 3],
  [2, 1, 3],
])('.add(%i, %i)', (a, b, expected) => {
  expect(a + b).toBe(expected);
});

The current approach in QUnit would be to use JavaScript's own native idioms, which result in an almost identical style. Perhaps even slightly more idiomatic by levering destructuring and template literals in a way that developers would recognise, understand, and know how to use without further explanation or dedicated documentation:

[
  [1, 1, 2],
  [1, 2, 3],
  [2, 1, 3],
].forEach(([a, b, expected]) => test(`add(${a}, ${b})`, assert => {
  assert.equal(a + b, expected);
}));

Personally, I tend to go for a slightly less dense style, but essentially the same:

[
  [1, 1, 2],
  [1, 2, 3],
  [2, 1, 3],
].forEach(([a, b, expected]) => {
  q.test(`add ${a} and ${b}`, assert => {
    assert.equal(a + b, expected);
  });
});

This seems simple and close enough to not really warrant a new API, and possibly why it's not come up before.

Most test frameworks in JavaScript and other languages do offer a form of data provider. But, in most languages and in some JS-based test runners that are more formal, it's not so easy to just handle this programmatically. E.g. in PHPUnit one couldn't use a foreach loop in the middle of a TestCase class to generate test cases.

At the very least we should have a story for this on the website with examples!

If we were to come up with our own, I think one of my wishes would be to make it look less "busy" than the above for the common case, and actually offer a meaningful abstraction layer that doesn't expose as many concepts as the DYI approach. Specifically:

• No manual formatting of test case names.
• No additional closure, function calls, or chaining complexity (more confusing to understand for new users, easy to write incorrectly).
• Allow labelling of data sets (somewhat inspired by PHPunit dataProvider (example), maybe by supporting object literal in place of the wrapper array).

A few ideas below. I toyed with having the base name go first, that might make for slightly better UX? I also tried both with and without variadic arguments. I like the simplicity of not needing to destructure, it looks cleaner. But, it also reserves this signature more or less forever for any other purpose, which makes it difficult for plugins to support this in a way that isn't confusing. For example, if we want to allow data sets to not all have the same number of parameters, then a plugin that "appends" a parameter to the signature would sometimes wrongly take the place of a dataset parameter. Maybe that's okay if we say we don't encourage/support plugins for this new method?

q.test.each('add', {
  foo: [ 1, 2, 3 ],
  bar: [ 2, 3, 5 ]
}, (assert, a, b, expected) => {
  assert.equal(a + b, expected);
});
// Test: add foo
// Test: add bar

q.test.each('add', [
  [ 1, 2, 3 ],
  [ 2, 3, 5 ]
], (assert, a, b, expected) => {
  assert.equal(a + b, expected);
});
// Test: add #0
// Test: add #1

I think with these, I could confidently say that we've provided a simpler interface, with fewer concepts exposed that the user would need to understand or interact with. Thoughts?

[smcclure15]

I've been approached with this as well in my domain, and I've also retorted with favoring the native/modern JS patterns.

This is a related project to that end:
https://github.com/AStepaniuk/qunit-parameterize
Though I'm not positive it survived past QUnit 2.x.

The "sequential" and "combinatorial" flavors are certainly interesting, though again, not too far off of vanilla JS. Just something to keep in mind if we move forward with the test.each approach (which looks pretty lean and I'd be onboard with that as long as it doesn't choke off any API evolution).

[ventuno]

Thanks @Krinkle, @smcclure15 :-).
I agree having multiple calls to the test function in a forEach is probably good enough, but I think a QUnit-provided API would be better for a couple of reasons:

1. When we provide a ready to use API instead of just letting users find their own way of achieving the same results (e.g.: people could use forEach, a for loop, or provide their own custom method which wraps one of the two) which makes for more standardized code bases and reduces the amount of conversations among developers to align on a solution. This could also be solved by providing examples in the documentation, of course.
2. Reduce indentation. Definitely not the strongest argument, but given that most tests already live inside one (or multiple) module callback it is probably safe to say reducing indentation can help readability.

I do not have a strong preference on this, but I would rather keep the test cases at the beginning (or at the end) of the arguments' list. This is because QUnit users are used to the <testName, callback> format and breaking that pattern just for test.each feels like a violation of the existing UI.

I also like qunit-parameterize's approach with the chained calls to cases(...) and test(...), but probably find test.each to be simpler.

Let me know how you would like to proceed. As a next step I could add documentation to #1569. I'm assuming this would be its own page just like test.only.

[raycohen]

IMO We should document the straightforward javascript idiom and not add this to the qunit api

Without any api changes, the same pattern can give you single test, multiple test, or module reuse without any new apis

[configObject1, configObject2].forEach((config) => {
  test(/*...*/);
  test(/*...*/);
  module(/*...*/);
});

// or equivalently
function sharedBehavior(config) {
  test(/*...*/);
  test(/*...*/);
  module(/*...*/);
}

[configObject1, configObject2].forEach(sharedBehavior);

[ventuno]

@Krinkle, @smcclure15 any further thoughts on this?

[smcclure15]

It's a close call for me, but I lean more towards builtin support via QUnit core, particularly with the "free" test-naming that @Krinkle proposed over the %i, %s sort of formatters.

Copy/paste solutions breeds deviation, customizations, and mistakes, in addition to the sheer duplication.
FWIW, this has been my copy/paste test helper utility for awhile now:

function parameterizedTest (parameters) {
    const keys = Object.keys(parameters);
    return (name, fcn) => {
        keys.forEach(key => {
            let values = parameters[key];
            QUnit.test(`${name} (param=${key})`, assert => {
                fcn(assert, ...values);
            });
        });
    };
}

// USAGE:
const P_TEST = parameterizedTest({
    foo: ['tagname1', 'myvalue1'],
    bar: ['tagname2', 'myvalue1']
});

P_TEST('does ABC', (assert, tag, value) => {
    ...
});
// produces:
// does ABC (param=foo)
// does ABC (param=bar)

P_TEST('does XYZ', (assert, tag, value) => {
    ...
});
// produces
// does XYZ (param=foo)
// does XYZ (param=bar)

That is favoring the (param=value) grammar found in MATLAB Unit Test Framework's parameterization support (which is my "hometeam" / day job).

I've also used Google Test and am familiar with its parameterization support. That uses a /0, /1, /2 sort of test-naming convention.

I like @Krinkle's idea to support objects and array, where the names are generated as <fieldname> and #index respectively.

I also wanted to think through a use case around some very simple API, like "isVowel" is one-input and a boolean output...
I'd want to naively write those test cases as the following:

QUnit.test.each('Vowel', ['a','e','i','o','u'], (assert, vowel) => {
    assert.true(isVowel(vowel));
});
QUnit.test.each('Consonant', ['b','c','d','x','z'], (assert, consonant) => {
    assert.false(isVowel(consonant));
});
QUnit.test.each('Invalid', ['word', '', 2, {}, null, undefined], (assert, bad) => {
    assert.false(isVowel(bad));
});

The hard part is where we want to test an array value within those (eg, test that [] or [a] is not a vowel), and all of a sudden it's not a flat list. I was pleasantly surprised to find that Jest supports "1D array of primitives". I'm not sure what that heuristic is though; maybe

if (dataArray.length === dataArray.flat().length) {

?

I think that "flat" workflow will be a popular, casual usage that is very readable in tests, and is important to support with as clear boundaries (when it's "flat" and when it requires a richer array/object specification) as possible.

In general, I'd want to add as much input-validation as possible, to produce nice clean error messages when the API is accidentally misused; that's part of the real power and benefits of the builtin support.

[ventuno]

Thanks @smcclure15 I looked into Jest's support of "1D array of primitives" and they achieve that by checking that every item in the input array is an array itself (e.g.: if not every element of the array is an array, then it's considered an array of primitives).
I can do that too if that's the direction we want to take. Just let me know.

Concerning naming, at the moment I'm just prefixing each test name with the test index. Perhaps we can look into more complex naming into a follow up PR?

Both you and @Krinkle seem to prefer test inputs to be passed in between the test name and the callback function, so changed the API in #1569 to look like that.
I also added docs and I'll open the PR up for reviews.

[ventuno]

@smcclure15, @Krinkle: as #1569 seems to approach completion I wanted to discuss the follow up steps to support objects as input to test.each.
Using one of the examples in #1569 (comment):

QUnit.test.each('named object', {
  foo: { from: 'a', to: 'b' },
  bar: { from: 'x', to: 'y' }
}, (assert, { from, to }) => {
  // …
});
// named object foo
// named object bar

My interpretation here is that for each key in the input object (e.g.: Object.keys) we want to create a new test case. In this case we could use the key to generate individual test names similarly to what we do now with array indexes.

Is there any other special case you would like me to support? Let me know if you prefer to discuss this in a separate issue. I am more than happy to create a new one if needed.

[Krinkle]

@ventuno That looks right, yes. Just treating them the same as for array indexes.

[ventuno]

Thanks @Krinkle are we still on the same page to merge #1569 first and then take this as a follow up PR?

[Krinkle]

Yep!

[ventuno]

@Krinkle do you know when a new release (including this feature) is planned?

[Krinkle]

@ventuno Today/tomorrow. Running through the checklist as I speak.