Skip to content

Commit b35ca65

Browse files
committed
compiletest: Report filtered and --skip-ed tests in metrics
1 parent 3a64770 commit b35ca65

7 files changed

Lines changed: 72 additions & 28 deletions

File tree

src/bootstrap/src/utils/render_tests.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,9 @@ impl<'a> Renderer<'a> {
155155
}
156156

157157
fn render_test_outcome(&mut self, outcome: Outcome<'_>, test: &TestOutcome) {
158-
self.executed_tests += 1;
158+
if !matches!(outcome, Outcome::FilteredOut) {
159+
self.executed_tests += 1;
160+
}
159161

160162
if let Outcome::Ignored { reason } = outcome {
161163
self.ignored_tests += 1;
@@ -171,6 +173,7 @@ impl<'a> Renderer<'a> {
171173
match outcome {
172174
Outcome::Ok | Outcome::BenchOk => build_helper::metrics::TestOutcome::Passed,
173175
Outcome::Failed => build_helper::metrics::TestOutcome::Failed,
176+
Outcome::FilteredOut => build_helper::metrics::TestOutcome::FilteredOut,
174177
Outcome::Ignored { reason } => build_helper::metrics::TestOutcome::Ignored {
175178
ignore_reason: reason.map(|s| s.to_string()),
176179
},
@@ -209,7 +212,9 @@ impl<'a> Renderer<'a> {
209212
self.terse_tests_in_line = 0;
210213
}
211214

212-
self.terse_tests_in_line += 1;
215+
if !matches!(outcome, Outcome::FilteredOut) {
216+
self.terse_tests_in_line += 1;
217+
}
213218
self.builder.colored_stdout(|stdout| outcome.write_short(stdout, &test.name)).unwrap();
214219
let _ = std::io::stdout().flush();
215220
}
@@ -359,6 +364,9 @@ impl<'a> Renderer<'a> {
359364
&outcome,
360365
);
361366
}
367+
Message::Test(TestMessage::FilteredOut(outcome)) => {
368+
self.render_test_outcome(Outcome::FilteredOut, &outcome);
369+
}
362370
Message::Test(TestMessage::Failed(outcome)) => {
363371
self.render_test_outcome(Outcome::Failed, &outcome);
364372
self.failures.push(outcome);
@@ -371,11 +379,13 @@ impl<'a> Renderer<'a> {
371379
}
372380
}
373381

382+
#[derive(Debug)]
374383
enum Outcome<'a> {
375384
Ok,
376385
BenchOk,
377386
Failed,
378387
Ignored { reason: Option<&'a str> },
388+
FilteredOut,
379389
}
380390

381391
impl Outcome<'_> {
@@ -400,6 +410,7 @@ impl Outcome<'_> {
400410
writer.set_color(ColorSpec::new().set_fg(Some(Color::Yellow)))?;
401411
write!(writer, "i")?;
402412
}
413+
Outcome::FilteredOut => {}
403414
}
404415
writer.reset()
405416
}
@@ -425,13 +436,14 @@ impl Outcome<'_> {
425436
write!(writer, ", {reason}")?;
426437
}
427438
}
439+
Outcome::FilteredOut => {}
428440
}
429441
writer.reset()
430442
}
431443

