vscode-codeql
vscode-codeql copied to clipboard
Remove use of CLI server for resolving tests
This PR changes the test resolver to use an implementation in the extension instead of calling out to the CLI server's resolve tests --strict-test-discovery
. This results in improved performance in the extension due to less blocking operations on the CLI server. It's also more fault tolerant and is able to produce a result for the internal CodeQL repository.
I've verified that for github/codeql
the results are exactly the same: all tests resolved by the CLI server are present in the list of this implementation and vice-versa. For the internal CodeQL repository, the only difference is that this version finds more results because it follows symlinks, which resolve tests
doesn't do.
In the internal CodeQL repository, this implementation takes about 50 seconds to complete. codeql resolve tests
takes about 4 seconds, but I'm not sure if that list is complete because it completes with an error. In github/codeql
, this implementation takes 9 seconds while resolve tests
takes about 3 seconds. Almost all of the time in this implementation is in the await pathExists(expectedFile)
call. Removing it makes it complete in less than a second.
It's easiest to review this commit-by-commit.
Checklist
- [ ] CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
- [ ] Issues have been created for any UI or other user-facing changes made by this pull request.
- [ ] [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the
ready-for-doc-review
label there.
Almost all of the time in this implementation is in the
await pathExists(expectedFile)
call. Removing it makes it complete in less than a second.
Could you point to where this call is made? I'm struggling to find it. Or was this performance optimization already made?
Could you point to where this call is made? I'm struggling to find it. Or was this performance optimization already made?
Sorry, I already removed this call as part of this commit. It was in qltest-discovery.ts
before that. I worked around that slow call by discovering all .expected
files beforehand, which is much faster.