From 663a1d437d35155a41c55f27a8dabf4589001a07 Mon Sep 17 00:00:00 2001 From: VPRamon Date: Sun, 4 Oct 2026 23:39:11 +0200 Subject: [PATCH 1/4] test: enforce service integration architecture Consolidate service-oriented test documentation and lifecycle/artifact assertions, add source hygiene enforcement, and make CI dependency-locked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 15 ++++--- docs/architecture/boundaries.md | 7 +-- docs/architecture/dependency-rules.md | 53 +++++++++++++---------- docs/architecture/dispatch.md | 26 +++++------- docs/architecture/lints-and-safety.md | 1 + docs/architecture/overview.md | 9 ++++ docs/architecture/testing.md | 43 +++++++++++++++++++ scripts/check_architecture.sh | 35 +++++++++++++++ src/service/job.rs | 39 +++++++++++++++++ src/service/rest.rs | 15 ++++++- tests/service_synthetic.rs | 61 ++++++++++++++++++++++++++- 11 files changed, 254 insertions(+), 50 deletions(-) create mode 100644 docs/architecture/testing.md create mode 100755 scripts/check_architecture.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 03f71db..f7282f4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,7 +34,7 @@ jobs: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 - - run: cargo check --workspace --all-targets --all-features + - run: cargo check --locked --workspace --all-targets --all-features clippy: name: Clippy @@ -45,7 +45,7 @@ jobs: with: components: clippy - uses: Swatinem/rust-cache@v2 - - run: cargo clippy --workspace --all-targets --all-features -- -D warnings + - run: cargo clippy --locked --workspace --all-targets --all-features -- -D warnings tests: name: Tests (${{ matrix.features }}) @@ -55,11 +55,11 @@ jobs: matrix: include: - features: default - command: cargo test --workspace --no-fail-fast + command: cargo test --locked --workspace --no-fail-fast - features: no-default-features - command: cargo test --workspace --no-default-features --no-fail-fast + command: cargo test --locked --workspace --no-default-features --no-fail-fast - features: all-features - command: cargo test --workspace --all-features --no-fail-fast + command: cargo test --locked --workspace --all-features --no-fail-fast steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable @@ -73,7 +73,7 @@ jobs: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 - - run: cargo test --doc --workspace --all-features + - run: cargo test --locked --doc --workspace --all-features docs: name: Documentation @@ -82,7 +82,7 @@ jobs: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 - - run: cargo doc --workspace --no-deps + - run: cargo doc --locked --workspace --no-deps env: RUSTDOCFLAGS: -D rustdoc::broken-intra-doc-links @@ -95,6 +95,7 @@ jobs: - run: bash scripts/check_dep_graph.sh - run: bash scripts/check_no_todos.sh - run: bash scripts/check_legacy_name_refs.sh + - run: bash scripts/check_architecture.sh msrv: name: MSRV (Rust 1.89) diff --git a/docs/architecture/boundaries.md b/docs/architecture/boundaries.md index 918bac4..de207c5 100644 --- a/docs/architecture/boundaries.md +++ b/docs/architecture/boundaries.md @@ -7,6 +7,7 @@ | `spod::service` | Configuration and validation, canonical job lifecycle/status/error contracts, workflow dispatch, run orchestration, input boundaries, artifact layout, and provenance finalisation using `siderust::pod`. | Reusable numerical algorithms, HTTP semantics, and durable scheduling/persistence. | | `spod` binaries | `clap`-based commands and the REST server; delegates to `spod::service`. | Any numerical logic. | -The full set of allowed and forbidden dependency edges is in -`dependency-rules.md` and is enforced by `scripts/check_dep_graph.sh` -in CI. +The current dependency and source-ownership rules are in +`dependency-rules.md`. Registry/lockfile provenance is enforced by +`scripts/check_dep_graph.sh`; local scientific implementation drift is +enforced by `scripts/check_architecture.sh` in CI. diff --git a/docs/architecture/dependency-rules.md b/docs/architecture/dependency-rules.md index e8a3c57..e9e3dd1 100644 --- a/docs/architecture/dependency-rules.md +++ b/docs/architecture/dependency-rules.md @@ -1,28 +1,35 @@ # Dependency rules +`spod` is a single service/application crate built on the canonical Siderust +0.12 APIs. The dependency direction is intentionally small: + ```text -ALLOWED: - qtty, tempoch, affn, cheby (independent foundations) - siderust -> qtty, tempoch, affn, cheby - pod-core -> siderust, qtty, tempoch, affn, cheby - pod-dynamics -> pod-core, siderust, cheby - pod-io -> siderust - pod-observations -> pod-dynamics, siderust - pod-estimation -> siderust, faer - pod-qc -> pod-products, siderust - pod-products -> pod-io, siderust - pod-service -> ALL pod-* crates, siderust - pod-cli -> pod-service (only) - -FORBIDDEN: - siderust -> pod-* - qtty/tempoch/affn/cheby -> siderust or pod-* - pod-core -> any other pod-* - pod-estimation -> pod-io, pod-observations - pod-observations -> pod-io - pod-dynamics -> pod-io, pod-observations, pod-estimation - pod-qc -> pod-estimation - pod-cli -> pod-{core,dynamics,io,observations,estimation,qc,products} +spod service/application + | + v + siderust::pod and siderust::formats ``` -The forbidden edges are checked by `scripts/check_dep_graph.sh`. +Runtime crates such as Axum, Tokio, Clap, and Serde support the application +boundary. They do not provide or replace POD scientific ownership. + +## Allowed exceptions + +The direct `sgp4` dependency and `src/sgp4/` implementation are temporary +product-parity exceptions. They provide full Vallado/AFSPC semantics that are +not yet available in Siderust 0.12. Their removal is gated by spod #29 and +Siderust #98, with the fixtures in `tests/sgp4_vallado.rs` preserved until +semantic parity is demonstrated. + +## Forbidden drift + +- Path or Git dependencies for canonical scientific crates. +- New local generic force, measurement, estimation, QC, format-parser, or + propagation implementations outside the documented SGP4 exception. +- CLI or REST code that assembles scientific pipelines instead of dispatching + through `spod::service`. + +`scripts/check_dep_graph.sh` enforces reproducible registry/lockfile +dependencies. `scripts/check_architecture.sh` enforces the source boundary +using narrowly scoped implementation type patterns rather than a filename +blacklist. Both checks run in the CI project-guards job. diff --git a/docs/architecture/dispatch.md b/docs/architecture/dispatch.md index 8567d0d..c0fcf63 100644 --- a/docs/architecture/dispatch.md +++ b/docs/architecture/dispatch.md @@ -1,17 +1,13 @@ -# Static vs dynamic dispatch +# Dispatch boundary -- **Static / generic dispatch** is used in inner loops: - - integrator step (`rk4_step` is generic over `F: ForceModel`), - - residual evaluation, - - per-step force evaluation through `CompositeForce`. +`spod::service` performs configuration validation, workflow selection, and +artifact/provenance orchestration. Scientific execution is delegated to the +canonical Siderust APIs; `spod` does not define local generic force, +measurement, estimation, or propagation kernels. -- **Dynamic dispatch** (`Box`, `Box`) - is used at *configuration assembly* time inside `spod::service`. - The boxed objects are converted into a concrete composite type before - entering the integration / estimation kernels, so the inner loops see a - monomorphised call. - -- **Rationale:** generic monomorphisation gives competitive performance - on the hot path; the dynamic seam at the top of the pipeline keeps - configuration ergonomic and avoids a combinatorial explosion of - generic instantiations exposed in the public API. +The source-hygiene guard in +[`scripts/check_architecture.sh`](../../scripts/check_architecture.sh) checks +for newly declared local scientific implementation traits and types while +allowing service code to compose upstream names. The only current local +scientific exception is `src/sgp4/`, retained for full Vallado/AFSPC parity +until Siderust provides equivalent semantics. diff --git a/docs/architecture/lints-and-safety.md b/docs/architecture/lints-and-safety.md index 03b03f9..c00f81f 100644 --- a/docs/architecture/lints-and-safety.md +++ b/docs/architecture/lints-and-safety.md @@ -82,6 +82,7 @@ The CI pipeline runs (in order): see [`../conventions/examples-and-doctests.md`](../conventions/examples-and-doctests.md)). 5. `scripts/snapshot_public_api.sh` — public-API diff (advisory pre-1.0). 6. `cargo deny check` — license / advisory / source policy. +7. `scripts/check_architecture.sh` — service-to-Siderust source boundary. A red light on any of these blocks merge. diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index 68e2d6c..733e655 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -68,3 +68,12 @@ The REST adapter is intentionally asynchronous and experimental. It does not provide durable persistence, scheduling, retries, or distributed workers. Those concerns are deferred to later service/API work, including the stable artifact API tracked by #28. + +## Testing boundary + +`spod` integration tests validate product and service behavior: configuration, +dispatch, lifecycle, interfaces, artifacts, and provenance. Reusable +scientific correctness belongs in Siderust. The full Vallado SGP4/SDP4 path +is the explicit temporary exception and remains covered by local parity tests +until spod #29 and Siderust #98 are resolved. See +[Testing architecture](testing.md) for the test pyramid and CI guardrails. diff --git a/docs/architecture/testing.md b/docs/architecture/testing.md new file mode 100644 index 0000000..46c315a --- /dev/null +++ b/docs/architecture/testing.md @@ -0,0 +1,43 @@ +# Testing architecture + +`spod` tests the product and service boundary. The primary integration path +is the synthetic workflow in +[`tests/service_synthetic.rs`](../../tests/service_synthetic.rs): it loads a +configuration, validates and dispatches it through `Runner`, writes the +service artifact set, and verifies manifest provenance, hashes, and sizes. +It intentionally checks application contracts rather than reproducing +scientific reference values owned by Siderust. + +## Test ownership + +| Layer | Owned by | Examples | +| --- | --- | --- | +| Service integration | `spod` | configuration validation, workflow dispatch, job lifecycle, artifacts, manifests, provenance | +| Interface integration | `spod` | compiled CLI tests, in-process REST router tests, process and Docker startup smoke tests | +| Scientific unit tests | Siderust | force-model mathematics, propagation numerics, estimation kernels, generic observations, standard formats, and generic QC | +| Product parity | `spod` temporarily | Full Vallado SGP4/SDP4 compatibility required by the product | + +The CLI and REST adapters use the same `JobStatus` and `JobError` model. +Detailed behavior belongs in fast in-process tests; shell and Docker checks +only cover process, bind-address, image, and health-boundary behavior. + +## Architecture guardrails + +`scripts/check_architecture.sh` rejects newly declared local generic +scientific implementation types outside the service boundary. It checks +implementation traits/types rather than filenames, so legitimate orchestration +and upstream Siderust type names remain usable. `scripts/check_dep_graph.sh` +also rejects path and Git dependencies so scientific APIs resolve from the +canonical locked dependency graph. + +The local `src/sgp4/` implementation is an explicit exception. Siderust 0.12 +does not yet provide equivalent full Vallado/AFSPC semantics, so +[`tests/sgp4_vallado.rs`](../../tests/sgp4_vallado.rs) remains the product +parity safety net for near-Earth, deep-space, and propagation-error cases. +The exception is tracked by [VPRamon/spod#29](https://github.com/VPRamon/spod/issues/29) +and [Siderust/siderust#98](https://github.com/Siderust/siderust/issues/98). + +When adding a service workflow, add coverage for its configuration contract, +dispatch and lifecycle behavior, generated artifacts, and provenance. Add +scientific correctness tests upstream instead of copying Siderust's unit +tests into this repository. diff --git a/scripts/check_architecture.sh b/scripts/check_architecture.sh new file mode 100755 index 0000000..9d9dd3f --- /dev/null +++ b/scripts/check_architecture.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash +# Enforce the service -> Siderust dependency direction. +# +# This is deliberately a source-pattern guard, not a filename blacklist: +# service orchestration may mention or compose upstream scientific types. +# New local generic scientific implementations must either move upstream or +# be added to a documented exception. The only current exception is src/sgp4. + +set -euo pipefail + +cd "$(dirname "$0")/.." + +exception_dir="src/sgp4" +if [[ ! -d "$exception_dir" ]]; then + echo "Architecture check FAILED: expected SGP4 exception directory '$exception_dir'." >&2 + exit 1 +fi + +if ! grep -RInE \ + --include='*.rs' \ + --exclude-dir=target \ + --exclude-dir="$exception_dir" \ + '^[[:space:]]*(pub[[:space:]]+)?(struct|trait)[[:space:]]+(ForceModel|MeasurementModel|Propagator|Integrator|KalmanFilter|GaussNewton|LeastSquares|OrbitComparator)([[:space:]]|<|{|$)' \ + src tests; then + echo "Architecture source-hygiene check passed." + exit 0 +fi + +cat >&2 <<'EOF' +Architecture source-hygiene check FAILED. +New local generic scientific implementations were found outside the +documented SGP4 exception in src/sgp4/. Reuse Siderust APIs or document a +temporary upstream-gap exception before adding local science. +EOF +exit 1 diff --git a/src/service/job.rs b/src/service/job.rs index 2e41206..9106ec3 100644 --- a/src/service/job.rs +++ b/src/service/job.rs @@ -289,6 +289,25 @@ mod tests { let mut status = JobStatus::pending("job-1".into(), Workflow::Synthetic); assert!(status.start().is_ok()); assert_eq!(status.state, JobState::Running); + let artifact = |path: &str, kind: &str| { + siderust::pod::run::dataset::DatasetRef::from_bytes(path, kind, b"x") + }; + let result = JobResult { + run_id: "job-1".into(), + workflow: Workflow::Synthetic, + artifacts: Artifacts { + orbit_sp3: artifact("orbit.sp3", "orbit-sp3"), + orbit_oem: artifact("orbit.oem", "orbit-oem"), + residuals_csv: artifact("residuals.csv", "residuals"), + qc_json: artifact("qc.json", "qc"), + manifest: artifact("manifest.json", "manifest"), + }, + }; + assert!(status.succeed(result).is_ok()); + assert_eq!(status.state, JobState::Succeeded); + assert!(status.result.is_some()); + assert!(status.error.is_none()); + assert_eq!(serde_json::to_value(status).unwrap()["state"], "succeeded"); } #[test] @@ -308,6 +327,26 @@ mod tests { assert!(status.start().is_err()); } + #[test] + fn failed_terminal_state_has_error_and_stable_state_name() { + let mut status = JobStatus::pending("job-2".into(), Workflow::Synthetic); + status.start().unwrap(); + status + .fail(JobError { + code: JobErrorCode::Unsupported, + message: "unsupported input".into(), + }) + .unwrap(); + + assert_eq!(status.state, JobState::Failed); + assert!(status.result.is_none()); + assert_eq!( + status.error.as_ref().unwrap().code, + JobErrorCode::Unsupported + ); + assert_eq!(serde_json::to_value(status).unwrap()["state"], "failed"); + } + #[test] fn service_errors_map_to_stable_codes() { let error = JobError::from(&ServiceError::unsupported("not available")); diff --git a/src/service/rest.rs b/src/service/rest.rs index c9acd93..56262ad 100644 --- a/src/service/rest.rs +++ b/src/service/rest.rs @@ -271,7 +271,20 @@ mod tests { let status = poll_status(&app, id).await; assert_eq!(status["state"], "succeeded"); - assert!(status["result"]["artifacts"]["manifest"].is_object()); + assert_eq!(status["workflow"], "synthetic"); + assert_eq!(status["result"]["run_id"], id); + for artifact in [ + "orbit_sp3", + "orbit_oem", + "residuals_csv", + "qc_json", + "manifest", + ] { + assert!( + status["result"]["artifacts"][artifact].is_object(), + "missing artifact metadata for {artifact}" + ); + } fs::remove_dir_all(root).unwrap(); } diff --git a/tests/service_synthetic.rs b/tests/service_synthetic.rs index d83a995..e4c7e12 100644 --- a/tests/service_synthetic.rs +++ b/tests/service_synthetic.rs @@ -29,6 +29,16 @@ fn base_config(output_dir: &Path) -> RunConfig { } } +fn test_root(name: &str) -> PathBuf { + let suffix = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let root = std::env::temp_dir().join(format!("spod-service-{name}-{suffix}")); + fs::create_dir_all(&root).unwrap(); + root +} + #[test] fn synthetic_service_produces_valid_manifest_and_products() { let suffix = SystemTime::now() @@ -98,7 +108,10 @@ fn synthetic_service_produces_valid_manifest_and_products() { .iter() .any(|v| v == field)); } - for output in &manifest.outputs { + let expected_kinds = ["orbit-oem", "orbit-sp3", "qc", "residuals"]; + assert_eq!(manifest.outputs.len(), expected_kinds.len()); + for (output, expected_kind) in manifest.outputs.iter().zip(expected_kinds) { + assert_eq!(output.kind, expected_kind); let path = if output.path.is_absolute() { output.path.clone() } else { @@ -160,6 +173,52 @@ fn invalid_configuration_fails_before_execution() { assert!(matches!(err, ServiceError::Configuration { .. })); } +#[test] +fn configuration_validation_rejects_empty_run_id_and_disabled_two_body() { + let output_dir = std::env::temp_dir().join("spod-invalid-semantic-config-test"); + + let mut empty_id = base_config(&output_dir); + empty_id.run_id = " ".into(); + assert!(matches!( + empty_id.validate().unwrap_err(), + ServiceError::Configuration { .. } + )); + + let mut no_two_body = base_config(&output_dir); + no_two_body.forces.two_body = false; + assert!(matches!( + no_two_body.validate().unwrap_err(), + ServiceError::Configuration { .. } + )); +} + +#[test] +fn configuration_rejects_unknown_yaml_fields() { + let root = test_root("unknown-field"); + let config_path = root.join("run.yaml"); + fs::write( + &config_path, + "schema_version: 1.0.0\nrun_id: unknown-field\ninputs: {}\noutput_dir: output\nforces: {}\nunexpected: true\n", + ) + .unwrap(); + + let error = RunConfig::from_yaml_file(&config_path).unwrap_err(); + assert!(matches!(error, ServiceError::Configuration { .. })); + fs::remove_dir_all(root).unwrap(); +} + +#[test] +fn relative_output_directory_is_resolved_from_configuration_file() { + let root = test_root("relative-output"); + let config_path = root.join("nested/run.yaml"); + let config = base_config(Path::new("products")); + assert_eq!( + config.output_dir_relative_to(&config_path), + root.join("nested/products") + ); + fs::remove_dir_all(root).unwrap(); +} + #[test] fn filesystem_failures_are_artifact_errors() { let suffix = SystemTime::now() From a32caf4bd15e4d49644ef4f04dfdf4e56bf853c8 Mon Sep 17 00:00:00 2001 From: VPRamon Date: Sun, 4 Oct 2026 23:44:33 +0200 Subject: [PATCH 2/4] fix: harden architecture guard tests Exclude only src/sgp4 explicitly, distinguish grep matches from checker errors, and make the unknown-field configuration test isolate deny_unknown_fields diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- scripts/check_architecture.sh | 34 ++++++++++++++++++++++++---------- tests/service_synthetic.rs | 5 ++++- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/scripts/check_architecture.sh b/scripts/check_architecture.sh index 9d9dd3f..f43bd71 100755 --- a/scripts/check_architecture.sh +++ b/scripts/check_architecture.sh @@ -16,20 +16,34 @@ if [[ ! -d "$exception_dir" ]]; then exit 1 fi -if ! grep -RInE \ - --include='*.rs' \ - --exclude-dir=target \ - --exclude-dir="$exception_dir" \ - '^[[:space:]]*(pub[[:space:]]+)?(struct|trait)[[:space:]]+(ForceModel|MeasurementModel|Propagator|Integrator|KalmanFilter|GaussNewton|LeastSquares|OrbitComparator)([[:space:]]|<|{|$)' \ - src tests; then - echo "Architecture source-hygiene check passed." - exit 0 +mapfile -t source_files < <( + find src tests -type f -name '*.rs' ! -path "$exception_dir/*" -print +) + +if grep -InE \ + '^[[:space:]]*(pub[[:space:]]+)?(struct|trait)[[:space:]]+(ForceModel|MeasurementModel|Propagator|Integrator|KalmanFilter|GaussNewton|LeastSquares|OrbitComparator)([[:space:]]|<|{|$)' \ + "${source_files[@]}"; then + grep_status=0 +else + grep_status=$? fi -cat >&2 <<'EOF' +case "$grep_status" in + 0) + cat >&2 <<'EOF' Architecture source-hygiene check FAILED. New local generic scientific implementations were found outside the documented SGP4 exception in src/sgp4/. Reuse Siderust APIs or document a temporary upstream-gap exception before adding local science. EOF -exit 1 + exit 1 + ;; + 1) + echo "Architecture source-hygiene check passed." + exit 0 + ;; + *) + echo "Architecture source-hygiene check FAILED: grep exited with status $grep_status." >&2 + exit "$grep_status" + ;; +esac diff --git a/tests/service_synthetic.rs b/tests/service_synthetic.rs index e4c7e12..014cfc9 100644 --- a/tests/service_synthetic.rs +++ b/tests/service_synthetic.rs @@ -198,12 +198,15 @@ fn configuration_rejects_unknown_yaml_fields() { let config_path = root.join("run.yaml"); fs::write( &config_path, - "schema_version: 1.0.0\nrun_id: unknown-field\ninputs: {}\noutput_dir: output\nforces: {}\nunexpected: true\n", + "schema_version: 1.0.0\nrun_id: unknown-field\nworkflow: synthetic\ninputs:\n sp3: null\n rinex_obs: null\n rinex_nav: null\n antex: null\noutput_dir: output\nforces:\n two_body: true\n j2: false\n third_body: false\nunexpected: true\n", ) .unwrap(); let error = RunConfig::from_yaml_file(&config_path).unwrap_err(); assert!(matches!(error, ServiceError::Configuration { .. })); + let diagnostic = error.to_string(); + assert!(diagnostic.contains("unknown field")); + assert!(diagnostic.contains("unexpected")); fs::remove_dir_all(root).unwrap(); } From 3239a220e778ba885df92d51179474fc8e62af40 Mon Sep 17 00:00:00 2001 From: VPRamon Date: Sun, 4 Oct 2026 23:48:39 +0200 Subject: [PATCH 3/4] test: add architecture guard regressions Cover restricted Rust visibility forms, the exact src/sgp4 exception boundary, and checker failures in temporary fixture trees. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 1 + scripts/check_architecture.sh | 3 +- scripts/check_architecture_test.sh | 63 ++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) create mode 100755 scripts/check_architecture_test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f7282f4..7523a44 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -96,6 +96,7 @@ jobs: - run: bash scripts/check_no_todos.sh - run: bash scripts/check_legacy_name_refs.sh - run: bash scripts/check_architecture.sh + - run: bash scripts/check_architecture_test.sh msrv: name: MSRV (Rust 1.89) diff --git a/scripts/check_architecture.sh b/scripts/check_architecture.sh index f43bd71..9e3a926 100755 --- a/scripts/check_architecture.sh +++ b/scripts/check_architecture.sh @@ -8,7 +8,8 @@ set -euo pipefail -cd "$(dirname "$0")/.." +repository_root="${SPOD_ARCH_ROOT:-$(cd "$(dirname "$0")/.." && pwd)}" +cd "$repository_root" exception_dir="src/sgp4" if [[ ! -d "$exception_dir" ]]; then diff --git a/scripts/check_architecture_test.sh b/scripts/check_architecture_test.sh new file mode 100755 index 0000000..a0e0c62 --- /dev/null +++ b/scripts/check_architecture_test.sh @@ -0,0 +1,63 @@ +#!/usr/bin/env bash +# Regression tests for check_architecture.sh using temporary source trees. + +set -euo pipefail + +script_dir="$(cd "$(dirname "$0")" && pwd)" +checker="$script_dir/check_architecture.sh" +root="$(mktemp -d)" +trap 'rm -rf "$root"' EXIT + +mkdir -p "$root/src/service" "$root/src/sgp4" "$root/tests/sgp4" "$root/scripts" +cp "$checker" "$root/scripts/check_architecture.sh" +chmod +x "$root/scripts/check_architecture.sh" + +expect_failure() { + local expected="$1" + if SPOD_ARCH_ROOT="$root" "$root/scripts/check_architecture.sh" \ + >"$root/stdout" 2>"$root/stderr"; then + echo "architecture regression expected failure: $expected" >&2 + exit 1 + fi + grep -q "$expected" "$root/stderr" +} + +cat >"$root/src/service/forbidden.rs" <<'EOF' +struct ForceModel; +pub struct MeasurementModel; +pub(crate) struct Integrator; +pub(super) trait Propagator {} +pub(self) struct KalmanFilter; +pub(in crate::foo) struct GaussNewton; +struct LeastSquares; +pub struct OrbitComparator; +EOF +expect_failure "New local generic scientific implementations" +rm "$root/src/service/forbidden.rs" + +cat >"$root/src/sgp4/exception.rs" <<'EOF' +pub(crate) struct ForceModel; +pub(super) trait Propagator {} +EOF +cat >"$root/tests/sgp4/other.rs" <<'EOF' +pub struct ForceModel; +EOF +expect_failure "New local generic scientific implementations" +rm "$root/tests/sgp4/other.rs" +SPOD_ARCH_ROOT="$root" "$root/scripts/check_architecture.sh" + +mock_bin="$root/mock-bin" +mkdir "$mock_bin" +cat >"$mock_bin/grep" <<'EOF' +#!/usr/bin/env bash +exit 2 +EOF +chmod +x "$mock_bin/grep" +if PATH="$mock_bin:$PATH" SPOD_ARCH_ROOT="$root" \ + "$root/scripts/check_architecture.sh" >"$root/stdout" 2>"$root/stderr"; then + echo "architecture regression expected checker error to fail" >&2 + exit 1 +fi +grep -q "grep exited with status 2" "$root/stderr" + +echo "Architecture guard regression tests passed." From 05f9b0a0cf166d2aa0e679418ab10a13a7145170 Mon Sep 17 00:00:00 2001 From: VPRamon Date: Sun, 4 Oct 2026 23:53:40 +0200 Subject: [PATCH 4/4] fix: harden architecture guard regressions --- scripts/check_architecture.sh | 7 +++- scripts/check_architecture_test.sh | 67 +++++++++++++++++++++--------- 2 files changed, 54 insertions(+), 20 deletions(-) diff --git a/scripts/check_architecture.sh b/scripts/check_architecture.sh index 9e3a926..06159c5 100755 --- a/scripts/check_architecture.sh +++ b/scripts/check_architecture.sh @@ -21,8 +21,13 @@ mapfile -t source_files < <( find src tests -type f -name '*.rs' ! -path "$exception_dir/*" -print ) +if ((${#source_files[@]} == 0)); then + echo "Architecture source-hygiene check passed." + exit 0 +fi + if grep -InE \ - '^[[:space:]]*(pub[[:space:]]+)?(struct|trait)[[:space:]]+(ForceModel|MeasurementModel|Propagator|Integrator|KalmanFilter|GaussNewton|LeastSquares|OrbitComparator)([[:space:]]|<|{|$)' \ + '^[[:space:]]*(pub(\([^)]*\))?[[:space:]]+)?(struct|trait)[[:space:]]+(ForceModel|MeasurementModel|Propagator|Integrator|KalmanFilter|GaussNewton|LeastSquares|OrbitComparator)([[:space:]]|<|{|$)' \ "${source_files[@]}"; then grep_status=0 else diff --git a/scripts/check_architecture_test.sh b/scripts/check_architecture_test.sh index a0e0c62..c7a7056 100755 --- a/scripts/check_architecture_test.sh +++ b/scripts/check_architecture_test.sh @@ -12,28 +12,33 @@ mkdir -p "$root/src/service" "$root/src/sgp4" "$root/tests/sgp4" "$root/scripts" cp "$checker" "$root/scripts/check_architecture.sh" chmod +x "$root/scripts/check_architecture.sh" -expect_failure() { - local expected="$1" +expect_forbidden_declaration() { + local description="$1" + local declaration="$2" + + printf '%s\n' "$declaration" >"$root/src/service/forbidden.rs" if SPOD_ARCH_ROOT="$root" "$root/scripts/check_architecture.sh" \ >"$root/stdout" 2>"$root/stderr"; then - echo "architecture regression expected failure: $expected" >&2 + echo "architecture regression expected failure: $description" >&2 exit 1 fi - grep -q "$expected" "$root/stderr" + grep -q "New local generic scientific implementations" "$root/stderr" + rm "$root/src/service/forbidden.rs" } -cat >"$root/src/service/forbidden.rs" <<'EOF' -struct ForceModel; -pub struct MeasurementModel; -pub(crate) struct Integrator; -pub(super) trait Propagator {} -pub(self) struct KalmanFilter; -pub(in crate::foo) struct GaussNewton; -struct LeastSquares; -pub struct OrbitComparator; -EOF -expect_failure "New local generic scientific implementations" -rm "$root/src/service/forbidden.rs" +for case in \ + "private|struct ForceModel;" \ + "public|pub struct MeasurementModel;" \ + "pub(crate)|pub(crate) struct Integrator;" \ + "pub(super)|pub(super) trait Propagator {}" \ + "pub(self)|pub(self) struct KalmanFilter;" \ + "pub(in path)|pub(in crate::foo) struct GaussNewton;" \ + "private second name|struct LeastSquares;" \ + "public second name|pub struct OrbitComparator;" +do + IFS='|' read -r description declaration <<<"$case" + expect_forbidden_declaration "$description" "$declaration" +done cat >"$root/src/sgp4/exception.rs" <<'EOF' pub(crate) struct ForceModel; @@ -42,17 +47,41 @@ EOF cat >"$root/tests/sgp4/other.rs" <<'EOF' pub struct ForceModel; EOF -expect_failure "New local generic scientific implementations" +if SPOD_ARCH_ROOT="$root" "$root/scripts/check_architecture.sh" \ + >"$root/stdout" 2>"$root/stderr"; then + echo "architecture regression expected tests/sgp4 declaration to fail" >&2 + exit 1 +fi +grep -q "New local generic scientific implementations" "$root/stderr" rm "$root/tests/sgp4/other.rs" -SPOD_ARCH_ROOT="$root" "$root/scripts/check_architecture.sh" +SPOD_ARCH_ROOT="$root" "$root/scripts/check_architecture.sh" \ + >"$root/stdout" 2>"$root/stderr" +grep -qx "Architecture source-hygiene check passed." "$root/stdout" +rm "$root/src/sgp4/exception.rs" mock_bin="$root/mock-bin" mkdir "$mock_bin" cat >"$mock_bin/grep" <<'EOF' #!/usr/bin/env bash -exit 2 +echo "grep must not run without inspectable Rust files" >&2 +exit 99 EOF chmod +x "$mock_bin/grep" +PATH="$mock_bin:$PATH" SPOD_ARCH_ROOT="$root" \ + "$root/scripts/check_architecture.sh" >"$root/stdout" 2>"$root/stderr" +grep -qx "Architecture source-hygiene check passed." "$root/stdout" + +cat >"$root/src/service/valid.rs" <<'EOF' +pub(crate) struct ServiceState; +EOF +SPOD_ARCH_ROOT="$root" "$root/scripts/check_architecture.sh" \ + >"$root/stdout" 2>"$root/stderr" +grep -qx "Architecture source-hygiene check passed." "$root/stdout" + +cat >"$mock_bin/grep" <<'EOF' +#!/usr/bin/env bash +exit 2 +EOF if PATH="$mock_bin:$PATH" SPOD_ARCH_ROOT="$root" \ "$root/scripts/check_architecture.sh" >"$root/stdout" 2>"$root/stderr"; then echo "architecture regression expected checker error to fail" >&2