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

Change Request: Add "test name" option to ValidTestCase / InvalidTestCase #15090

Closed
1 task
iliubinskii opened this issue Sep 22, 2021 · 9 comments · Fixed by #15179
Closed
1 task

Change Request: Add "test name" option to ValidTestCase / InvalidTestCase #15090

iliubinskii opened this issue Sep 22, 2021 · 9 comments · Fixed by #15179
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects

Comments

@iliubinskii
Copy link

ESLint version

7.32.0

What problem do you want to solve?

When there are many tests it is easier to find failed test by name than by code.

What do you think is the correct solution?

Add "test name" option to ValidTestCase / InvalidTestCase

Participation

  • I am willing to submit a pull request for this change.

Additional comments

Related issue:
typescript-eslint/typescript-eslint#3920

@iliubinskii iliubinskii added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Sep 22, 2021
@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage Sep 22, 2021
@nzakas
Copy link
Member

nzakas commented Sep 23, 2021

Add "test name" option to ValidTestCase / InvalidTestCase

How would this work? Are you expecting that the name would be output instead of the code or in addition to it?

I’m not opposed to this but it seems unlikely people would use it very often as rules can have a lot of tests and it’s often hard to come up with descriptive names for each test.

@nzakas nzakas removed the triage An ESLint team member will look at this issue soon label Sep 23, 2021
@nzakas nzakas moved this from Needs Triage to Feedback Needed in Triage Sep 23, 2021
@iliubinskii
Copy link
Author

iliubinskii commented Sep 23, 2021

Thx for reply.

How would this work? Are you expecting that the name would be output instead of the code or in addition to it?

I would prefer the name (if present) to be output instead of the code. This would make output more compact.

it seems unlikely people would use it very often

1:
The name can be optional. So user can omit it if he does not find it useful.

2:
If you look at Jest syntax you will see that they have name for each test and it is even required:

it("The name of the test", () => {
  // Test code
});

3:
From my expirience, when you have many tests it is quite difficult to find failed test by code. So, I end up adding test name as comment:

[
    {
      code: `
        // Test 1
        ...
     `
   },
   {
      code: `
        // Test 2
        ...
     `
   },

it’s often hard to come up with descriptive names for each test.

This is true, but I am just using number to distinguish test "Test 1", "Test 2", etc. and it works fine.
The only problem is that I have to add it as a comment.


By the way, I would also suggest "only" option.

I find this option both in Jest and in typescript/eslint project and it is very convenient. It helps to quickly disable all tests but one without commenting out other tests.

I already have this option because I am using typescript/eslint, but those people who write directly for eslint will apreciate it.

@mdjermanovic
Copy link
Member

By the way, I would also suggest "only" option.

I find this option both in Jest and in typescript/eslint project and it is very convenient. It helps to quickly disable all tests but one without commenting out other tests.

I already have this option because I am using typescript/eslint, but those people who write directly for eslint will apreciate it.

This option already exist: https://eslint.org/docs/developer-guide/unit-tests#running-individual-tests

@iliubinskii
Copy link
Author

Sorry, probably "only" option was added after I switched to typescript/eslint.

But "name" option is still wanted.

@nzakas
Copy link
Member

nzakas commented Sep 24, 2021

I’m not opposed to adding a name option. Would you be interested in implementing it if we can point you to the code?

@iliubinskii
Copy link
Author

I tried to follow:
https://eslint.org/docs/developer-guide/development-environment

But when I run "npm test" I get this:
Screenshot
and "coverage" folder is not created. So, testing is not done.

If this can be easily solved, I could try to make an edit.

Though I would prefer this to be done by somebody familiar with this project.

@nzakas
Copy link
Member

nzakas commented Sep 28, 2021

@ilyub I'm unable to reproduce this with the master branch. I'd suggest re-syncing with the master branch, removing node_modules folder, and re-installing.

https://docs.github.com/en/github/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@G-Rath
Copy link
Contributor

G-Rath commented Oct 17, 2021

I've made #15179 implementing this

@nzakas nzakas added the accepted There is consensus among the team that this change meets the criteria for inclusion label Oct 19, 2021
@nzakas nzakas moved this from Feedback Needed to Pull Request Opened in Triage Oct 19, 2021
@nzakas
Copy link
Member

nzakas commented Oct 19, 2021

Marking as accepted as it’s a small change with no opposition.

Triage automation moved this from Pull Request Opened to Complete Oct 21, 2021
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Apr 20, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Apr 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
Archived in project
Triage
Complete
Development

Successfully merging a pull request may close this issue.

4 participants