From c1b75114cee9764fdcc5aa24e6c9ff6e79ae7895 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 11 Sep 2024 10:59:15 +0200 Subject: [PATCH 1/3] Reject colons in target specs --- CHANGELOG.md | 2 ++ src/config.rs | 4 ++-- src/parser.rs | 44 ++++++++++++++++++++++++++++++++++++++++---- src/parser/tests.rs | 27 +++++++++++++++++++++++++-- 4 files changed, 69 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2249199..5c210fc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +* `only`/`ignore` filters now only accept integers, alphabetic characters, `-` and `_` + ### Removed ## [0.26.4] - 2024-09-09 diff --git a/src/config.rs b/src/config.rs index 4b498b3c..6d489d07 100644 --- a/src/config.rs +++ b/src/config.rs @@ -395,8 +395,8 @@ impl Config { let target = self.target.as_ref().unwrap(); match condition { Condition::Bitwidth(bits) => bits.iter().any(|bits| self.get_pointer_width() == *bits), - Condition::Target(t) => t.iter().any(|t| target.contains(t)), - Condition::Host(t) => t.iter().any(|t| self.host.as_ref().unwrap().contains(t)), + Condition::Target(t) => t.iter().any(|t| target.contains(&**t)), + Condition::Host(t) => t.iter().any(|t| self.host.as_ref().unwrap().contains(&**t)), Condition::OnHost => self.host_matches_target(), } } diff --git a/src/parser.rs b/src/parser.rs index 26e3d7be..4c5d57a5 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -225,15 +225,51 @@ impl std::ops::DerefMut for CommentParser { #[derive(Debug, Clone)] pub enum Condition { /// One of the given strings must appear in the host triple. - Host(Vec), + Host(Vec), /// One of the given string must appear in the target triple. - Target(Vec), + Target(Vec), /// Tests that the bitwidth is one of the given ones. Bitwidth(Vec), /// Tests that the target is the host. OnHost, } +#[derive(Debug, Clone)] +/// A sub string of a target (or a whole target). +/// Effectively a `String` that only allows lowercase chars, integers and dashes +pub struct TargetSubStr(String); + +impl PartialEq<&str> for TargetSubStr { + fn eq(&self, other: &&str) -> bool { + self.0 == *other + } +} + +impl std::ops::Deref for TargetSubStr { + type Target = str; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl TryFrom for TargetSubStr { + type Error = String; + + fn try_from(value: String) -> std::result::Result { + if value + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') + { + Ok(Self(value)) + } else { + Err(format!( + "target strings can only contain integers, basic alphabet characters or dashes" + )) + } + } +} + #[derive(Debug, Clone)] /// An error pattern parsed from a `//~` comment. pub enum Pattern { @@ -270,8 +306,8 @@ impl Condition { })).collect::, _>>()?; Ok(Condition::Bitwidth(bits)) } - "target" => Ok(Condition::Target(args.map(|arg|arg.to_owned()).collect())), - "host" => Ok(Condition::Host(args.map(|arg|arg.to_owned()).collect())), + "target" => Ok(Condition::Target(args.map(|arg|TargetSubStr::try_from(arg.to_owned())).collect::>()?)), + "host" => Ok(Condition::Host(args.map(|arg|TargetSubStr::try_from(arg.to_owned())).collect::>()?)), _ => Err(format!("`{c}` is not a valid condition, expected `on-host`, /[0-9]+bit/, /host-.*/, or /target-.*/")), } } diff --git a/src/parser/tests.rs b/src/parser/tests.rs index 0687cfa9..abe6db1a 100644 --- a/src/parser/tests.rs +++ b/src/parser/tests.rs @@ -255,7 +255,10 @@ fn parse_x86_64() { let revisioned = &comments.revisioned[&vec![]]; assert_eq!(revisioned.only.len(), 1); match &revisioned.only[0] { - Condition::Target(t) => assert_eq!(t, &["x86_64-unknown-linux"]), + Condition::Target(t) => { + assert_eq!(t.len(), 1); + assert_eq!(t[0], "x86_64-unknown-linux") + } _ => unreachable!(), } } @@ -279,7 +282,27 @@ fn parse_two_only_filters() { let revisioned = &comments.revisioned[&vec![]]; assert_eq!(revisioned.only.len(), 1); match &revisioned.only[0] { - Condition::Target(t) => assert_eq!(t, &["hello", "world"]), + Condition::Target(t) => { + assert_eq!(t.len(), 2); + assert_eq!(t[0], "hello"); + assert_eq!(t[1], "world") + } _ => unreachable!(), } } + +#[test] +fn parse_invalid_filter() { + let s = r"//@only-target: hello world: somecomment"; + Comments::parse( + Spanned::new( + s.as_bytes(), + Span { + file: PathBuf::new(), + bytes: 0..s.len(), + }, + ), + &Config::dummy(), + ) + .unwrap_err(); +} From 62a7abadbf33dd69d47ef2df1348c8046068c3b8 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 11 Sep 2024 11:38:38 +0200 Subject: [PATCH 2/3] Allow `#` comments in only/ignore filters --- CHANGELOG.md | 1 + src/parser.rs | 6 +++--- src/parser/tests.rs | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c210fc1..ee22e7bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed * `only`/`ignore` filters now only accept integers, alphabetic characters, `-` and `_` +* `only`/ `ignore` filters allow comments by ignoring everything from an `#` onwards ### Removed diff --git a/src/parser.rs b/src/parser.rs index 4c5d57a5..e0f755f8 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -306,8 +306,8 @@ impl Condition { })).collect::, _>>()?; Ok(Condition::Bitwidth(bits)) } - "target" => Ok(Condition::Target(args.map(|arg|TargetSubStr::try_from(arg.to_owned())).collect::>()?)), - "host" => Ok(Condition::Host(args.map(|arg|TargetSubStr::try_from(arg.to_owned())).collect::>()?)), + "target" => Ok(Condition::Target(args.take_while(|&arg| arg != "#").map(|arg|TargetSubStr::try_from(arg.to_owned())).collect::>()?)), + "host" => Ok(Condition::Host(args.take_while(|&arg| arg != "#").map(|arg|TargetSubStr::try_from(arg.to_owned())).collect::>()?)), _ => Err(format!("`{c}` is not a valid condition, expected `on-host`, /[0-9]+bit/, /host-.*/, or /target-.*/")), } } @@ -612,7 +612,7 @@ impl CommentParser { } Some(i) => { let (command, args) = command.split_at(i); - // Commands are separated from their arguments by ':' or ' ' + // Commands are separated from their arguments by ':' let next = args .chars() .next() diff --git a/src/parser/tests.rs b/src/parser/tests.rs index abe6db1a..51c30f73 100644 --- a/src/parser/tests.rs +++ b/src/parser/tests.rs @@ -265,7 +265,7 @@ fn parse_x86_64() { #[test] fn parse_two_only_filters() { - let s = r"//@only-target: hello world"; + let s = r"//@only-target: hello world # some comment"; let comments = Comments::parse( Spanned::new( s.as_bytes(), From 8ea47ced4664ecdb69f20e4b582d0813a8d0281d Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 11 Sep 2024 12:18:38 +0200 Subject: [PATCH 3/3] Allow the creation of build managers that cannot create nested builds --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/build_manager.rs | 7 ++++++- tests/integrations/basic-bin/Cargo.lock | 2 +- tests/integrations/basic-fail-mode/Cargo.lock | 2 +- tests/integrations/basic-fail/Cargo.lock | 2 +- tests/integrations/basic/Cargo.lock | 2 +- tests/integrations/cargo-run/Cargo.lock | 2 +- tests/integrations/dep-fail/Cargo.lock | 2 +- tests/integrations/ui_test_dep_bug/Cargo.lock | 2 +- 10 files changed, 15 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 029cb8fc..f1131f15 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -592,7 +592,7 @@ dependencies = [ [[package]] name = "ui_test" -version = "0.26.4" +version = "0.26.5" dependencies = [ "annotate-snippets", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index 78aeb34f..8448442e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ui_test" -version = "0.26.4" +version = "0.26.5" edition = "2021" license = "MIT OR Apache-2.0" description = "A test framework for testing rustc diagnostics output" diff --git a/src/build_manager.rs b/src/build_manager.rs index 325b0fab..ff0a41ec 100644 --- a/src/build_manager.rs +++ b/src/build_manager.rs @@ -7,7 +7,7 @@ use std::{ }; use color_eyre::eyre::Result; -use crossbeam_channel::Sender; +use crossbeam_channel::{bounded, Sender}; use crate::{ status_emitter::{RevisionStyle, TestStatus}, @@ -47,6 +47,11 @@ impl BuildManager { } } + /// Create a new `BuildManager` that cannot create new sub-jobs. + pub fn one_off(config: Config) -> Self { + Self::new(config, bounded(0).0) + } + /// Lazily add more jobs after a test has finished. These are added to the queue /// as normally, but nested below the test. pub fn add_new_job(&self, job: impl Send + 'static + FnOnce() -> TestRun) { diff --git a/tests/integrations/basic-bin/Cargo.lock b/tests/integrations/basic-bin/Cargo.lock index 51ae75e7..c5450714 100644 --- a/tests/integrations/basic-bin/Cargo.lock +++ b/tests/integrations/basic-bin/Cargo.lock @@ -716,7 +716,7 @@ dependencies = [ [[package]] name = "ui_test" -version = "0.26.4" +version = "0.26.5" dependencies = [ "annotate-snippets", "anyhow", diff --git a/tests/integrations/basic-fail-mode/Cargo.lock b/tests/integrations/basic-fail-mode/Cargo.lock index 04ef877b..f4b9d007 100644 --- a/tests/integrations/basic-fail-mode/Cargo.lock +++ b/tests/integrations/basic-fail-mode/Cargo.lock @@ -716,7 +716,7 @@ dependencies = [ [[package]] name = "ui_test" -version = "0.26.4" +version = "0.26.5" dependencies = [ "annotate-snippets", "anyhow", diff --git a/tests/integrations/basic-fail/Cargo.lock b/tests/integrations/basic-fail/Cargo.lock index 189d9210..6783110d 100644 --- a/tests/integrations/basic-fail/Cargo.lock +++ b/tests/integrations/basic-fail/Cargo.lock @@ -716,7 +716,7 @@ dependencies = [ [[package]] name = "ui_test" -version = "0.26.4" +version = "0.26.5" dependencies = [ "annotate-snippets", "anyhow", diff --git a/tests/integrations/basic/Cargo.lock b/tests/integrations/basic/Cargo.lock index 89d3a5de..df5689af 100644 --- a/tests/integrations/basic/Cargo.lock +++ b/tests/integrations/basic/Cargo.lock @@ -639,7 +639,7 @@ dependencies = [ [[package]] name = "ui_test" -version = "0.26.4" +version = "0.26.5" dependencies = [ "annotate-snippets", "anyhow", diff --git a/tests/integrations/cargo-run/Cargo.lock b/tests/integrations/cargo-run/Cargo.lock index 51ae75e7..c5450714 100644 --- a/tests/integrations/cargo-run/Cargo.lock +++ b/tests/integrations/cargo-run/Cargo.lock @@ -716,7 +716,7 @@ dependencies = [ [[package]] name = "ui_test" -version = "0.26.4" +version = "0.26.5" dependencies = [ "annotate-snippets", "anyhow", diff --git a/tests/integrations/dep-fail/Cargo.lock b/tests/integrations/dep-fail/Cargo.lock index ba44c065..4d65ed83 100644 --- a/tests/integrations/dep-fail/Cargo.lock +++ b/tests/integrations/dep-fail/Cargo.lock @@ -565,7 +565,7 @@ dependencies = [ [[package]] name = "ui_test" -version = "0.26.4" +version = "0.26.5" dependencies = [ "annotate-snippets", "anyhow", diff --git a/tests/integrations/ui_test_dep_bug/Cargo.lock b/tests/integrations/ui_test_dep_bug/Cargo.lock index 2c42a3f0..51a379ee 100644 --- a/tests/integrations/ui_test_dep_bug/Cargo.lock +++ b/tests/integrations/ui_test_dep_bug/Cargo.lock @@ -570,7 +570,7 @@ dependencies = [ [[package]] name = "ui_test" -version = "0.26.4" +version = "0.26.5" dependencies = [ "annotate-snippets", "anyhow",