Jest: Jest should mark test as 'skipped' if empty function is provided as second argument

[ashtonsix]

This is a feature

Background, current behavior & proposed behavior

When planning tests I would often write code like this:

describe('counter', () => {
  it('updates the value')
  it('re-renders the counter when the value is updated')
})

Until a recent release these examples would crash, I started supplying an empty function as a work-around like so (& expect others do also for various reasons):

it('updates the value', () => {})

Using the latest version of Jest both examples will work, the difference is the first example will create 2 _skipped_ tests & the second example will create a _passed_ test. I believe the tests should be skipped in both cases. Example logic for this might look like:

const shouldSkip = f => {
  if (f === undefined) return true
  f = f.toString()
  if (
    f.test(/\(\s*\)\s*=>\s*{\s*}/) ||
    f.test(/function\s*\(\s*\)\s*{\s*}/)
  ) return true
  return false
}

Using

Windows 7, Jest 17

[thymikee]

Let's discuss this topic in this issue: https://github.com/facebook/jest/issues/2209 since it refers to the same problem, but with different solutions.

[ashtonsix]

@thymikee That's a very different issue. Handling empty functions is unrelated to handling functions with no assertions. Can this be re-opened?

I write lots of tests like this which have no assertions but I certainly don't want to fail or skip where I'm just making sure the code doesn't throw:

it('renders without crashing', () => {
  mount(<Counter />)
})

[thymikee]

You shouldn't rely on this behaviour, use toThrow/toThrowError/toThrowErrorMatchingSnapshot instead.

[thymikee]

Reopening this so it can be discussed further.

[ashtonsix]

@thymikee I started writing some tests w/o assertions because create-react-app does & @gaearon recommended it on Twitter, you might want to create a relevant issue inside create-react-app to discuss.

Wasn't aware toThrowErrorMatchingSnapshot existed - thanks for letting me know! That's very useful

Happy to create a PR for this issue FYI

[thymikee]

@cpojer what do you think?

[gaearon]

You shouldn't rely on this behaviour, use toThrow/toThrowError/toThrowErrorMatchingSnapshot instead.

I wasn’t aware of that, and Create React App has shipped for many versions with test cases that rely on crash failing the test. Is this no longer how it works? Why? Do we need to tell thousands of users to change their test cases? Since which version of Jest has this behavior changed?

[gaearon]

Oh, I missed the discussion in #2209, sorry. I got the impression that this wasn't implemented.
So do we need to change anything in CRA? I'm a bit confused by this thread.

[thymikee]

@gaearon cool, nothing has changed 😄. What I mean is that the behaviour of empty tests or tests without expects may change in the future.

As for toThrow or not.toThrow I just personally find it more verbose, but based on this comment https://github.com/facebook/jest/issues/2209#issuecomment-264595440 you can rely on the behaviour currently used in CRA, as expects throw anyway.

2209 wasn't implemented and is still discussed.

[thymikee]

Now as I look at this, I like simpler approach of CRA better than not.toThrow.

[gaearon]

Thanks for clarifying!

[cpojer]

@ashtonsix I understand your confusion around the behavior change we made to make tests without a provided function skipped but I don't think we should make it so empty functions are skipped based on the source. It doesn't seem like a reliable method especially when we have an officially supported way for it. I'm sorry you got confused by the feature we added:

test('skipped test');

Also, we will always support tests that simply throw. They will turn into a failing test.

[Devid-]

With the 21 update tests as this will be skipped in jest: it('showTitle');
Is it possible to configure Jest not to skip this test ? Just like in the version 19 ?
The behaviour was interesting for me because I am reading a txt file where each row is a test. And the above test was something like a group title, it was easier to distinguish because it was a yellow circle and all other tests where green or red.
So I had something like this:

○ <<<< General Tests >>>>
✓ general1
✓ general2
✓ general3
✓ general4
○ <<<< Special Tests >>>>
✓ special1
✓ special2
✓ special3
✓ special4

[RobinDaugherty]

Somehow this discussion got derailed. The request is to have a way to add pending specs without an implementation. That way we can add specs first, then implementations. Ideally, these pending specs would actually appear as "pending". But "skipped" would be sufficient. This supports actual TDD process.

Today, the following:

      it('does not show the Cancel button');

results in the error "Missing second argument. It must be a callback function."

Like I said, this supports TDD, and is implemented in other testing frameworks. For example, rspec.

Passing an "empty function" as the second parameter might be a better developer experience, but is not necessary to implement "pending implementation" examples.

[SimenB]

Jest 24 will come with test.todo('my pending test'). From #6996

Install jest@beta to test now.