FIX(regex): @W-18696814@: Lower p-limit for windows and only read fil… #305
Conversation
stephen-carter-at-sf
force-pushed
the
sc/W-18696814
branch
from
f24be3e to
e41230f
Compare
| const violations: Violation[] = []; | ||
| let fileContents: string = await fs.promises.readFile(fileName, {encoding: 'utf8'}) |
There was a problem hiding this comment.
Before we were reading the file for every single rule. Now I changed it so that we only read a file once and then run the regex for each of its rules the on the file contents.
| const violationPromiseFunctions: (() => Promise<Violation[]>)[] = rulesToRun.map( | ||
| ruleName => () => this.scanFile(file, ruleName)); | ||
| return (await this.promiseLimiter.execute(violationPromiseFunctions)).flat(); | ||
| private async runRulesForFile(fileName: string, ruleNames: string[]): Promise<Violation[]>{ |
There was a problem hiding this comment.
I'm surprised there weren't any test changes for this fix. Are there any that make sense to add as a result of this change?
There was a problem hiding this comment.
What do you mean? These changes are implementation detail only changes for performance boost. I wouldn't expect a test to fail.
| @@ -290,9 +290,14 @@ describe('Tests for selecting rules', () => { | |||
| workspace: new engApi.Workspace('FixedId', [path.resolve('src'), path.resolve('test')]) | |||
| }; | |||
| const stubEngine1: stubs.StubEngine1 = plugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1; | |||
| expect(stubEngine1.describeRulesCallHistory).toEqual([{describeOptions: expectedDescribeOptions}]); | |||
There was a problem hiding this comment.
With cache warmed up, this test failed because it was doing an exact check on the workspace object. So reworked this test to just look to see if it has the workspace by id.
stephen-carter-at-sf
force-pushed
the
sc/W-18696814
branch
from
2d17555 to
5bf0d23
Compare
…e once