432444
fn write_ci(&self, writer: &mut dyn WriteColor, name: &str) -> Result<(), std::io::Error> {
433445
match self {
434-
Outcome::Ok | Outcome::BenchOk | Outcome::Ignored { .. } => {}
446+
Outcome::Ok | Outcome::BenchOk | Outcome::FilteredOut | Outcome::Ignored { .. } => {}
435447
Outcome::Failed => {
436448
writer.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?;
437449
writeln!(writer, " {name} ... FAILED")?;

src/build_helper/src/metrics.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ pub mod compiletest {
3838
Ok(TestOutcome),
3939
Failed(TestOutcome),
4040
Ignored(TestOutcome),
41+
FilteredOut(TestOutcome),
4142
Timeout { name: String },
4243
Started,
4344
}
@@ -155,6 +156,7 @@ pub struct Test {
155156
pub enum TestOutcome {
156157
Passed,
157158
Failed,
159+
FilteredOut,
158160
Ignored { ignore_reason: Option<String> },
159161
}
160162

src/ci/citool/src/analysis.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,9 @@ fn report_debuginfo_statistics(suites: &[&TestSuite]) {
155155
debugger_test_record.entry(kind).or_insert(TestSuiteRecord::default());
156156
match test.outcome {
157157
TestOutcome::Passed => record.passed += 1,
158-
TestOutcome::Ignored { .. } => record.ignored += 1,
158+
TestOutcome::Ignored { .. } | TestOutcome::FilteredOut => {
159+
record.ignored += 1
160+
}
159161
TestOutcome::Failed => record.failed += 1,
160162
}
161163
}
@@ -336,7 +338,7 @@ fn aggregate_test_suites(suites: &[&TestSuite]) -> BTreeMap<String, TestSuiteRec
336338
TestOutcome::Failed => {
337339
record.failed += 1;
338340
}
339-
TestOutcome::Ignored { .. } => {
341+
TestOutcome::Ignored { .. } | TestOutcome::FilteredOut => {
340342
record.ignored += 1;
341343
}
342344
}
@@ -470,6 +472,7 @@ fn report_test_diffs(
470472
match outcome {
471473
TestOutcome::Passed => "pass".to_string(),
472474
TestOutcome::Failed => "fail".to_string(),
475+
TestOutcome::FilteredOut => "ignore".to_string(),
473476
TestOutcome::Ignored { ignore_reason } => {
474477
let reason = match ignore_reason {
475478
Some(reason) => format!(" ({reason})"),

src/ci/citool/src/test_dashboard.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ fn gather_test_suites(job_metrics: &HashMap<JobName, JobMetrics>) -> TestSuites<
7777
TestOutcome::Passed => {
7878
variant_entry.passed.push(test_metadata);
7979
}
80-
TestOutcome::Ignored { ignore_reason: _ } => {
80+
TestOutcome::Ignored { ignore_reason: _ } | TestOutcome::FilteredOut => {
8181
variant_entry.ignored.push(test_metadata);
8282
}
8383
TestOutcome::Failed => {

src/tools/compiletest/src/executor.rs

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ mod json;
2323

2424
pub(crate) fn run_tests(config: &Config, tests: Vec<CollectedTest>) -> bool {
2525
let tests_len = tests.len();
26-
let filtered = filter_tests(config, tests);
26+
let (filtered, ignored) = filter_tests(config, tests);
2727
// Iterator yielding tests that haven't been started yet.
2828
let mut fresh_tests = (0..).map(TestId).zip(&filtered);
2929

@@ -41,6 +41,11 @@ pub(crate) fn run_tests(config: &Config, tests: Vec<CollectedTest>) -> bool {
4141
let num_filtered_out = tests_len - filtered.len();
4242
listener.suite_started(filtered.len(), num_filtered_out);
4343

44+
for (id, test) in (filtered.len()..).map(TestId).zip(ignored) {
45+
let completion = TestCompletion { id, outcome: TestOutcome::FilteredOut, stdout: None };
46+
listener.test_finished(&test, &completion);
47+
}
48+
4449
// Channel used by test threads to report the test outcome when done.
4550
let (completion_tx, completion_rx) = mpsc::channel::<TestCompletion>();
4651

@@ -263,6 +268,7 @@ enum TestOutcome {
263268
Succeeded,
264269
Failed { message: Option<&'static str> },
265270
Ignored,
271+
FilteredOut,
266272
}
267273

268274
impl TestOutcome {
@@ -278,9 +284,10 @@ impl TestOutcome {
278284
/// FIXME(#139660): Now that libtest has been removed, redesign the whole filtering system to
279285
/// do a better job of understanding and filtering _paths_, instead of being tied to libtest's
280286
/// substring/exact matching behaviour.
281-
fn filter_tests(opts: &Config, tests: Vec<CollectedTest>) -> Vec<CollectedTest> {
282-
let mut filtered = tests;
283-
287+
fn filter_tests(
288+
opts: &Config,
289+
tests: Vec<CollectedTest>,
290+
) -> (Vec<CollectedTest>, Vec<CollectedTest>) {
284291
let matches_filter = |test: &CollectedTest, filter_str: &str| {
285292
if opts.filter_exact {
286293
// When `--exact` is used we must use `filterable_path` to get
@@ -293,17 +300,23 @@ fn filter_tests(opts: &Config, tests: Vec<CollectedTest>) -> Vec<CollectedTest>
293300
}
294301
};
295302

296-
// Remove tests that don't match the test filter
297-
if !opts.filters.is_empty() {
298-
filtered.retain(|test| opts.filters.iter().any(|filter| matches_filter(test, filter)));
299-
}
303+
let should_run = |test: &CollectedTest| {
304+
// Remove tests that don't match the test filter
305+
if !opts.filters.is_empty()
306+
&& !opts.filters.iter().any(|filter| matches_filter(test, filter))
307+
{
308+
return false;
309+
}
300310

301-
// Skip tests that match any of the skip filters
302-
if !opts.skip.is_empty() {
303-
filtered.retain(|test| !opts.skip.iter().any(|sf| matches_filter(test, sf)));
304-
}
311+
// Skip tests that match any of the skip filters
312+
if opts.skip.iter().any(|sf| matches_filter(test, sf)) {
313+
return false;
314+
}
315+
316+
true
317+
};
305318

306-
filtered
319+
tests.into_iter().partition(should_run)
307320
}
308321

309322
/// Determines the number of tests to run concurrently.

src/tools/compiletest/src/executor/json.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ impl Listener {
7171
maybe_message = test.desc.ignore_message.as_deref();
7272
event = "ignored";
7373
}
74+
TestOutcome::FilteredOut => {
75+
maybe_message = test.desc.ignore_message.as_deref();
76+
event = "filtered_out";
77+
}
7478
};
7579

7680
// This emits optional fields as `null`, instead of omitting them

src/tools/compiletest/tests/flags.rs

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,12 @@ impl CompiletestMetrics {
9696
Message::Test(inner @ TestMessage::Timeout { name }) => (inner, name),
9797
_ => return None,
9898
};
99-
let stripped = name.strip_prefix("[ui] tests/ui/compiletest-self-test/").unwrap()
100-
.strip_suffix(".rs").unwrap();
101-
if stripped == expected {
102-
Some(msg)
103-
} else {
104-
None
105-
}
99+
let stripped = name
100+
.strip_prefix("[ui] ")
101+
.unwrap_or_else(|| panic!("unknown name format {name}"))
102+
// This can be None if compiletest prints ignored tests from other directories.
103+
.strip_prefix("tests/ui/compiletest-self-test/");
104+
if stripped == Some(expected) { Some(msg) } else { None }
106105
})
107106
}
108107
}
@@ -129,7 +128,18 @@ impl CompiletestMetricsExt for Assert {
129128

130129
#[test]
131130
fn ignore_directive_tests_reported_ignored() {
131+
let ct = compiletest();
132+
let metrics = run_ok(ct).metrics();
133+
assert!(matches!(metrics.find_test("ignore-directive.rs").unwrap(), TestMessage::Ignored(..)));
134+
}
135+
136+
#[test]
137+
fn skipped_tests_reported_filtered() {
132138
let mut ct = compiletest();
133-
let metrics = dbg!(run_ok(ct).metrics());
134-
assert!(matches!(metrics.find_test("ignore-directive").unwrap(), TestMessage::Ignored(..)));
139+
ct.arg("--skip=ignore-directive");
140+
let metrics = run_ok(ct).metrics();
141+
assert!(matches!(
142+
metrics.find_test("ignore-directive.rs").unwrap(),
143+
TestMessage::FilteredOut(..)
144+
));
135145
}

0 commit comments

Comments
 (0)