Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[valid-title] Allow types that yield string for test titles #1234

Closed
ColtHands opened this issue Sep 5, 2022 · 3 comments · Fixed by #1460
Closed

[valid-title] Allow types that yield string for test titles #1234

ColtHands opened this issue Sep 5, 2022 · 3 comments · Fixed by #1460

Comments

@ColtHands
Copy link

ColtHands commented Sep 5, 2022

Right now there is ability to { ignoreTypeOfDescribeName: true } that ignores any type whether its a number or a function.

How about we have a rule that restricts everything that does not yield type typeof {title} === 'string'. Or allows only the things that yield a string title.

Why?

Because a lot of codebases use action_types, enums, consts, etc.

Right now:

// "jest/valid-title": ["error", { ignoreTypeOfDescribeName: true }]

it.todo(123) // is good
// "jest/valid-title": ["error", { ignoreTypeOfDescribeName: false }]

const SOME_ACTION_TYPE = "SOME_ACTION_TYPE"
it.todo(SOME_ACTION_TYPE) // error

enum Direction = {
  Up = "Up"
}
describe(Direction.Up, () => {...}) // error
@ColtHands
Copy link
Author

Also allow ignoreTypeOfDescribeName to be applied to the it or test keywords.
Right now if jest/valid-title: ["error", { ignoreTypeOfDescribeName: true }], rule is on

const title = "should yield true"

it(title, () => {...}) // error

will error out

@G-Rath
Copy link
Collaborator

G-Rath commented Sep 5, 2022

I'm pretty lukewarm on this - tbh I'm not really sure having ignoreTypeOfDescribeName was a good idea.

Titles have two purposes:

  1. explain what is being tested
  2. make it easy to track down where a specific test lives (i.e by being able to search for the title)

By not using a string literal, you halve the effectiveness of having the title since it's a lot harder to find tests by their name alone. Meanwhile, you can already control where this rule is applied both via eslint config and inline disables which imo is better as its more explicit that you intended to use this pattern.

As such, I don't think its worth the extra complexity to use typechecking in this rule, but because we already have ignoreTypeOfDescribeName its probably fine to have a similar option for tests themselves.

@G-Rath G-Rath changed the title [New option] [valid-title] - Allow types that yield string. [valid-title] Allow types that yield string in tests Oct 26, 2023
@G-Rath G-Rath changed the title [valid-title] Allow types that yield string in tests [valid-title] Allow types that yield string for test titles Oct 26, 2023
@github-actions
Copy link

🎉 This issue has been resolved in version 27.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants