Ensure test runnable generated by rust-project works with the default rust test harness#1168
Open
nick96 wants to merge 1 commit intofacebook:mainfrom
Open
Ensure test runnable generated by rust-project works with the default rust test harness#1168nick96 wants to merge 1 commit intofacebook:mainfrom
nick96 wants to merge 1 commit intofacebook:mainfrom
Conversation
Contributor
|
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D88587950. (Because this pull request was imported automatically, there will not be any future comments.) |
Contributor
|
This is good, I like the exact and no-capture. You're running one test so may as well show all output. Note that the previous code is for Meta's internal-only TPX test runner. We don't have that but they still use it and so this code should be different depending on cfg(fbcode_build), like it is in #767, which does a similar thing. |
8ce66c9 to
67e64ce
Compare
Author
|
Thanks for the feedback @cormacrelf! I've updated my PR based on it |
Contributor
|
@nick96 would you mind running rustfmt on your changes? |
The generated runnable hardcodes "buck", rather than respecting the `--buck2-command` flag. Also, the flags passed to the rust test binary are not understood by the default rust test harness, they're from meta's internal test runner (e.g. `--print-passing-details`). Now, the command is generated from the `Buck` struct so `--buck2-command is respected`. I've also tweaked the flags that get passed through to map to the default rust test harness, perhaps there should be a way of configuring these flags for custom test harnesses? The rational for each flag is: - `--exact`: we only want to run the one test and have its full name. Since rust allows functions and modules to have the same name in the same namespace, it's possible that a test name is the prefix of another. - `--no-capture`: to give feedback as soon as possible If the `fbcode` feature is enabled then the internal test runner flags are still used. I've kept support for the custom buck2 command in all cases as I think that's universally useful.
67e64ce to
aea8399
Compare
Author
|
@Wilfred sorry for the delay. I've just pushed with formatting |
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
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.
I've been working on setting up a project with buck2 lately and ran into an issue with editor integration (specifically vscode with rust analyser).
The generated runnable doesn't work with buck2 because it hardcodes "buck". It also doesn't respect the
--buck2-commandflag and passed flags to the rust test binary that the default rust test harness does not understand (e.g.--print-passing-details). Now, the command is generated from theBuckstruct so--buck2-command is respected. I've also tweaked the flags that get passed through to map to the default rust test harness, perhaps there should be a way of configuring these flags for custom test harnesses? The rational for each flag is:--exact: we only want to run the one test and have its full name. Since rust allows functions and modules to have the same name in the same namespace, it's possible that a test name is the prefix of another.--no-capture: I'm assuming--print-passing-detailsused to do something like this?