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

Enhance test.each message format #6413

Closed
stropho opened this issue Jun 7, 2018 · 23 comments
Closed

Enhance test.each message format #6413

stropho opened this issue Jun 7, 2018 · 23 comments
Labels

Comments

@stropho
Copy link

stropho commented Jun 7, 2018

Enhance test.each message format:

  1. positional placeholders such as %2$s
  2. index of test set to clarify which test set (table row) was used in a failed test

Motivation

  1. Easier way to customize test messages
  2. Easier way to find which test set (table row) failed

Example

There are a few ways I can think of
A/ smarter message format like in sprintf-js package

// not a real world example
test.each([[1, 1, 2], [1, 2, 3], [2, 1, 3]])(
  '`%3$i` created using .add(%1$i, %2$i)',
  (a, b, expected) => {
    expect(a + b).toBe(expected);
  },
)

Still not sure how to specify optional test set (table row) index

B/ message as a function - more customizable, and in my opinion easier to implement

test.each([[1, 1, 2], [1, 2, 3], [2, 1, 3]])(
 (a, b, expected, index) => // or in a similar way
   `${index}: ${expectedResult} created using .add(${a}, ${b}) `,
  (a, b, expected) => {
    expect(a + b).toBe(expected);
  },
)

Is this something that could be considered as a helpful feature and implemented in jest?

@SimenB
Copy link
Member

SimenB commented Jun 7, 2018

@mattphillips ^

@mattphillips
Copy link
Contributor

I like option A as jest-each was originally using sprintf-js - the only problem with going back to sprintf-js is that it would be a breaking change as not all of the placeholders have the same semantics as Node's util.format i.e. %o differs.

Option B is too verbose IMO and you'd probably be better off just using Array.forEach at that point.

A middle ground could be to add argument swapping support with $ and still use util.format to do the interpolation?

What do you think @SimenB ?

@mattphillips
Copy link
Contributor

mattphillips commented Jun 7, 2018

index of test set to clarify which test set (table row) was used in a failed test

I'm not sure of a nice way to solve this. We could easily prefix/suffix a test with the index. This again could be considered a break in change as it would break any tests that are using snapshots.

I generally don't consider parameterised tests as being a part of an array even though they are defined that way. I tend to think of them as independent tests and the interpolated values are enough to identify the failing tests.

That said I think if there was enough interest in this then it might be a worthwhile addition, its just not a problem I've ran into yet 😄

EDIT: I’m open to adding a placeholder for the index see #6414

@mattphillips
Copy link
Contributor

Another idea I’ve had around this could be to ditch printf all together and just interpolate $index Syntax with pretty-format

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

@borela
Copy link

borela commented Jun 12, 2018

@mattphillips Is it possible to add something like %index or %row to reference the current row being tested? It would be useful when the dataset is too large.

    test.each([
      // Big dataset.
    ])('%index- some test', () => {
    })

@rickhanlonii
Copy link
Member

@borela I believe that's almost done #6414

@stropho
Copy link
Author

stropho commented Jun 15, 2018

Alright 2., i.e. row index is almost done - great.

1.
A/ sprintf-js - breaking change, formatting works a bit differently than util.format
B/ message as a function - verbose
C/ argument swapping support with $ and still use util.format
D/ pretty-format - interesting, can't really estimate how difficult it would be to pass formatting options for different values. But maybe I just didn't get it right.

Honestly, any of these options is fine by me. The only workaround I could think of right now is to swap values in the data table rows - cumbersome. Using [].forEach(...) just does not feel right when test.each exists.

And now the hardest part - which option is the best and is going to be implemented? :)

@malhotraashna
Copy link

A different question on test.each. I want to use variable values in test.each rather than literal values. Do we have any workaround for that?

I have a question for the same on Stackoverflow.

@mattphillips
Copy link
Contributor

@malhotraashna you can use variables in test.each but the value of the variable needs to be defined with the scope of the test.each and not in the beforeAll. See #6888 for more context

@malhotraashna
Copy link

@mattphillips I have several describe blocks many of which are nested. The beforeEach block at the root is using jest.resetModules(). And then I need to include other modules in my immediate beforeEach block just before the it.each statement. Depending on the require of that module can only I specify my table inside it.each.

The structure is somewhat like this:

beforeEach(()=>{
  jest.resetModules();
  // require modules
});

describe('test',()=>{
  describe();

  describe('test1',()=>{
    beforeEach(()=>{
      // requireModules
      // initialise table to pass to it.each
    });

    describe('test2',()=>{
      it.each(table)();
    });
  });
});

When you say variable needs to be defined with the scope, you mean initialising table just inside describe block without beforeEach() right?
Something like this

