Let debugged Go tests find their runfiles#3120
Closed
fmeum wants to merge 1 commit intobazelbuild:masterfrom
Closed
Let debugged Go tests find their runfiles#3120fmeum wants to merge 1 commit intobazelbuild:masterfrom
fmeum wants to merge 1 commit intobazelbuild:masterfrom
Conversation
Go runfiles discovery requires at least one of the runfiles variables RUNFILES_DIR or RUNFILES_MANIFEST to be set. With the script path experiment, which is enabled by default, only TEST_SRCDIR is currently extracted from the script and passed to the test binary. As a result, runfiles discovery fails on Windows (where a manifest is required) as well as when following the official runfiles discovery process, which does not include TEST_SRCDIR. This is fixed by including all RUNFILES_* variables in the environment used to start the test binary.
|
Looks like this issue got fixed in another PR? |
Contributor
Author
No, this PR resolves a different issue: Tests run with debug symbols thanks to bazel-contrib/rules_go#3029, but they will still fail to find their runfiles. The latter is fixed by setting the runfiles environment variables. |
Contributor
Author
Collaborator
|
Overall, this change looks good. I'll start the import of it. Please note that without a test, this functionality might easily break in the future again. Hence, consider to add tests with future PRs. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Checklist
Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.
Discussion thread for this change
Issue number: #2313
Description of this change
Go runfiles discovery requires at least one of the runfiles variables
RUNFILES_DIR or RUNFILES_MANIFEST to be set. With the script path
experiment, which is enabled by default, only TEST_SRCDIR is currently
extracted from the script and passed to the test binary. As a result,
runfiles discovery fails on Windows (where a manifest is required) as
well as when following the official runfiles discovery process, which
does not include TEST_SRCDIR.
This is fixed by including all RUNFILES_* variables in the environment
used to start the test binary.