diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9e06feeb8..fff89f980 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,6 +49,16 @@ jobs: if: github.event_name == 'pull_request' run: cargo xtask check test-settings --base 'HEAD^1' --head 'HEAD' -v + test-targets: + name: Check test targets + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v6 + + - name: Check centralized integration tests + run: cargo xtask check test-targets -v + typos: name: Check typos runs-on: ubuntu-latest @@ -266,7 +276,7 @@ jobs: success: name: Success if: ${{ always() }} - needs: [formatting, test-settings, typos, checks, fuzz, web, ffi, feature-matrix-setup, feature-matrix] + needs: [formatting, test-settings, test-targets, typos, checks, fuzz, web, ffi, feature-matrix-setup, feature-matrix] runs-on: ubuntu-latest steps: diff --git a/xtask/src/check.rs b/xtask/src/check.rs index 840fc01e7..e7c2dc5f8 100644 --- a/xtask/src/check.rs +++ b/xtask/src/check.rs @@ -236,6 +236,7 @@ pub fn install(sh: &Shell) -> anyhow::Result<()> { pub fn tests_compile(sh: &Shell) -> anyhow::Result<()> { let _s = Section::new("TESTS-COMPILE"); cmd!(sh, "{CARGO} test --workspace --locked --no-run").run()?; + cmd!(sh, "{CARGO} test -p xtask --bin xtask --locked --no-run").run()?; cmd!( sh, "{CARGO} test -p ironrdp-testsuite-extra --test integration_tests_extra --no-default-features --features native-tls --locked --no-run" @@ -248,6 +249,7 @@ pub fn tests_compile(sh: &Shell) -> anyhow::Result<()> { pub fn tests_run(sh: &Shell) -> anyhow::Result<()> { let _s = Section::new("TESTS-RUN"); cmd!(sh, "{CARGO} test --workspace --locked").run()?; + cmd!(sh, "{CARGO} test -p xtask --bin xtask --locked").run()?; cmd!( sh, "{CARGO} test -p ironrdp-testsuite-extra --test integration_tests_extra --no-default-features --features native-tls --locked" diff --git a/xtask/src/cli.rs b/xtask/src/cli.rs index aa7fc017b..88399fc02 100644 --- a/xtask/src/cli.rs +++ b/xtask/src/cli.rs @@ -16,6 +16,7 @@ TASKS: check dependencies Check dependency-graph invariants between crates check test-settings --base --head Prevent removal of protected Cargo test settings + check test-targets Allow only centralized Cargo integration-test targets check tests [--no-run] Compile tests and, unless specified otherwise, run them check typos Check for typos in the codebase check features Run every feature-matrix case sequentially @@ -90,6 +91,7 @@ pub enum Action { base: String, head: String, }, + CheckTestTargets, CheckTests { no_run: bool, }, @@ -150,6 +152,7 @@ pub fn parse_args() -> anyhow::Result { base: args.value_from_str("--base")?, head: args.value_from_str("--head")?, }, + Some("test-targets") => Action::CheckTestTargets, Some("tests") => Action::CheckTests { no_run: args.contains("--no-run"), }, diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 3b79bba2a..c1fc82f2a 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -15,6 +15,7 @@ mod fuzz; mod pr; mod prelude; mod section; +mod test_targets; mod wasm; mod web; @@ -64,6 +65,7 @@ fn main() -> anyhow::Result<()> { Action::CheckLocks => check::lock_files(&sh)?, Action::CheckDependencies => check::dependencies(&sh)?, Action::CheckTestSettings { base, head } => check::test_settings(&sh, &base, &head)?, + Action::CheckTestTargets => test_targets::check(&sh)?, Action::CheckTests { no_run } => { if no_run { check::tests_compile(&sh)?; @@ -92,6 +94,7 @@ fn main() -> anyhow::Result<()> { Action::Ci => { check::fmt(&sh)?; check::typos(&sh)?; + test_targets::check(&sh)?; check::tests_compile(&sh)?; check::tests_run(&sh)?; check::lints(&sh)?; diff --git a/xtask/src/test_targets.rs b/xtask/src/test_targets.rs new file mode 100644 index 000000000..fe3679c70 --- /dev/null +++ b/xtask/src/test_targets.rs @@ -0,0 +1,199 @@ +use tinyjson::JsonValue; + +use crate::prelude::*; + +const ALLOWED_TEST_TARGETS: &[(&str, &str)] = &[ + ("ironrdp-testsuite-core", "integration_tests_core"), + ("ironrdp-testsuite-extra", "integration_tests_extra"), +]; + +pub fn check(sh: &Shell) -> anyhow::Result<()> { + let _s = Section::new("TEST-TARGETS"); + let metadata = cmd!(sh, "{CARGO} metadata --format-version=1 --no-deps --locked") + .read() + .context("read Cargo metadata")?; + + validate(&metadata)?; + + println!("All good!"); + Ok(()) +} + +fn validate(metadata: &str) -> anyhow::Result<()> { + let metadata: JsonValue = metadata.parse().context("parse Cargo metadata")?; + let metadata = json_object(&metadata, "Cargo metadata")?; + let packages = json_array( + metadata + .get("packages") + .context("Cargo metadata is missing `packages`")?, + "`packages`", + )?; + let mut unauthorized = Vec::new(); + + for package in packages { + let package = json_object(package, "Cargo metadata package")?; + let package_name = json_string( + package + .get("name") + .context("Cargo metadata package is missing `name`")?, + "Cargo metadata package name", + )?; + let targets = json_array( + package + .get("targets") + .context("Cargo metadata package is missing `targets`")?, + "Cargo metadata package targets", + )?; + + for target in targets { + let target = json_object(target, "Cargo metadata target")?; + let kinds = json_array( + target.get("kind").context("Cargo metadata target is missing `kind`")?, + "Cargo metadata target kind", + )?; + let mut is_test = false; + for kind in kinds { + if json_string(kind, "Cargo metadata target kind")? == "test" { + is_test = true; + break; + } + } + + if !is_test { + continue; + } + + let target_name = json_string( + target.get("name").context("Cargo metadata target is missing `name`")?, + "Cargo metadata target name", + )?; + if ALLOWED_TEST_TARGETS.contains(&(package_name, target_name)) { + continue; + } + + let source_path = json_string( + target + .get("src_path") + .context("Cargo metadata target is missing `src_path`")?, + "Cargo metadata target source path", + )?; + unauthorized.push((package_name, target_name, source_path)); + } + } + + if !unauthorized.is_empty() { + let targets = unauthorized + .into_iter() + .map(|(package, target, source)| format!("- package: `{package}`, target: `{target}`, source: `{source}`")) + .collect::>() + .join("\n"); + anyhow::bail!("unauthorized Cargo test target(s):\n{targets}"); + } + + Ok(()) +} + +fn json_object<'a>( + value: &'a JsonValue, + name: &str, +) -> anyhow::Result<&'a std::collections::HashMap> { + match value { + JsonValue::Object(object) => Ok(object), + _ => anyhow::bail!("{name} must be an object"), + } +} + +fn json_array<'a>(value: &'a JsonValue, name: &str) -> anyhow::Result<&'a [JsonValue]> { + match value { + JsonValue::Array(array) => Ok(array), + _ => anyhow::bail!("{name} must be an array"), + } +} + +fn json_string<'a>(value: &'a JsonValue, name: &str) -> anyhow::Result<&'a str> { + match value { + JsonValue::String(string) => Ok(string), + _ => anyhow::bail!("{name} must be a string"), + } +} + +#[cfg(test)] +mod tests { + use super::validate; + + #[test] + fn accepts_centralized_test_targets() { + let metadata = r#"{ + "packages": [ + { + "name": "ironrdp-testsuite-core", + "targets": [ + { + "kind": ["test"], + "name": "integration_tests_core", + "src_path": "crates/ironrdp-testsuite-core/tests/main.rs" + } + ] + }, + { + "name": "ironrdp-testsuite-extra", + "targets": [ + { + "kind": ["test"], + "name": "integration_tests_extra", + "src_path": "crates/ironrdp-testsuite-extra/tests/main.rs" + } + ] + } + ] + }"#; + + validate(metadata).unwrap(); + } + + #[test] + fn rejects_unauthorized_explicit_test_target() { + let metadata = r#"{ + "packages": [ + { + "name": "ironrdp-example", + "targets": [ + { + "kind": ["test"], + "name": "explicit_test", + "src_path": "crates/ironrdp-example/tests/explicit.rs" + } + ] + } + ] + }"#; + + let error = validate(metadata).unwrap_err().to_string(); + assert!(error.contains("package: `ironrdp-example`")); + assert!(error.contains("target: `explicit_test`")); + assert!(error.contains("source: `crates/ironrdp-example/tests/explicit.rs`")); + } + + #[test] + fn rejects_unauthorized_auto_discovered_test_target() { + let metadata = r#"{ + "packages": [ + { + "name": "ironrdp-example", + "targets": [ + { + "kind": ["test"], + "name": "auto_discovered", + "src_path": "crates/ironrdp-example/tests/auto_discovered.rs" + } + ] + } + ] + }"#; + + let error = validate(metadata).unwrap_err().to_string(); + assert!(error.contains("package: `ironrdp-example`")); + assert!(error.contains("target: `auto_discovered`")); + assert!(error.contains("source: `crates/ironrdp-example/tests/auto_discovered.rs`")); + } +}