Use sarif parser for reopened results
When we attempt to re-open query results, instead of using the streaming SARIF parser written in #1004, we used JSON.parse(). This couldn't handle large SARIF files and we ran into an error.
This change uses the streaming SARIF parser instead. Note that this is a draft pull request as it is untested locally. cc @aschackmull (sorry, wrong Anders tagged at first!)
Checklist
- [ ] CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
- [x] Issues have been created for any UI or other user-facing changes made by this pull request.
- [x] [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-reviewlabel there.
Looks like there are some genuine unit test errors on the method I changed so this will need more 🕵️ work
I can't tell why the windows tests are failing. It seems to be failing consistently. If you have access to a windows box, maybe try running the tests there.
I can't tell why the windows tests are failing. It seems to be failing consistently. If you have access to a windows box, maybe try running the tests there.
I don't have one, but will 🍐 with Dave tomorrow on this!
Now, looks like the tests are failing because interpreted.json is trying to be deleted, but a lock on it is being held by another process or something. It has to do with the safeDel method. For some reason, it is not able to be deleted and it looks like the tests can't even read it.
Not a pleasing solution, but perhaps, you can use a different name for interpreted results file for each test. This way, you won't need to delete it in afterEach and you can be sure you can read it in each test.
Now, looks like the tests are failing because
interpreted.jsonis trying to be deleted, but a lock on it is being held by another process or something. It has to do with thesafeDelmethod. For some reason, it is not able to be deleted and it looks like the tests can't even read it.Not a pleasing solution, but perhaps, you can use a different name for interpreted results file for each test. This way, you won't need to delete it in
afterEachand you can be sure you can read it in each test.
Dave and I just came up with something like this in pairing too. It looked like there was a separate issue with Windows Defender that needed a sleep after the file was written (because it couldn't be opened while Windows Defender was scanning it), so we added 10s sleep as well. After that we discovered the problem in the final test where it wasn't able to be deleted/read.
I've renamed the file for just the last test to see if it passes for now (can't reproduce locally without a Windows machine). I can also rename the others for consistency.
Not sure if you've tried this already, but you can launch the test version of vscode with --verbose and --log trace` to get some more info while you are running. Augment the CLI options in getLaunchArgs.
Phew, renaming the path worked 😸 so this PR is finally ready for re-review