Sane defaults

CHANGELOG.md

@@ -5,7 +5,21 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [Unreleased]
+## [Unrealeased]
+
+### Added
+
+### Fixed
+
+### Changed
+
+* Split up `Revisioned::mode` into `Revisioned::exit_status` and `Revisioned::require_annotations`
+* `Config::output_conflict_handling` is now `Error` instead of `Bless`
+
+### Removed
+
+
+## [0.23.0] - 2024-05-02
 
 ### Added
 

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ui_test"
-version = "0.23.0"
+version = "0.24.0"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 description = "A test framework for testing rustc diagnostics output"
@@ -29,7 +29,7 @@ indicatif = "0.17.6"
 prettydiff = { version = "0.6.4", default_features = false }
 annotate-snippets = { version = "0.11.2" }
 levenshtein = "1.0.5"
-spanned = "0.2"
+spanned = "0.2.1"
 
 [dependencies.regex]
 version = "1.5.5"

src/config.rs

@@ -6,7 +6,7 @@ use spanned::Spanned;
 use crate::{
     aux_builds::AuxBuilder, build_manager::BuildManager, custom_flags::run::Run,
     custom_flags::rustfix::RustfixMode, custom_flags::Flag, filter::Match,
-    per_test_config::TestConfig, rustc_stderr, Errored, Mode,
+    per_test_config::TestConfig, rustc_stderr, Errored,
 };
 use crate::{
     diagnostics::Diagnostics,
@@ -129,16 +129,14 @@ impl Config {
         ];
         comment_defaults.base().normalize_stderr = filters.clone();
         comment_defaults.base().normalize_stdout = filters;
-        comment_defaults.base().mode = Spanned::dummy(Mode::Fail {
-            require_patterns: true,
-        })
-        .into();
+        comment_defaults.base().exit_status = Spanned::dummy(1).into();
+        comment_defaults.base().require_annotations = Spanned::dummy(true).into();
         let mut config = Self {
             host: None,
             target: None,
             root_dir: root_dir.into(),
             program: CommandBuilder::rustc(),
-            output_conflict_handling: OutputConflictHandling::Bless,
+            output_conflict_handling: OutputConflictHandling::Error,
             bless_command: None,
             out_dir: std::env::var_os("CARGO_TARGET_DIR")
                 .map(PathBuf::from)
@@ -175,14 +173,10 @@ impl Config {
             });
 
         config.custom_comments.insert("run", |parser, args, span| {
-            parser.check(
-                span.clone(),
-                parser.mode.is_none(),
-                "cannot specify test mode changes twice",
-            );
             let set = |exit_code| {
                 parser.set_custom_once("run", Run { exit_code }, args.span());
-                parser.mode = Spanned::new(Mode::Pass, args.span()).into();
+                parser.exit_status = Spanned::new(0, span.clone()).into();
+                parser.require_annotations = Spanned::new(false, span.clone()).into();
 
                 let prev = parser
                     .custom

src/custom_flags/run.rs

@@ -68,7 +68,6 @@ impl Flag for Run {
         let status = output.status;
         if status.code() != Some(exit_code) {
             errors.push(Error::ExitStatus {
-                mode: format!("run({exit_code})"),
                 status,
                 expected: exit_code,
                 reason: match (exit_code, status.code()) {

src/custom_flags/rustfix.rs

@@ -11,7 +11,7 @@ use crate::{
     build_manager::BuildManager,
     parser::OptWithLine,
     per_test_config::{Comments, Revisioned, TestConfig},
-    Error, Errored, Mode,
+    Error, Errored,
 };
 
 use super::Flag;
@@ -47,9 +47,11 @@ impl Flag for RustfixMode {
         output: &Output,
         build_manager: &BuildManager<'_>,
     ) -> Result<Option<Command>, Errored> {
-        let global_rustfix = match *config.mode()? {
-            Mode::Pass | Mode::Panic => RustfixMode::Disabled,
-            Mode::Fail { .. } | Mode::Yolo => *self,
+        let global_rustfix = match config.exit_status()? {
+            Some(Spanned {
+                content: 101 | 0, ..
+            }) => RustfixMode::Disabled,
+            _ => *self,
         };
 
         let output = output.clone();
@@ -120,9 +122,10 @@ impl Flag for RustfixMode {
                     error_in_other_files: vec![],
                     error_matches: vec![],
                     require_annotations_for_level: Default::default(),
-                    mode: OptWithLine::new(Mode::Pass, Span::default()),
                     diagnostic_code_prefix: OptWithLine::new(String::new(), Span::default()),
                     custom: config.comments().flat_map(|r| r.custom.clone()).collect(),
+                    exit_status: OptWithLine::new(0, Span::default()),
+                    require_annotations: OptWithLine::default(),
                 },
             ))
             .collect(),
@@ -182,7 +185,6 @@ impl Flag for RustfixMode {
             Err(Errored {
                 command: cmd,
                 errors: vec![Error::ExitStatus {
-                    mode: "rustfix".into(),
                     expected: 0,
                     status: output.status,
                     reason: Spanned::new(

src/dependencies.rs

@@ -16,7 +16,7 @@ use crate::{
     custom_flags::Flag,
     per_test_config::TestConfig,
     test_result::Errored,
-    CommandBuilder, Config, Mode, OutputConflictHandling,
+    CommandBuilder, Config, OutputConflictHandling,
 };
 
 #[derive(Default, Debug)]
@@ -66,17 +66,8 @@ fn build_dependencies_inner(config: &Config, info: &DependencyBuilder) -> Result
     }
 
     // Reusable closure for setting up the environment both for artifact generation and `cargo_metadata`
-    let set_locking = |cmd: &mut Command| match (
-        &config.output_conflict_handling,
-        config
-            .comment_defaults
-            .base_immut()
-            .mode
-            .as_deref()
-            .unwrap(),
-    ) {
-        (_, Mode::Yolo { .. }) => {}
-        (OutputConflictHandling::Error, _) => {
+    let set_locking = |cmd: &mut Command| match config.output_conflict_handling {
+        OutputConflictHandling::Error => {
             cmd.arg("--locked");
         }
         _ => {}

src/error.rs

@@ -8,10 +8,8 @@ use std::{num::NonZeroUsize, path::PathBuf, process::ExitStatus};
 #[derive(Debug)]
 #[must_use]
 pub enum Error {
-    /// Got an invalid exit status for the given mode.
+    /// Got an invalid exit status.
     ExitStatus {
-        /// The expected mode.
-        mode: String,
         /// The exit status of the command.
         status: ExitStatus,
         /// The expected exit status as set in the file or derived from the mode.

src/lib.rs

@@ -55,7 +55,6 @@ mod tests;
 pub use cmd::*;
 pub use config::*;
 pub use error::*;
-pub use mode::*;
 
 pub use spanned;
 

src/mode.rs

@@ -1,64 +1,35 @@
 use spanned::Spanned;
 
+use crate::{per_test_config::TestConfig, Errored};
+
 use super::Error;
-use std::fmt::Display;
 use std::process::ExitStatus;
 
-#[derive(Copy, Clone, Debug)]
-/// Decides what is expected of each test's exit status.
-pub enum Mode {
-    /// The test passes a full execution of the rustc driver
-    Pass,
-    /// The rustc driver panicked
-    Panic,
-    /// The rustc driver emitted an error
-    Fail {
-        /// Whether failing tests must have error patterns. Set to false if you just care about .stderr output.
-        require_patterns: bool,
-    },
-    /// Run the tests, but always pass them as long as all annotations are satisfied and stderr files match.
-    Yolo,
-}
-
-impl Mode {
+impl TestConfig<'_> {
     #[allow(clippy::result_large_err)]
-    pub(crate) fn ok(self, status: ExitStatus) -> Result<(), Error> {
-        let expected = match self {
-            Mode::Pass => 0,
-            Mode::Panic => 101,
-            Mode::Fail { .. } => 1,
-            Mode::Yolo { .. } => return Ok(()),
+    pub(crate) fn ok(&self, status: ExitStatus) -> Result<Option<Error>, Errored> {
+        let Some(expected) = self.exit_status()? else {
+            return Ok(None);
         };
-        if status.code() == Some(expected) {
-            Ok(())
+        if status.code() == Some(*expected) {
+            Ok(None)
         } else {
-            Err(Error::ExitStatus {
-                mode: self.to_string(),
+            let span = expected.span.clone();
+            let expected = expected.content;
+            Ok(Some(Error::ExitStatus {
                 status,
                 expected,
-                reason: Spanned::dummy(
+                reason: Spanned::new(
                     match (expected, status.code()) {
                         (_, Some(101)) => "the compiler panicked",
                         (0, Some(1)) => "compilation failed, but was expected to succeed",
                         (1, Some(0)) => "compilation succeeded, but was expected to fail",
                         _ => "",
                     }
                     .into(),
+                    span,
                 ),
-            })
-        }
-    }
-}
-
-impl Display for Mode {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Mode::Pass => write!(f, "pass"),
-            Mode::Panic => write!(f, "panic"),
-            Mode::Fail {
-                require_patterns: _,
-            } => write!(f, "fail"),
-            Mode::Yolo => write!(f, "yolo"),
+            }))
         }
     }
 }

src/parser.rs

@@ -10,7 +10,6 @@ use regex::bytes::Regex;
 
 use crate::{
     custom_flags::Flag, diagnostics::Level, filter::Match, test_result::Errored, Config, Error,
-    Mode,
 };
 
 use color_eyre::eyre::Result;
@@ -107,19 +106,18 @@ impl Comments {
         self.revisioned.get(&[][..]).unwrap()
     }
 
-    pub(crate) fn mode(&self, revision: &str) -> Result<Spanned<Mode>, Errored> {
-        let mode = self
-            .find_one_for_revision(revision, "`mode` annotations", |r| r.mode.clone())?
-            .into_inner()
-            .ok_or_else(|| Errored {
-                command: Command::new(format!("<finding mode for revision `{revision}`>")),
-                errors: vec![Error::ConfigError(
-                    "no mode set up in Config::comment_defaults".into(),
-                )],
-                stderr: vec![],
-                stdout: vec![],
-            })?;
-        Ok(mode)
+    pub(crate) fn exit_status(&self, revision: &str) -> Result<Option<Spanned<i32>>, Errored> {
+        Ok(self
+            .find_one_for_revision(revision, "`exit_status` annotations", |r| {
+                r.exit_status.clone()
+            })?
+            .into_inner())
+    }
+
+    pub(crate) fn require_annotations(&self, revision: &str) -> Option<Spanned<bool>> {
+        self.for_revision(revision).fold(None, |acc, elem| {
+            elem.require_annotations.as_ref().cloned().or(acc)
+        })
     }
 }
 
@@ -151,8 +149,13 @@ pub struct Revisioned {
     /// Ignore diagnostics below this level.
     /// `None` means pick the lowest level from the `error_pattern`s.
     pub require_annotations_for_level: OptWithLine<Level>,
-    /// The mode this test is being run in.
-    pub mode: OptWithLine<Mode>,
+    /// The exit status that the driver is expected to emit.
+    /// If `None`, any exit status is accepted.
+    pub exit_status: OptWithLine<i32>,
+    /// `Some(true)` means annotations are required
+    /// `Some(false)` means annotations are forbidden
+    /// `None` means this revision does not change the base annoatation requirement.
+    pub require_annotations: OptWithLine<bool>,
     /// Prefix added to all diagnostic code matchers. Note this will make it impossible
     /// match codes which do not contain this prefix.
     pub diagnostic_code_prefix: OptWithLine<String>,
@@ -438,7 +441,8 @@ impl CommentParser<Comments> {
             error_in_other_files,
             error_matches,
             require_annotations_for_level,
-            mode,
+            exit_status,
+            require_annotations,
             diagnostic_code_prefix,
             custom,
         } = self.comments.base();
@@ -457,8 +461,11 @@ impl CommentParser<Comments> {
         if require_annotations_for_level.is_none() {
             *require_annotations_for_level = defaults.require_annotations_for_level;
         }
-        if mode.is_none() {
-            *mode = defaults.mode;
+        if exit_status.is_none() {
+            *exit_status = defaults.exit_status;
+        }
+        if require_annotations.is_none() {
+            *require_annotations = defaults.require_annotations;
         }
         if diagnostic_code_prefix.is_none() {
             *diagnostic_code_prefix = defaults.diagnostic_code_prefix;
@@ -736,13 +743,8 @@ impl CommentParser<Comments> {
                 this.error(span, "rustfix is now ran by default when applicable suggestions are found");
             }
             "check-pass" => (this, _args, span){
-                let prev = this.mode.set(Mode::Pass, span.clone());
-                // args are ignored (can be used as comment)
-                this.check(
-                    span,
-                    prev.is_none(),
-                    "cannot specify test mode changes twice",
-                );
+                _ = this.exit_status.set(0, span.clone());
+                _ = this.require_annotations = Spanned::new(false, span.clone()).into();
             }
             "require-annotations-for-level" => (this, args, span){
                 let args = args.trim();