describe('test1',()=>{
    beforeEach(()=>{
      // requireModules
      // initialise table to pass to it.each
    });

    describe('test2',()=>{
      it.each()();
    });
  });

@r4j4h
Copy link

r4j4h commented Oct 19, 2018

Sorry for chiming in with something slightly unrelated, but I felt like it fits under this Issue as another example use case rather than deserving another issue asking for about the same thing:

I wish test.each supported accessing a key from arrays of objects in the test name, without having to use tagged template literals.

With Jasmine long ago I used https://github.com/MortalFlesh/jasmine-data-provider and it's method of arrays of objects and keyed object with the key being the test name partial I found worked far more intuitively than array positionals or funky Cucumber tables with value interpolations scattered everywhere..

For example, this currently works, but I can't help but feel keeping the whitespace and aligning pipes is annoying and not as friendly to common tools or IDE refactorings as JSON objects would be:

test.each`
name    | main_arg | additional_arg | expected
${'given an expected thing'} | ${1} | ${undefined} | ${true}
${'given an expected thing with example'} | ${1} | ${'example'} | ${true}
${'given cool unexpected thing'} | ${"cool"} | ${undefined} | ${false}
${'given cool unexpected thing with example'} | ${"cool"} | ${'example'} | ${false}
`("Table format: The thing gracefully handles being $name", async ({name,expected}) => {
    expect(name).toBe(expected);
});

results in:

Table format: The thing gracefully handles being "given cool unexpected thing with example"

expect(received).toBe(expected) // Object.is equality

Expected: false
Received: "given cool unexpected thing with example"

This seems like it should work, but I can only get to print the ENTIRE row's object:

test.each([{
    name: 'given an expected thing',
    main_identity: 1,
    another_argument: 'example',
    expected: true
}, {
    name: 'given an expected thing with example',
    main_identity: 1,
    another_argument: 'example',
    expected: true
}, {
    name: 'given cool unexpected thing',
    the_argument: "cool",
    another_argument: 'example',
    expected: false
}, {
    name: 'given cool unexpected thing with example',
    the_argument: "cool",
    another_argument: 'example',
    expected: false
}])("Array format: The thing gracefully handles being %(name)s :(", async ({name, expected}) => {
    expect(name).toBe(expected);
});

results in:

 Array format: The thing gracefully handles being { name: 'given cool unexpected thing with example',
  the_argument: 'cool',
  another_argument: 'example',
  expected: false }

expect(received).toBe(expected) // Object.is equality

Expected: false
Received: "given cool unexpected thing with example"

From what I gather, this is really due to using Node's util.format under the hood. Earlier in this thread sprintf-js was brought up
@mattphillips , earlier in this thread you mentioned

I like option A as jest-each was originally using sprintf-js - the only problem with going back to sprintf-js is that it would be a breaking change as not all of the placeholders have the same semantics as Node's util.format i.e. %o differs.

Could you go into more detail on the negative aspects of how %o changes? Like, does its default case format the entire object differently than Node's util.format does?

I only ask because if the breaking change is not so severe, it may be warranted for the extra benefits it brings. Above the array positional was mentioned, and here I believe sprintf-js would allow grabbing specific keys from objects with %(name)s while Node's util.format does not allow any way (that I am aware of) to extract only a certain key from an object.

