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.
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
Rust test targets now create a test launcher allow for setting env vars #579
Rust test targets now create a test launcher allow for setting env vars #579
Changes from 8 commits
7f043a4
e29d204
3d31d1a
eb6c46a
ee32e42
249dbdd
a14c27c
6d65621
aa41b58
195828d
90c8c38
9dc8038
069bee2
7e384f3
a644c83
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Do we need to do all of this? What I had in mind was to add this into rust_test rule declaration:
And then:
0) add a rust_binary at //util/test_runner:test_runner
ctx.executable._test_runner
Does it make sense?
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 different from the process wrapper because the process wrapper is used as an executable for an action. However, the launcher/test_runner needs to be the executable returned by the rule so it gets executed when you
bazel test //:my_target
. Bazel will otherwise throw an error if you return an executable that is not created by an action of that rule. Is there another way around this? Aside from having to create the binary in an action in the rule, I what you've written is more or less what I'm doing, right?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.
There's a
toolchain.os
property I think you could use? The toolchain should be in the right configuration?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.
I wasn't sure if the toolchain would always match the configuration of
launcher
but I think in most cases it's safe to use the toolchain. UpdatedThere 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.
I think the rust_binary will deal with the extension automatically, so you won't need the installer
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 installer is just a script that helps us create the test runner/launcher in an action in the test rule. I made this target so I can use a batch file on windows and not require the use of
bash
.