From 789ce5c973c949964aaadd222630a667058da220 Mon Sep 17 00:00:00 2001 From: Harin Date: Wed, 10 Dec 2025 23:39:51 +0530 Subject: [PATCH 1/4] Fix #537: Invert UI test logic to filter by pass markers instead of error patterns --- build_system/src/test.rs | 46 ++++++++++++---------------------------- 1 file changed, 13 insertions(+), 33 deletions(-) diff --git a/build_system/src/test.rs b/build_system/src/test.rs index ca2a2a7dc2d..902026a3c68 100644 --- a/build_system/src/test.rs +++ b/build_system/src/test.rs @@ -834,8 +834,9 @@ fn valid_ui_error_pattern_test(file: &str) -> bool { .any(|to_ignore| file.ends_with(to_ignore)) } -fn contains_ui_error_patterns(file_path: &Path, keep_lto_tests: bool) -> Result { - // Tests generating errors. +fn contains_ui_error_patterns(file_path: &Path, _keep_lto_tests: bool) -> Result { + // Inverted logic: UI tests are expected to fail by default unless marked with pass markers. + // Return false (keep) for tests with pass markers, true (remove) for everything else. let file = File::open(file_path) .map_err(|error| format!("Failed to read `{}`: {:?}", file_path.display(), error))?; for line in BufReader::new(file).lines().map_while(Result::ok) { @@ -843,43 +844,22 @@ fn contains_ui_error_patterns(file_path: &Path, keep_lto_tests: bool) -> Result< if line.is_empty() { continue; } + + // Check for pass markers - these tests should be kept (return false) if [ - "//@ error-pattern:", - "//@ build-fail", - "//@ run-fail", - "//@ known-bug", - "-Cllvm-args", - "//~", - "thread", + "//@ check-pass", + "//@ build-pass", + "//@ run-pass", ] .iter() - .any(|check| line.contains(check)) + .any(|marker| line.contains(marker)) { - return Ok(true); + return Ok(false); } - - if !keep_lto_tests - && (line.contains("-Clto") - || line.contains("-C lto") - || line.contains("compile-flags: -Clinker-plugin-lto")) - && !line.contains("-Clto=thin") - { - return Ok(true); - } - - if line.contains("//[") && line.contains("]~") { - return Ok(true); - } - } - let file_path = file_path.display().to_string(); - if file_path.contains("ambiguous-4-extern.rs") { - eprintln!("nothing found for {file_path:?}"); } - // The files in this directory contain errors. - if file_path.contains("/error-emitter/") { - return Ok(true); - } - Ok(false) + + // Default: remove tests without pass markers (expected to fail by default) + Ok(true) } // # Parameters From 16cf7dc80740a8966198b497ac5d5e7504ef5842 Mon Sep 17 00:00:00 2001 From: Harin Date: Sun, 14 Dec 2025 14:38:01 +0530 Subject: [PATCH 2/4] Fix rustfmt check failure --- build_system/src/test.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/build_system/src/test.rs b/build_system/src/test.rs index 902026a3c68..265f67aaff6 100644 --- a/build_system/src/test.rs +++ b/build_system/src/test.rs @@ -846,13 +846,9 @@ fn contains_ui_error_patterns(file_path: &Path, _keep_lto_tests: bool) -> Result } // Check for pass markers - these tests should be kept (return false) - if [ - "//@ check-pass", - "//@ build-pass", - "//@ run-pass", - ] - .iter() - .any(|marker| line.contains(marker)) + if ["//@ check-pass", "//@ build-pass", "//@ run-pass"] + .iter() + .any(|marker| line.contains(marker)) { return Ok(false); } From 0e5be1bbe2dc214d6c8d5a3e57e61905526a2ec1 Mon Sep 17 00:00:00 2001 From: Harin Date: Wed, 17 Dec 2025 20:57:10 +0530 Subject: [PATCH 3/4] updated ui error pattern check logic --- build_system/src/test.rs | 49 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/build_system/src/test.rs b/build_system/src/test.rs index 265f67aaff6..bde6584d2ba 100644 --- a/build_system/src/test.rs +++ b/build_system/src/test.rs @@ -834,28 +834,67 @@ fn valid_ui_error_pattern_test(file: &str) -> bool { .any(|to_ignore| file.ends_with(to_ignore)) } -fn contains_ui_error_patterns(file_path: &Path, _keep_lto_tests: bool) -> Result { +fn contains_ui_error_patterns(file_path: &Path, keep_lto_tests: bool) -> Result { // Inverted logic: UI tests are expected to fail by default unless marked with pass markers. // Return false (keep) for tests with pass markers, true (remove) for everything else. let file = File::open(file_path) .map_err(|error| format!("Failed to read `{}`: {:?}", file_path.display(), error))?; + + let mut has_pass_marker = false; for line in BufReader::new(file).lines().map_while(Result::ok) { let line = line.trim(); if line.is_empty() { continue; } - // Check for pass markers - these tests should be kept (return false) + if [ + "//@ error-pattern:", + "//@ build-fail", + "//@ run-fail", + "//@ known-bug", + "-Cllvm-args", + "//~", + "thread", + ] + .iter() + .any(|check| line.contains(check)) + { + return Ok(true); + } + + if !keep_lto_tests + && (line.contains("-Clto") + || line.contains("-C lto") + || line.contains("compile-flags: -Clinker-plugin-lto")) + && !line.contains("-Clto=thin") + { + return Ok(true); + } + + if line.contains("//[") && line.contains("]~") { + return Ok(true); + } + // Check for pass markers if ["//@ check-pass", "//@ build-pass", "//@ run-pass"] .iter() .any(|marker| line.contains(marker)) { - return Ok(false); + has_pass_marker = true; } } + let file_path = file_path.display().to_string(); + if file_path.contains("ambiguous-4-extern.rs") { + eprintln!("nothing found for {file_path:?}"); + } + + // The files in this directory contain errors. + if file_path.contains("/error-emitter/") { + return Ok(true); + } - // Default: remove tests without pass markers (expected to fail by default) - Ok(true) + // If there are no error patterns, check for pass markers + // Keep tests with pass markers, remove tests without them + Ok(!has_pass_marker) } // # Parameters From c690eb75fcb4826e6016a69a81c37025ea8edd59 Mon Sep 17 00:00:00 2001 From: Harin Date: Wed, 17 Dec 2025 21:00:17 +0530 Subject: [PATCH 4/4] formatted code --- build_system/src/test.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build_system/src/test.rs b/build_system/src/test.rs index bde6584d2ba..6f462daad14 100644 --- a/build_system/src/test.rs +++ b/build_system/src/test.rs @@ -839,8 +839,8 @@ fn contains_ui_error_patterns(file_path: &Path, keep_lto_tests: bool) -> Result< // Return false (keep) for tests with pass markers, true (remove) for everything else. let file = File::open(file_path) .map_err(|error| format!("Failed to read `{}`: {:?}", file_path.display(), error))?; - - let mut has_pass_marker = false; + + let mut has_pass_marker = false; for line in BufReader::new(file).lines().map_while(Result::ok) { let line = line.trim(); if line.is_empty() {