Skip to content

Tests: Reduce the giant number of naming-convention tests #9691

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

Open
JoshuaKGoldberg opened this issue Jul 31, 2024 · 2 comments
Open

Tests: Reduce the giant number of naming-convention tests #9691

JoshuaKGoldberg opened this issue Jul 31, 2024 · 2 comments
Labels
accepting prs Go ahead, send a pull request that resolves this issue repo maintenance things to do with maintenance of the repo, and not with code/docs tests anything to do with testing

Comments

@JoshuaKGoldberg
Copy link
Member

Suggestion

The auto-generated naming-convention tests are by far the slowest part of packages/eslint-plugin. We've done work in the past to speed them up:

...but they still take >10 seconds for some files on the Windows CI jobs. Example from https://github.com/typescript-eslint/typescript-eslint/actions/runs/10184575611/job/28172431302?pr=9165:

 PASS  tests/index.test.ts (312 MB heap size)
 PASS  tests/rules/naming-convention/cases/accessor.test.ts (19.277 s, 880 MB heap size)
 PASS  tests/rules/naming-convention/cases/method.test.ts (37.199 s, 281 MB heap size)
 PASS  tests/areOptionsValid.test.ts (185 MB heap size)
 PASS  tests/rules/naming-convention/cases/default.test.ts (20.911 s, 485 MB heap size)
 PASS  tests/rules/naming-convention/cases/property.test.ts (31.41 s, 677 MB heap size)
 PASS  tests/rules/naming-convention/cases/autoAccessor.test.ts (15.2 s, 484 MB heap size)
 PASS  tests/rules/naming-convention/cases/classicAccessor.test.ts (17.029 s, 328 MB heap size)
 PASS  tests/rules/naming-convention/cases/parameter.test.ts (15.839 s, 320 MB heap size)
 PASS  tests/eslint-rules/prefer-const.test.ts (180 MB heap size)

Let's see if we can shrink those tests down to be less explosively many.

💖

@JoshuaKGoldberg JoshuaKGoldberg added triage Waiting for team members to take a look tests anything to do with testing repo maintenance things to do with maintenance of the repo, and not with code/docs labels Jul 31, 2024
@abrahamguo
Copy link
Contributor

Right now, what we have is basically 16,000 integration tests! Here are the counts:

{
  accessor: { valid: 425, invalid: 338, total: 763 },
  autoAccessor: { valid: 425, invalid: 338, total: 763 },
  class: { valid: 425, invalid: 338, total: 763 },
  classicAccessor: { valid: 425, invalid: 338, total: 763 },
  default: { valid: 425, invalid: 338, total: 763 },
  enum: { valid: 425, invalid: 338, total: 763 },
  enumMember: { valid: 425, invalid: 338, total: 763 },
  function: { valid: 425, invalid: 338, total: 763 },
  interface: { valid: 425, invalid: 338, total: 763 },
  method: { valid: 1275, invalid: 1014, total: 2289 },
  parameter: { valid: 425, invalid: 338, total: 763 },
  parameterProperty: { valid: 850, invalid: 676, total: 1526 },
  property: { valid: 1275, invalid: 1014, total: 2289 },
  typeAlias: { valid: 425, invalid: 338, total: 763 },
  typeParameter: { valid: 425, invalid: 338, total: 763 },
  variable: { valid: 425, invalid: 338, total: 763 },
  total: 16023
}

Would it be appropriate to split up the tests into two parts:

  1. The custom selector portion
  2. The naming convention validation portion

By doing that, we wouldn't have to worry about testing every combination of selector multiplied by every combination of naming convention.

@bradzacher
Copy link
Member

It seems pretty bonkers to me that it takes 30 seconds to run 763 non-type-aware tests.

Either there is something else at play or those windows test boxes are infinitesimally small boxes.

@JoshuaKGoldberg JoshuaKGoldberg added accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for team members to take a look labels Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting prs Go ahead, send a pull request that resolves this issue repo maintenance things to do with maintenance of the repo, and not with code/docs tests anything to do with testing
Projects
None yet
Development

No branches or pull requests

3 participants