For what its worth, trying things like that currently just pass it through as-is:

 Array format: The thing gracefully handles being %(name)s :(

expect(received).toBe(expected) // Object.is equality

Expected: false
Received: "given cool unexpected thing with example"

But would in theory result in exactly what I am looking for.

Hope this was useful input and my apologies if this is just adding noise to this thread.


In the meantime, most of these issues can be worked around with manual loopings

const tests = [{
    ...
}, {
    name: 'given cool unexpected thing with example',
    the_argument: "cool",
    another_argument: 'example',
    expected: false
}];
for ( var thisTest of tests ) {
    test(`Manual format: The thing gracefully hands being ${thisTest.name}`, async () => {
        expect(thisTest.name).toBe(thisTest.expected);
    });
}

results in

Manual format: The thing gracefully hands being given cool unexpected thing with example

expect(received).toBe(expected) // Object.is equality

Expected: false
Received: "given cool unexpected thing with example"

Also please forgive the non-sensical expectation in those example tests, it is only there to illustrate how the test is handed each object in the array the same way the table format is handed them - only the name reference is limiting.

@ryall
Copy link

ryall commented Apr 21, 2020

Super solution from @r4j4h thanks!

Just including a slightly simplified/integrated version to match the .each syntax a little more closely:

for (const [invalidPassword, expectedError] of [
  ['duck', 'short'],
  ['password', 'weak'],
  ['pineapple', 'weak'],
]) {
  test(`Cannot register with ${expectedError} password: "${invalidPassword}"`, () => {
    //...
  });
}

With TS, use a tuple to avoid type errors:

for (const [invalidPassword, expectedError] of [
  ['duck', 'short'],
  ['password', 'weak'],
  ['pineapple', 'weak'],
] as const) { // "as const" creates the array as a typed tuple
  test(`Cannot register with ${expectedError} password: "${invalidPassword}"`, () => {
    //...
  });
}

@kirill-konshin
Copy link

kirill-konshin commented May 7, 2020

@ryall @r4j4h great solution!

@SimenB @mattphillips is manual looping a valid (aka approved) use-case?

It also has one more advantage because you can define the table like so:

for (const {name, main_identity, another_argument, expected} of [
{
    name: 'given an expected thing',
    main_identity: 1,
    another_argument: 'example',
    expected: true
}, {
    name: 'given an expected thing with example',
    main_identity: 1,
    another_argument: 'example',
    expected: true
}, {
    name: 'given cool unexpected thing',
    the_argument: "cool",
    another_argument: 'example',
    expected: false
}, {
    name: 'given cool unexpected thing with example',
    the_argument: "cool",
    another_argument: 'example',
    expected: false
}
]) {
    test(`Testing with input ${name}`, () => {
        expect(name).toBe(expected);
    });
}

The only problem with this solution is integration with IDE: WebStorm won't launch the test since there will be no such control (which is good because it would fail to find a test):

image

@jeysal
Copy link
Contributor

jeysal commented Jun 6, 2020

@kirill-konshin as you noticed some integrations with editors, lint rules, etc. might not like this, but Jest itself totally supports this kind of looping officially, as long as it's all synchronous.

@kirill-konshin
Copy link

@jeysal so are there any plans to support objects in test.each? As said before, arrays are not very handy...

@murphyke
Copy link

murphyke commented Sep 2, 2020

How is this as a workaround?

test.each(testData.map(tc => [tc.description, tc]))('%s', (_, tc) => {
  expect(myFunc(tc.arg1, tc.arg2)).toBe(tc.expected);
});

@kirill-konshin
Copy link

@murphyke your code gave me the idea, what about this:

test.each([['foo', {a: 'a', b: 'b'}], ['bar', {a: 'a1', b: 'b1'}], ['baz', {a: 'a1', b: 'b1'}]])(
'test for %s',
  (testName, {a, b}) => {
    // do something
  },
)

E.g. use strings directly to include them in test name and object for the rest.

@murphyke
Copy link

murphyke commented Sep 2, 2020

@kirill-konshin I dig it; that's the best adaptation yet.

@exapsy
Copy link

exapsy commented Feb 17, 2021

@kirill-konshin gets it! 😁 Most clean way to do it without having to make your own implementations and loops that look less clean.

In typescript that's the way I did it and it's the most clean I've seen to do the job

interface Test {
  args: PageItemsArgs
  expected: Expected
}
test.each<[string, Test]>([
  ['no items', {
    args: { items: [], page: 0 },
    expected: { totalPages: 0 },
  }],
])('%s', (_, test) => { /* Test */})

@mmichaelis
Copy link

So far, I solved it very similar to @kirill-konshin and @exapsy. Pushing it even further and incorporating the suggestion by @murphyke, you can use some utility functions, if you don't like this mixed array/object syntax for defining test cases.

In the following example, I show two examples of how to handle it: Generate a test name from a name property in test data, or generating a test name just from the data, which then is very similar to the test.each table syntax:

The Types

interface NamedTest {
  name: string;
}

interface RawTestData {
  input: string;
  expected: string;
}

type TestData = NamedTest & RawTestData;

Test Data and Method Under Test

const data: TestData[] = [
  {
    name: "First",
    input: "in",
    expected: "out",
  },
  {
    name: "Second",
    input: "out",
    expected: "in",
  },
];

const underTest = (val: string): string => {
  if (val === "in") {
    return "out";
  }
  return "in";
};

Utility Functions as workaround

This is similar as already suggested by @murphyke:

// Uses the name property for a test name.
const fromNamed = <T extends NamedTest>(data: T[]): [string, T][] => {
  return data.map((d) => [d.name, d]);
};

// Uses just the data for generating a name, similar to Jest Table Syntax
const fromData = <T extends RawTestData>(data: T[]): [string, T][] => {
  return data.map((d) => [`On input '${d.input}' expecting '${d.expected}'`, d]);
};

Using Utility Methods

test.each<[string, TestData]>(fromNamed(data))("[%#] %s", (name, data) => {
  expect(underTest(data.input)).toStrictEqual(data.expected);
});

test.each<[string, TestData]>(fromData(data))("[%#] %s", (name, data) => {
  expect(underTest(data.input)).toStrictEqual(data.expected);
});

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Dec 21, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 20, 2023
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests