Fix Go tracing on Windows, and fix tests #1209
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aeisenberg
approved these changes
Aug 23, 2022
henrymercer
force-pushed
the
henrymercer/fix-go-tracing-tests
branch
3 times, most recently
from
916e4f2
to
d14f5c9
Compare
henrymercer
changed the title
These were supplying CODEQL_EXTRACTOR_GO_BUILD_TRACING=true rather than CODEQL_EXTRACTOR_GO_BUILD_TRACING=on, therefore tracing wasn't being tested.
This is currently broken in CLI versions 2.10.3 and earlier.
henrymercer
force-pushed
the
henrymercer/fix-go-tracing-tests
branch
from
bd4e2cc
to
569f78c
Compare
|
Rebased on main to address a merge conflict. |
aeisenberg
reviewed
Aug 23, 2022
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
It is enough to pass the checks now that we only use the runner for PR checks.
henrymercer
force-pushed
the
henrymercer/fix-go-tracing-tests
branch
from
972fe28
to
182342c
Compare
edoardopirovano
approved these changes
Aug 24, 2022
6 tasks
4 tasks
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix Go tracing tests
We're currently supplying
CODEQL_EXTRACTOR_GO_BUILD_TRACING=truerather than the expectedCODEQL_EXTRACTOR_GO_BUILD_TRACING=onin the PR checks, therefore tracing isn't being tested.This PR updates the PR checks to the expected
CODEQL_EXTRACTOR_GO_BUILD_TRACING=onand also adds an error if it's set totrue. Makingtruea synonym ofonis more difficult than it's worth as the Go autobuilder also looks at this environment variable, so whethertrueis a synonym or not would depend on the version of the CodeQL CLI.Fix Go tracing on Windows
After fixing the tests, we discovered a bug with Go tracing on Windows with the Lua tracer. We have a fix here: github/codeql#10147. Until this fix is released, we disable Lua tracing on Windows when analyzing Go.
Merge / deployment checklist