compiletest: Report filtered and --skip-ed tests in metrics #155555
compiletest: Report filtered and --skip-ed tests in metrics #155555jyn514 wants to merge 4 commits intorust-lang:mainfrom
Conversation
|
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. Some changes occurred in src/tools/compiletest cc @jieyouxu |
rustbot
added
A-compiletest
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
jyn514
force-pushed
the
jyn/compiletest-integration-tests
branch
2 times, most recently
from
2235bed to
c4f30fb
Compare
rustbot
added
A-CI
This comment has been minimized.
This comment has been minimized.
jyn514
force-pushed
the
jyn/compiletest-integration-tests
branch
from
c4f30fb to
a04daaf
Compare
This comment has been minimized.
This comment has been minimized.
jyn514
force-pushed
the
jyn/compiletest-integration-tests
branch
from
a04daaf to
b35ca65
Compare
This comment has been minimized.
This comment has been minimized.
| #[derive(serde_derive::Deserialize, Debug)] | ||
| pub struct TestOutcome { | ||
| pub name: String, | ||
| pub exec_time: Option<f64>, |
There was a problem hiding this comment.
Do we standardize on a particular unit for time in this file? Maybe time_secs, or add comments indicating expected unit?
| } | ||
|
|
||
| #[derive(serde_derive::Deserialize, Debug)] | ||
| pub struct BenchOutcome { |
There was a problem hiding this comment.
Huh, I didn't know compiletest ever ran benchmarks... maybe I'm misreading though and some of these are really not from compiletest? Maybe we can add a comment on mod compiletest indicating the mixed origin?
There was a problem hiding this comment.
This is output that compiletest is parsing, not that compiletest is emitting. I'll leave a comment.
There was a problem hiding this comment.
ah no I'm wrong ... I'm not sure why this is here. I think left over from before the refactor away from libtest?
There was a problem hiding this comment.
in any case, I'd prefer to hear from a reviewer they're ok with the overall approach before I spend a bunch of time doing cleanups.
| match outcome { | ||
| TestOutcome::Passed => "pass".to_string(), | ||
| TestOutcome::Failed => "fail".to_string(), | ||
| TestOutcome::FilteredOut => "ignore".to_string(), |
There was a problem hiding this comment.
Should this be ignore (filtered out)?
jyn514
force-pushed
the
jyn/compiletest-integration-tests
branch
from
c2f5e3c to
1552ae4
Compare
This comment has been minimized.
This comment has been minimized.
5 participants
Ferrocene depends on these metrics for our certification. For various reasons our CI invokes bootstrap with
--skipflags, so we want to report ignored tests even if they're ignored manually with--skip.To make sure this doesn't regress, add an integration test suite for compiletest itself. This can't fit into the existing
compiletest-self-testsuite because it's testing the interface between bootstrap and compiletest, not just whether the tests pass or fail. Hopefully this will come in handy for more things in the future.Changes in this PR:
build_helper. DeriveDebugon all its types.FilteredOutmetric kind. Handle it appropriately (by not doing anything) in progress reporting, but save it to the metrics.json file.