Improving Eslint Test Performance

[ethanresnick]

I'm running betterer on a large TS codebase, and using it to track progress towards enabling a handful of eslint rules. Because I want betterer to track progress toward (and regressions on) each lint rule separately, I create one betterer test per rule.

However, this leads betterer to start up a new ESLint instance for each betterer test, and each ESLint instance seems to result in a new load + parse + partial typecheck of our code for the Typescript-enabled lint rules. The result is a huge amount of repeat work that makes things very slow.

Concretely:

• I'm running on a 12 core machine and I have only 5 eslint betterer tests (all of which inherit an eslint config that has the @typescript-eslint/parser)
• Just running tsc --noEmit on my codebase, without any tsbuildinfo cache, takes almost 15 seconds. Therefore, I assume this is close to the lower bound for the linting time of my type-enabled rules.
• Currently, my betterer run takes 45 seconds with only 5 tests.

If I simply merge all my 5 eslint tests into one betterer test with 5 eslint rules, betterer's runtime drops to only 20s. To me, this strongly suggests that all the repeat ESLint initialization is the issue.

Of course, if I create one composite betterer test, then I lose the ability to track progress/regressions on each individual lint rule. So, I'm wondering: would it make sense for betterer to be run one test function containing multiple eslint rules, but then split the violations out somehow (by error message, rule name, whatever) for tracking as if they were separate tests? Or, do you know of any other way to speed things up?

45 seconds is pretty excruciatingly slow for a precommit hook (and the precommit hook is barely any faster than a full betterer run, because ts-eslint still has to resolve a bunch of files for typechecking even if the list of staged files is small). My codebase isn't super amenable to being split up into TS projects, which would be part of an obvious mitigation here, but, even if I could do that, betterer's still gonna end up triggering a bunch of repeat work.

So, what do you think of the idea of one test function (with the shared state possibilities that allows) being able to generate output for multiple 'logical' tests?

[phenomnomnominal]

Hey @ethanresnick thanks for the issue! Yeah this is something that has come up a bit, and as you've noted, the usual answer I've given is to combine the tests into one. Are you able to give me a bit more insight into why you're looking to track them separately, just for the context?

I'd be curious to see what happened if you were to have a local copy of the ESLint test that instantiates the ESLint instance outside the main test and reuses that - would you give that go?

The general idea is sound I think though, but might not be something I get to in the near future!

[ethanresnick]

Thanks @phenomnomnominal! Btw, I continue to love Betterer and can't wait for v6 :)

Re separate tracking, I think it's mostly nice for two things. First, it makes it easier to see when a given lint rule's violations have been entirely fixed, which is the signal that I should move it out of betterer and into my main eslint config. Second, it makes it easier to see when the lint rule's violations have been almost entirely fixed, which I've found can motivate a developer to say "hey, this rule only has a handful of errors left, and I'm in the mood to do a bit of cleanup work, so let me just fix the remaining migrations and complete/kill off that betterer test".

As I've been thinking about this problem a little bit more, though, it seems like there may also need to be changes to how @betterer/eslint invokes Eslint when the betterer tests are being triggered by the Betterer VS Code extension. Specifically, I think typescript-eslint has some logic to detect when it's being run in a persistent/editor context, and it'll preserve the TS.Program information if so, so the Betterer VS Code extension has to make sure that logic is being triggered.

Currently, that doesn't seem to happen and, because initializing typescript-eslint takes us about 15s for our project and we edit files much faster than that, the queue of validation tasks in the Betterer VS Code extension would just keep growing indefinitely/the extension kept falling more and more behind, and we wouldn't get any in-editor betterer warning messages. So, we just had to give up on the VS Code extension entirely.

I'd be curious to see what happened if you were to have a local copy of the ESLint test that instantiates the ESLint instance outside the main test and reuses that - would you give that go?

I'll try to do it at some point, but no promises on when haha. For now, we just decided to combine all the rules into one test, and to run the standard VSCode ESLint extension pointed at a modified config that extends our current config with all these rules we'd like to eventually enable, so that we can at least get in-editor warnings about those.