Re-allowed single string arguments for pending tests

[pk-nb]

Summary

This PR updates the logic in the jasmine and circus reporters to allow valid single string arguments. It still keeps the nice feature work from #5558 that noisily fails on accidental pending tests or other incorrect arguments.

This solves #6430 and #6256, allowing teams (mine included) and individuals that are used to starting off with writing pending tests to continue to use this workflow. I appreciate the flexibility from the contributors on this and think moving to a linter rule would be great for those who want to avoid committing or merging pending tests.

EDIT: It looks like there is already a linter rule for avoiding single-argument (pending) tests here https://github.com/jest-community/eslint-plugin-jest/blob/master/docs/rules/no-disabled-tests.md so I think we should have everything to merge this.

Let me know if I can address any concerns! Thanks.

Test plan

Snapshots were updates with the pending style again. For the package tests, I opted to leave the tests in as a not.toThrowError to ensure this doesn't regress unintentionally. I used the regex form for the jasmine tests as they throw a different error with nesting it blocks.

I'm not sure why the circus suite is not running on Circle as it was working locally with JEST_CIRCUS=1. Any assistance would be helpful!

[gavboulton]

+1 for this. Is there anything I can do to help?

[pk-nb]

@gavboulton if you'd like to diagnose why the circus suite is failing and rebase, that would be a big help in getting this mergeable!

[gavboulton]

With JEST_CIRCUS=1 as an environment variable, the tests in packages/jest-jasmine2/src/tests/it_test_error.test.js fail locally for me so at least this is consistent with CircleCI. I need to look into Jest Circus more to attempt to understand why.

[SimenB]

What about copying Ava, which uses a modifier (test.todo)? It can then throw if implementation is provided.

@thymikee @rickhanlonii thoughts on that?

[mattphillips]

@SimenB I think test.todo might be a better solution to this problem, at least it’s more explicit and users won’t need to know anything about Jest to understand what it’s doing :)

[arcdev1]

I don't think it's worth re-litigating the need for this. That was already done in #6430 and #6256 and there's clearly a desire/need for this behaviour. We should focus instead on making this work and try to support @gavboulton

[SimenB]

The whole point of removing the functionality in the first place was to remove a footgun. Reintroducing it behind explicit function calls seems like a good middleground to me. We can also update the error message on just a string to include something like "if you meant to create a placeholder test, please use test.todo instead".

[mattphillips]

Again I completely agree with @SimenB.

@arcdev1 we removed this behaviour because of various issues raised (sorry on mobile so cannot link them right now) so there is as much interest in having this as a feature as there is to not have it.

For me being explicit wins.

[gavboulton]

Thanks for the input guys.

My preference would still be test('something yet to be implemented'). But it's a preference. If its removal helps other, then I'm for that.

test.todo seems like a good middle ground between the original functionality and test.skip with a noop.

[arcdev1]

Seems I may have jumped the gun. I took the existence of this pull request along with the comment by @rickhanlonii as evidence that this was a done deal.

FWIW I agree that test.todo seems to be a reasonable compromise. Though, I would still prefer the old behaviour: test('something yet to be implemented'). Maybe we put the original behaviour behind a flag for safety?

[mattphillips]

@gavboulton @arcdev1 I've just opened #6996 to add test.todo feel free to take a look and share any thoughts :)

[arcdev1]

@mattphillips #6996 looks pretty slick. Good job!

[rickhanlonii]

@pk-nb what do you think about closing this in favor of the test.todo feature above and the config I outlined here

[pk-nb]

Looks great! Thanks everyone for coming up with a nice solution

[github-actions]

This pull request 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.