Custom buck test runner #147
Conversation
facebook-github-bot
added
the
CLA Signed
tools/test-runner/src/main.rs
Outdated
| env: HashMap<String, String>, | ||
|
|
||
| /// Labels that are defined on the test rule. NOTE: not used | ||
| labels: Vec<String>, |
There was a problem hiding this comment.
Just leaving a note here - this is a good place if we ever need a way to control the test runner behavior from the unitttest target itself - it's a common pattern we use internally but I don't see a need for it in oss yet
tools/test-runner/src/buck_test.rs
Outdated
| /// Absolute paths to any files required by this test target. | ||
| required_paths: Option<Vec<PathBuf>>, | ||
| } | ||
| fn default_kind() -> TestKind { |
There was a problem hiding this comment.
Does this allow falling back to shell tests for the unsupported gtest type? If so cool, otherwise I still like it :)
There was a problem hiding this comment.
I don't think so. This default only applies when the JSON doesn't have a type tag at all, so If serde finds "type" : <something> in the test spec, <something> needs to match some case in the TestKind enum.
This means that, right now, passing gtest will give a syntax error when parsing the JSON spec
There was a problem hiding this comment.
After realizing buck does add a "custom" tag when no type is provided in sh_test, I removed this default.
In the future, we could change the parsing behavior so as to assign the Shell type to anything not recognized. This would mean implementing a custom serde Deserialize for the TestKind enum.
| return Ok(()); | ||
| } | ||
|
|
||
| fn xml_escape_text(unescaped: String) -> String { |
There was a problem hiding this comment.
This is fine for now, but may end up causing unexpected issues in the future. Don't fix it in this PR, but serde_xml_rs may work (https://docs.rs/serde-xml-rs/0.5.0/serde_xml_rs/) - if not (there are some things that it can't serialize) then I have used quick_xml in the past
There was a problem hiding this comment.
Yeah since I had a very specific format to generate, I thought doing it by hand would be easier.
Later on I realized we would need to escape stdout/err, so using a dedicated library would probably be less error prone.
| } | ||
|
|
||
| if !error.is_empty() { | ||
| bail!("Invalid context for test target {}\n{}", spec.target, error); |
There was a problem hiding this comment.
I like how you collect all the errors instead of just bailing on the first one :)
|
@vmagro has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Custom buck test runner for #145
This PR provides an external test runner which is currently able to list out individual unit tests from Python and Rust binaries coming from buck tests, execute them in parallel and report results in JUnit's XML format, which should be CI-compatible (although integration with the test reporter is yet to be tested).
It can also retry tests and will silently ignore those manually marked for exclusion (for instance, those with a
"disabled"label).The next step after this is implementing the rest of #145, namely a way to keep track of state between test runs on the CI so as to automatically disable some tests.