-
Notifications
You must be signed in to change notification settings - Fork 322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make use of multi-language and indirect tracing #744
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, the tracer is still supposed to be resistant towards build systems that clobber nonessential environment variables like this one. Even if the concrete regression this test was introduced to catch, wouldn't it be a good idea to keep it present, as defense in depth against tracer breakage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good point - in fact, I'm not convinced this test was ever testing that the
.win32env
file was doing what it should as the tracer is resistant towards the unsetting the environment variable as you say. I've left the code as it is, and updated the comment to better describe what is going on here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
.win32env
file is how the tracer resists unsetting ...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right. So if I understand correctly, before this the test was checking that the
.win32env
file that the Action created for the new compound environment it synthesised worked correctly. Now, the test is just checking that the tracer is resistant to environment variables being unset by whatever mechanism it chooses which happens to also be creating a.win32env
file.