Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests are (uselessly) combining //~ERROR with --error-format #118752

Closed
jyn514 opened this issue Dec 8, 2023 · 4 comments · Fixed by #119184
Closed

tests are (uselessly) combining //~ERROR with --error-format #118752

jyn514 opened this issue Dec 8, 2023 · 4 comments · Fixed by #119184
Assignees
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Dec 8, 2023

with this patch applied:

diff --git a/src/tools/compiletest/src/errors.rs b/src/tools/compiletest/src/errors.rs
index c33e66e02ac..e0ec76aa027 100644
--- a/src/tools/compiletest/src/errors.rs
+++ b/src/tools/compiletest/src/errors.rs
@@ -11,7 +11,7 @@
 use regex::Regex;
 use tracing::*;
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq)]
 pub enum ErrorKind {
     Help,
     Error,
diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs
index 055671afd14..f394444056c 100644
--- a/src/tools/compiletest/src/runtest.rs
+++ b/src/tools/compiletest/src/runtest.rs
@@ -3976,23 +3976,28 @@ fn run_ui_test(&self) {
             proc_res.status,
             self.props.error_patterns
         );
-        if !explicit && self.config.compare_mode.is_none() {
         let check_patterns = should_run == WillExecute::No
             && (!self.props.error_patterns.is_empty()
                 || !self.props.regex_error_patterns.is_empty());
-
+        if !explicit && self.config.compare_mode.is_none() {
             let check_annotations = !check_patterns || !expected_errors.is_empty();
 
-            if check_patterns {
-                // "// error-pattern" comments
-                let output_to_check = self.get_output(&proc_res);
-                self.check_all_error_patterns(&output_to_check, &proc_res, pm);
-            }
-
             if check_annotations {
                 // "//~ERROR comments"
                 self.check_expected_errors(expected_errors, &proc_res);
             }
+        } else if explicit && !expected_errors.is_empty() {
+            let msg = format!(
+                "line {}: cannot combine `--error-format` with {} annotations; use `error-pattern` instead",
+                expected_errors[0].line_num,
+                expected_errors[0].kind.unwrap_or(ErrorKind::Error),
+            );
+            self.fatal(&msg);
+        }
+        if check_patterns {
+            // "// error-pattern" comments
+            let output_to_check = self.get_output(&proc_res);
+            self.check_all_error_patterns(&output_to_check, &proc_res, pm);
         }
 
         if self.props.run_rustfix && self.config.compare_mode.is_none() {

i get 10 or so test failures:

; rg error-format tests/ -l | xargs x t
---- [ui] tests/ui/proc-macro/inner-attrs.rs stdout ----

error: line 64: cannot combine `--error-format` with error annotations; use `error-pattern` instead
thread '[ui] tests/ui/proc-macro/inner-attrs.rs' panicked at src/tools/compiletest/src/runtest.rs:2768:9:
fatal error


failures:
    [ui] tests/ui/annotate-snippet/missing-type.rs
    [ui] tests/ui/lint/unused_parens_json_suggestion.rs
    [ui] tests/ui/lint/unused_parens_remove_json_suggestion.rs
    [ui] tests/ui/diagnostic-width/flag-json.rs
    [ui] tests/ui/proc-macro/issue-75930-derive-cfg.rs
    [ui] tests/ui/proc-macro/inner-attrs.rs

failures:
    [ui] tests/rustdoc-ui/issues/issue-81662-shortness.rs

the ERROR annotations in these tests are useless. either they should stop using --error-format or they should switch from ERROR to error-pattern. several of the patterns involved are not present in the stderr, so the tests are doing nothing.

i started on fixing this and then ran out of time.

diff --git a/tests/rustdoc-ui/issues/issue-81662-shortness.rs b/tests/rustdoc-ui/issues/issue-81662-shortness.rs
index 0240d217bee..79fb65dec48 100644
--- a/tests/rustdoc-ui/issues/issue-81662-shortness.rs
+++ b/tests/rustdoc-ui/issues/issue-81662-shortness.rs
@@ -1,4 +1,6 @@
 // compile-flags:--test --error-format=short
+// check-stdout
+// error-pattern:cannot find function `foo` in this scope
 // normalize-stdout-test: "tests/rustdoc-ui/issues" -> "$$DIR"
 // normalize-stdout-test "finished in \d+\.\d+s" -> "finished in $$TIME"
 // failure-status: 101
@@ -6,7 +8,6 @@
 /// ```rust
 /// foo();
 /// ```
-//~^^ ERROR cannot find function `foo` in this scope
 fn foo() {
     println!("Hello, world!");
 }
diff --git a/tests/rustdoc-ui/issues/issue-81662-shortness.stdout b/tests/rustdoc-ui/issues/issue-81662-shortness.stdout
index 6313dde32c5..f32f51e12f2 100644
--- a/tests/rustdoc-ui/issues/issue-81662-shortness.stdout
+++ b/tests/rustdoc-ui/issues/issue-81662-shortness.stdout
@@ -1,16 +1,16 @@
 
 running 1 test
-test $DIR/issue-81662-shortness.rs - foo (line 6) ... FAILED
+test $DIR/issue-81662-shortness.rs - foo (line 8) ... FAILED
 
 failures:
 
----- $DIR/issue-81662-shortness.rs - foo (line 6) stdout ----
-$DIR/issue-81662-shortness.rs:7:1: error[E0425]: cannot find function `foo` in this scope
+---- $DIR/issue-81662-shortness.rs - foo (line 8) stdout ----
+$DIR/issue-81662-shortness.rs:9:1: error[E0425]: cannot find function `foo` in this scope
 error: aborting due to 1 previous error
 Couldn't compile the test.
 
 failures:
-    $DIR/issue-81662-shortness.rs - foo (line 6)
+    $DIR/issue-81662-shortness.rs - foo (line 8)
 
 test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME
 
diff --git a/tests/ui/proc-macro/inner-attrs.rs b/tests/ui/proc-macro/inner-attrs.rs
index 1000c9c755f..00b4c1972c0 100644
--- a/tests/ui/proc-macro/inner-attrs.rs
+++ b/tests/ui/proc-macro/inner-attrs.rs
@@ -1,5 +1,5 @@
 // gate-test-custom_inner_attributes
-// compile-flags: -Z span-debug --error-format human
+// compile-flags: -Z span-debug
 // aux-build:test-macros.rs
 // edition:2018
 
diff --git a/tests/ui/proc-macro/issue-75930-derive-cfg.rs b/tests/ui/proc-macro/issue-75930-derive-cfg.rs
index e0213527c50..1e37b40c954 100644
--- a/tests/ui/proc-macro/issue-75930-derive-cfg.rs
+++ b/tests/ui/proc-macro/issue-75930-derive-cfg.rs
@@ -1,13 +1,10 @@
 // check-pass
-// compile-flags: -Z span-debug --error-format human
+// compile-flags: -Z span-debug
 // aux-build:test-macros.rs
 
 // Regression test for issue #75930
 // Tests that we cfg-strip all targets before invoking
 // a derive macro
-// We need '--error-format human' to stop compiletest from
-// trying to interpret proc-macro output as JSON messages
-// (a pretty-printed struct may cause a line to start with '{' )
 // FIXME: We currently lose spans here (see issue #43081)
 
 #![no_std] // Don't load unnecessary hygiene information from std
@@ -47,6 +44,8 @@
 // that kind of correction caused the problem seen in #76399, so maybe not.
 
 #[print_helper(a)] //~ WARN derive helper attribute is used before it is introduced
+                   //~| WARN derive helper attribute is used before it is introduced
+                   //~| WARN this was previously accepted
                    //~| WARN this was previously accepted
 #[cfg_attr(not(FALSE), allow(dead_code))]
 #[print_attr]

@rustbot label +A-testsuite +C-bug

@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. labels Dec 8, 2023
@estebank estebank added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Dec 8, 2023
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 10, 2023
@Rajveer100
Copy link
Contributor

@rustbot claim

@Rajveer100
Copy link
Contributor

Rajveer100 commented Dec 19, 2023

@jyn514 @estebank
Could you elaborate a little on this to get me going? How do we reproduce this, will these show up if I just do ./x test considering the latest pull will have the merged PR?

@jyn514
Copy link
Member Author

jyn514 commented Dec 19, 2023

@Rajveer100 apply the patch in the issue description using git apply, or type it in manually

@Rajveer100
Copy link
Contributor

Rajveer100 commented Dec 20, 2023

I do see a few tests failing, so I guess what's left is to just update the ERROR to error-pattern in the required files and make sure the tests pass.

Rajveer100 added a commit to Rajveer100/rust that referenced this issue Dec 21, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 3, 2024
…, r=davidtwco

Switch from using `//~ERROR` annotations with `--error-format` to `error-pattern`

Fixes rust-lang#118752

As noticed by `@jyn514` while working on a patch, tests failed due to `//~ERROR` annotations used in combination with the older `--error-format` which is now `error-pattern`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 4, 2024
…, r=davidtwco

Switch from using `//~ERROR` annotations with `--error-format` to `error-pattern`

Fixes rust-lang#118752

As noticed by ``@jyn514`` while working on a patch, tests failed due to `//~ERROR` annotations used in combination with the older `--error-format` which is now `error-pattern`.
@bors bors closed this as completed in 9fa0c8e Jan 4, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 4, 2024
Rollup merge of rust-lang#119184 - Rajveer100:branch-for-issue-118752, r=davidtwco

Switch from using `//~ERROR` annotations with `--error-format` to `error-pattern`

Fixes rust-lang#118752

As noticed by ```@jyn514``` while working on a patch, tests failed due to `//~ERROR` annotations used in combination with the older `--error-format` which is now `error-pattern`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants