Test cleanups#1611
Conversation
21c5911 to
0a63a06
Compare
04b0e83 to
81c79b2
Compare
- Avoid Installing `xvfb` since it is already available.
- Ensure `supportsNewQueryServer()` takes the CLI version into account
- Always run the new query server tests on v2.11.1 and later
- Avoid printing directory contents in `run-remote-query-tests`
- Run tests with `--disable-workspace-trust` to avoid a non-fatal error
being thrown from the dialog service.
- Ensure the exit code of the extension host while running integration
tests is the exit code of the actual process. Otherwise, there is
a possibility that an error exit code is swallowed up and ignored.
- Remove a duplicate unhandledRejection handler.
- Handle Exit code 7 from windows. This appears to be a failure on
exiting and unrelated to the tests.
- Fix handling of configuration in tests:
1. It is not possible to update a configuration setting for internal
settings like `codeql.canary`.
2. On windows CI, updating configuration after global teardown. So,
on tests avoid resetting test configuration when tests are over.
Also, I tried to remove all those pesky errors in the logs like:
> [2094:1017/235357.424002:ERROR:bus.cc(398)] Failed to connect to the bus: Could not parse server address: Unknown address type (examples of valid types are "tcp" and on UNIX "unix")
I was following advice from here, but I can't get it working.
- microsoft/vscode-test#127
- electron/electron#31981
81c79b2 to
c85ef15
Compare
|
This is ready for review now. I think CI is now passing regularly. Some of the changes might be controversial, so please have a review @github/code-scanning-secexp-reviewers and @github/codeql-vscode-reviewers. |
robertbrignull
left a comment
There was a problem hiding this comment.
Thank you for putting in the effort to make the tests runs more reliably and cleanly! ❤️
I don't have context on most of the changes here, but I've had a look through those bits I do understand.
| parent?: Setting; | ||
|
|
||
| constructor(name: string, parent?: Setting) { | ||
| constructor(name: string, parent?: Setting, public readonly isHidden = false) { |
There was a problem hiding this comment.
What's the definition of a hidden config setting?
I think the set that are visible to users are the ones listed under contributes.configuration.properties in the package.json. I believe this list is:
codeQL.cli.executablePath
codeQL.logInsights.joinOrderWarningThreshold
codeQL.queryHistory.format
codeQL.queryHistory.ttl
codeQL.resultsDisplay.pageSize
codeQL.runningQueries.autoSave
codeQL.runningQueries.cacheSize
codeQL.runningQueries.customLogDirectory
codeQL.runningQueries.debug
codeQL.runningQueries.maxPaths
codeQL.runningQueries.maxQueries
codeQL.runningQueries.memory
codeQL.runningQueries.numberOfThreads
codeQL.runningQueries.quickEvalCodelens
codeQL.runningQueries.saveCache
codeQL.runningQueries.timeout
codeQL.runningTests.additionalTestArguments
codeQL.runningTests.numberOfThreads
codeQL.telemetry.enableTelemetry
codeQL.telemetry.logTelemetry
codeQL.variantAnalysis.controllerRepo
codeQL.variantAnalysis.repositoryLists
And the list of all settings defined in this file is:
codeQL.cli.executablePath
codeQL.logInsights.joinOrderWarningThreshold
codeQL.queryHistory.format
codeQL.queryHistory.ttl
codeQL.resultsDisplay.pageSize
codeQL.runningQueries.autoSave
codeQL.runningQueries.cacheSize
codeQL.runningQueries.customLogDirectory
codeQL.runningQueries.debug
codeQL.runningQueries.maxPaths
codeQL.runningQueries.maxQueries
codeQL.runningQueries.memory
codeQL.runningQueries.numberOfThreads
codeQL.runningQueries.quickEvalCodelens
codeQL.runningQueries.saveCache
codeQL.runningQueries.timeout
codeQL.runningTests.additionalTestArguments
codeQL.runningTests.numberOfThreads
codeQL.telemetry.enableTelemetry
codeQL.telemetry.logTelemetry
codeQL.variantAnalysis.controllerRepo
codeQL.variantAnalysis.repositoryLists
So by this definition the set of "hidden" settings is:
codeQL.astViewer.disableCache
codeQL.canary
codeQL.canaryQueryServer
codeQL.cli.includePrerelease
codeQL.cli.personalAccessToken
codeQL.variantAnalysis.actionBranch
codeQL.variantAnalysis.liveResults
codeQL.variantAnalysis.repositoryListsPath
telemetry.enableTelemetry
There was a problem hiding this comment.
Thank you for a detailed look here. You're right. I should be making the settings above "hidden". Now that I think about it, I can do this more precisely and compare the settings defined in the config module with those defined in package.json. This way, we won't need to remember to mark config settings as hidden. It will happen programmatically.
| async afterAll() { | ||
| // Restore all settings to their default values after each test suite | ||
| await Promise.all(TEST_SETTINGS.map(setting => setting.restoreToInitialValues())); | ||
| // Only do this outside of CI since the sometimes hangs on CI. |
There was a problem hiding this comment.
Why does this hang in the afterAll but not in the beforeEach?
There was a problem hiding this comment.
I'm not exactly sure why. It looks like by the time the afterAll the workspace is already shut down and errors are thrown when you try to write settings. This seems to have been causing sporadic issues with the cli-integration tests on windows. The error stack traces are helpfully(!) hidden unless you run vscode with verbose logging.
42cc30b to
881bf51
Compare
f00c4ce to
7637f94
Compare
xvfbsince it is already available.supportsNewQueryServer()takes the CLI version into accountrun-remote-query-tests--disable-workspace-trustto avoid a non-fatal errorbeing thrown from the dialog service.
Also, I tried to remove all those pesky errors in the logs like:
I was following advice from here, but I can't get it working.