From 4b053e15ae229cff1dce09d048f753ed2a2b7c67 Mon Sep 17 00:00:00 2001 From: David Mann Date: Sun, 1 Nov 2026 14:25:51 +0100 Subject: [PATCH] Integration: keep the ATN contract tests, drop the extra workflow (#7561) The contract tests no longer compiled on main (AtnItem::dictionary is now Vec, so it is compared with b"Future event"), sat outside main's tests/it layout, and the new atn.yml added six legs (two macOS) to every PR against ci.yml's policy. Move the tests into tests/it so the existing CI runs them, drop atn.yml and docs/atn-testing.md, keep main's ci.yml and fuzz target, and note the committed fixtures in ATTRIBUTION.md. Co-authored-by: denstu473 <63150090+denstu473@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 --- .github/workflows/atn.yml | 166 ------------------ ATTRIBUTION.md | 7 +- crates/psd/fuzz/fuzz_targets/atn.rs | 12 +- crates/psd/tests/fixtures/atn/README.md | 4 +- crates/psd/tests/{ => it}/atn_contract.rs | 12 +- .../tests/{ => it}/common/atn_roundtrip.rs | 2 +- crates/psd/tests/it/main.rs | 1 + docs/atn-testing.md | 97 ---------- docs/development.md | 5 +- 9 files changed, 24 insertions(+), 282 deletions(-) delete mode 100644 .github/workflows/atn.yml rename crates/psd/tests/{ => it}/atn_contract.rs (96%) rename crates/psd/tests/{ => it}/common/atn_roundtrip.rs (96%) delete mode 100644 docs/atn-testing.md diff --git a/.github/workflows/atn.yml b/.github/workflows/atn.yml deleted file mode 100644 index 1895598e28..0000000000 --- a/.github/workflows/atn.yml +++ /dev/null @@ -1,166 +0,0 @@ -name: ATN - -on: - push: - branches: [main] - pull_request: - types: [opened, synchronize, reopened, ready_for_review] - merge_group: - schedule: - - cron: "17 4 * * *" - workflow_dispatch: - inputs: - fuzz_seconds: - description: "Seconds of ATN parser fuzzing" - type: choice - options: ["60", "300", "600"] - default: "60" - -permissions: - contents: read - -concurrency: - group: atn-${{ github.ref }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} - -env: - CARGO_TERM_COLOR: always - RUST_BACKTRACE: 1 - CARGO_PROFILE_DEV_DEBUG: "0" - CARGO_PROFILE_RELEASE_DEBUG: "0" - # Keep optimized arithmetic behavior without paying for release LTO in a parser test job. - CARGO_PROFILE_RELEASE_LTO: "false" - CARGO_PROFILE_RELEASE_CODEGEN_UNITS: "16" - -jobs: - parser: - name: ATN parser (${{ matrix.target }}) - runs-on: ${{ matrix.os }} - timeout-minutes: 20 - strategy: - fail-fast: false - # These small CPU-only jobs can share the macOS pool with release builds. - max-parallel: 3 - matrix: - include: - - { os: ubuntu-24.04, target: x86_64-unknown-linux-gnu, arch: X64 } - - { os: ubuntu-24.04-arm, target: aarch64-unknown-linux-gnu, arch: ARM64 } - - { os: windows-2025, target: x86_64-pc-windows-msvc, arch: X64 } - - { os: windows-11-arm, target: aarch64-pc-windows-msvc, arch: ARM64 } - - { os: macos-15-intel, target: x86_64-apple-darwin, arch: X64 } - - { os: macos-15, target: aarch64-apple-darwin, arch: ARM64 } - defaults: - run: - shell: bash - steps: - - uses: actions/checkout@v4 - with: - persist-credentials: false - - uses: dtolnay/rust-toolchain@stable - with: - targets: ${{ matrix.target }} - - uses: Swatinem/rust-cache@v2 - with: - key: atn-parser-${{ matrix.target }} - save-if: ${{ github.ref == 'refs/heads/main' }} - cache-on-failure: true - - name: Record native execution environment - env: - EXPECTED_ARCH: ${{ matrix.arch }} - TEST_TARGET: ${{ matrix.target }} - run: | - mkdir -p atn-results - # A successful cross-compilation must never stand in for native execution. - test "$RUNNER_ARCH" = "$EXPECTED_ARCH" - { - git rev-parse HEAD - printf 'runner=%s arch=%s target=%s\n' "$RUNNER_OS" "$RUNNER_ARCH" "$TEST_TARGET" - printf 'image=%s version=%s\n' "${ImageOS:-unknown}" "${ImageVersion:-unknown}" - rustc -vV - cargo -V - } | tee atn-results/environment.txt - - name: Parser unit tests - env: - TEST_TARGET: ${{ matrix.target }} - run: | - set -o pipefail - cargo test --locked -p photocraft-psd --target "$TEST_TARGET" --lib atn::tests:: -- --nocapture 2>&1 | tee atn-results/unit.log - - name: Parser contract tests (debug) - env: - TEST_TARGET: ${{ matrix.target }} - run: | - set -o pipefail - cargo test --locked -p photocraft-psd --target "$TEST_TARGET" --test atn_contract -- --nocapture 2>&1 | tee atn-results/debug.log - - name: Parser contract tests (release) - env: - TEST_TARGET: ${{ matrix.target }} - run: | - set -o pipefail - cargo test --locked --release -p photocraft-psd --target "$TEST_TARGET" --test atn_contract -- --nocapture 2>&1 | tee atn-results/release.log - - name: Upload parser evidence - if: always() - uses: actions/upload-artifact@v4 - with: - name: atn-parser-${{ matrix.target }} - path: atn-results/ - if-no-files-found: error - retention-days: 14 - - parser-gate: - name: ATN parser gate - needs: parser - if: always() - runs-on: ubuntu-24.04 - timeout-minutes: 5 - steps: - - name: Require every native parser leg - env: - PARSER_RESULT: ${{ needs.parser.result }} - run: test "$PARSER_RESULT" = success - - fuzz: - name: ATN parser fuzz - if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' - runs-on: ubuntu-24.04 - timeout-minutes: 20 - steps: - - uses: actions/checkout@v4 - with: - persist-credentials: false - - uses: dtolnay/rust-toolchain@nightly - - uses: Swatinem/rust-cache@v2 - with: - key: atn-fuzz - workspaces: crates/psd/fuzz - save-if: ${{ github.ref == 'refs/heads/main' }} - cache-on-failure: true - - name: Install cargo-fuzz - run: cargo install cargo-fuzz --locked - - name: Seed parser fuzzing with the contract fixtures - run: | - mkdir -p crates/psd/fuzz/corpus/atn atn-results - cp crates/psd/tests/fixtures/atn/*.atn crates/psd/fuzz/corpus/atn/ - { - git rev-parse HEAD - rustc -vV - cargo fuzz --version - sha256sum crates/psd/tests/fixtures/atn/*.atn - } | tee atn-results/fuzz-environment.txt - - name: Fuzz ATN parsing and writing - working-directory: crates/psd - env: - FUZZ_SECONDS: ${{ inputs.fuzz_seconds || '60' }} - run: | - set -o pipefail - cargo fuzz run atn -- -max_total_time="$FUZZ_SECONDS" -max_len=65536 -rss_limit_mb=512 -timeout=5 2>&1 | tee ../../atn-results/fuzz.log - - name: Upload fuzz evidence - if: always() - uses: actions/upload-artifact@v4 - with: - name: atn-fuzz-evidence - path: | - atn-results/ - crates/psd/fuzz/artifacts/atn/ - crates/psd/fuzz/corpus/atn/ - if-no-files-found: warn - retention-days: 14 diff --git a/ATTRIBUTION.md b/ATTRIBUTION.md index 9088da74b7..94f30b1be3 100644 --- a/ATTRIBUTION.md +++ b/ATTRIBUTION.md @@ -120,7 +120,8 @@ Artwork shown in the screenshots (all public domain, via Wikimedia Commons; deta ## Test data (not committed, not shipped) -`corpus/` is gitignored and no test fixtures are committed to the repository. +`corpus/` is gitignored and no test fixtures are committed to the repository, except the small +synthetic parser fixtures listed under *Synthetic parser test fixtures* below. `cargo xtask corpus --all` fetches every corpus at the pinned commits in `xtask/src/corpus_pins.rs` and verifies each file against the sha256 lists in `xtask/*.sha256`, with the upstream licence next to the files: @@ -138,14 +139,14 @@ next to the files: | `corpus/pngsuite/` | PngSuite | Willem van Schaik | | Public domain | | `corpus/pixls/` (11 files) | Real camera raws, one per decode path: Canon PowerShot SX50 HS (CR2 and its DNG conversion), Nikon D3 uncompressed NEF and Z6 paired 12/14-bit lossy and packed NEFs, Sony DSC-RX0 cRAW ARW, Panasonic DC-G9 high-res RW2, Olympus E-1 GRBG ORF, plus the E-5 packed ORF as the documented known-unsupported case. Per file (each CC0, per its database entry): [IMG_4059.CR2](https://raw.pixls.us/data/Canon/PowerShot%20SX50%20HS/IMG_4059.CR2), [CRW_4061.DNG](https://raw.pixls.us/data/Canon/PowerShot%20SX50%20HS/CRW_4061.DNG), [JD1_8203.NEF](https://raw.pixls.us/data/Nikon/D3/JD1_8203.NEF), [DSC00009.ARW](https://raw.pixls.us/data/Sony/DSC-RX0/DSC00009.ARW), [P1000475.RW2](https://raw.pixls.us/data/Panasonic/DC-G9/P1000475.RW2), [E_1__C106743_gredos.ORF](https://raw.pixls.us/data/Olympus/E-1/E_1__C106743_gredos.ORF), [_7061961_copy.ORF](https://raw.pixls.us/data/Olympus/E-5/_7061961_copy.ORF), [DSC_0750.NEF](https://raw.pixls.us/data/Nikon/Z%206/DSC_0750.NEF), [DSC_0751.NEF](https://raw.pixls.us/data/Nikon/Z%206/DSC_0751.NEF), [DSC_0753.NEF](https://raw.pixls.us/data/Nikon/Z%206/DSC_0753.NEF), [DSC_0754.NEF](https://raw.pixls.us/data/Nikon/Z%206/DSC_0754.NEF) | The photographers of the pixls.us raw samples database | [raw.pixls.us](https://raw.pixls.us) `data/`, fetched per [xtask/src/corpus_pins.rs](xtask/src/corpus_pins.rs) | CC0 / public domain, per file as linked | +Files copied into `corpus/` by hand (tiff, exr, raw) must be MIT, BSD or CC0. + ## Synthetic parser test fixtures | Path | Title | Author | Source | License | |---|---|---|---|---| | `crates/psd/tests/fixtures/atn/*.atn` | Independently encoded ATN version-16 parser fixtures | PhotoCraft contributors | Original synthetic test data; wire layout documented in `crates/psd/src/atn.rs`; provenance in the adjacent README | MIT OR Apache-2.0 | -Files copied into `corpus/` by hand (tiff, exr, raw) must be MIT, BSD or CC0. - ## Embedded-vector editor source The external SVG watcher, engine/UI handoff and bounded SVG rendering in this change diff --git a/crates/psd/fuzz/fuzz_targets/atn.rs b/crates/psd/fuzz/fuzz_targets/atn.rs index 7d719a0b80..3bb1777c8a 100644 --- a/crates/psd/fuzz/fuzz_targets/atn.rs +++ b/crates/psd/fuzz/fuzz_targets/atn.rs @@ -1,12 +1,14 @@ //! Fuzz the `.atn` actions parser: any input must return `Ok` or `Err`, never panic, and -//! anything that parses must survive writing, reparsing and stable canonical writing. +//! anything that parses must survive a write and parse back equal. #![no_main] use libfuzzer_sys::fuzz_target; - -#[path = "../../tests/common/atn_roundtrip.rs"] -mod atn_roundtrip; +use photocraft_psd::atn; fuzz_target!(|data: &[u8]| { - atn_roundtrip::check(data); + if let Ok(f) = atn::parse(data) { + let bytes = atn::write(&f).expect("a parsed set is writable"); + let again = atn::parse(&bytes).expect("a written set parses"); + assert_eq!(again.actions, f.actions); + } }); diff --git a/crates/psd/tests/fixtures/atn/README.md b/crates/psd/tests/fixtures/atn/README.md index e26382f1ea..119f098c3e 100644 --- a/crates/psd/tests/fixtures/atn/README.md +++ b/crates/psd/tests/fixtures/atn/README.md @@ -13,7 +13,7 @@ oracles. | `descriptors.atn` | 859 | Every descriptor value variant and every reference form currently represented by the parser | `6c76502822c1f36c8060a71adb227c4540663518785bbab1b70951dac0ef58ad` | | `float-bits.atn` | 428 | NaN payload, negative zero, infinities, and floats inside lists, objects and arrays | `0aa6278a569c777f249b305e889896132c2a87e44a24cb1a55d1d3fc0928fd0c` | -`tests/atn_contract.rs` checks the independently encoded bytes and expected public structures. -The ATN fuzz job copies the same files into its seed corpus. A fixture change should explain +`tests/it/atn_contract.rs` checks the independently encoded bytes and expected public structures. +A fixture change should explain the changed wire fields and update this table; do not regenerate expectations merely to make a failing parser pass. diff --git a/crates/psd/tests/atn_contract.rs b/crates/psd/tests/it/atn_contract.rs similarity index 96% rename from crates/psd/tests/atn_contract.rs rename to crates/psd/tests/it/atn_contract.rs index 55d7a6623d..522d4ad2d4 100644 --- a/crates/psd/tests/atn_contract.rs +++ b/crates/psd/tests/it/atn_contract.rs @@ -1,4 +1,4 @@ -//! Portable public-API contract, executed natively in debug and release by ATN CI. +//! Public-API contract for the `.atn` parser, against independently encoded wire fixtures. use photocraft_psd::PsdError; use photocraft_psd::atn::{self, AtnAction, AtnEvent, AtnFile, AtnItem}; use photocraft_psd::descriptor::{Class, Descriptor, Id, MAX_DEPTH, ObjectArray, ReferenceItem, UnicodeString, Value}; @@ -7,10 +7,10 @@ use photocraft_psd::descriptor::{Class, Descriptor, Id, MAX_DEPTH, ObjectArray, mod atn_roundtrip; const FIXTURES: &[(&str, &[u8])] = &[ - ("empty", include_bytes!("fixtures/atn/empty.atn")), - ("metadata", include_bytes!("fixtures/atn/metadata.atn")), - ("descriptors", include_bytes!("fixtures/atn/descriptors.atn")), - ("float bits", include_bytes!("fixtures/atn/float-bits.atn")), + ("empty", include_bytes!("../fixtures/atn/empty.atn")), + ("metadata", include_bytes!("../fixtures/atn/metadata.atn")), + ("descriptors", include_bytes!("../fixtures/atn/descriptors.atn")), + ("float bits", include_bytes!("../fixtures/atn/float-bits.atn")), ]; fn file_with(desc: Descriptor) -> AtnFile { @@ -55,7 +55,7 @@ fn unicode_metadata_unknown_events_and_dialog_flags_are_preserved() { let future = &action.items[1]; assert_eq!(future.event, AtnEvent::Name("syntheticFutureEvent".into())); assert_eq!((future.expanded, future.enabled, future.with_dialog, future.dialog_options), (false, false, true, 255)); - assert_eq!(future.dictionary, "Future event"); + assert_eq!(future.dictionary, b"Future event"); assert!(future.descriptor.is_none()); assert_eq!(file.actions[1].name, UnicodeString::new_nul("Empty")); assert!(file.actions[1].items.is_empty()); diff --git a/crates/psd/tests/common/atn_roundtrip.rs b/crates/psd/tests/it/common/atn_roundtrip.rs similarity index 96% rename from crates/psd/tests/common/atn_roundtrip.rs rename to crates/psd/tests/it/common/atn_roundtrip.rs index 8b71754b56..ded43b55e2 100644 --- a/crates/psd/tests/common/atn_roundtrip.rs +++ b/crates/psd/tests/it/common/atn_roundtrip.rs @@ -1,4 +1,4 @@ -//! Shared invariant for the parser contract suite and the actual cargo-fuzz target. +//! Shared round-trip invariant for the `.atn` parser contract suite. use photocraft_psd::atn::{self, AtnFile}; use photocraft_psd::descriptor::{Descriptor, Value}; diff --git a/crates/psd/tests/it/main.rs b/crates/psd/tests/it/main.rs index ee6879aa66..8d83a12c17 100644 --- a/crates/psd/tests/it/main.rs +++ b/crates/psd/tests/it/main.rs @@ -1,5 +1,6 @@ //! The crate's integration tests, linked into one test binary. +mod atn_contract; mod builder; mod corpus; mod corpus_atn; diff --git a/docs/atn-testing.md b/docs/atn-testing.md deleted file mode 100644 index e882f76bdc..0000000000 --- a/docs/atn-testing.md +++ /dev/null @@ -1,97 +0,0 @@ -# ATN parser tests and CI - -`.github/workflows/atn.yml` adds a focused parser check on every pull request, merge-group run, -main push, nightly run and manual dispatch. It runs even on unrelated pull requests so its -stable `ATN parser gate` check is always available for branch protection. Making that check -required is a repository setting; adding the workflow does not change branch protection. - -## Native platform matrix - -| OS image | Executed target | -|---|---| -| Ubuntu 24.04 x64 | `x86_64-unknown-linux-gnu` | -| Ubuntu 24.04 ARM64 | `aarch64-unknown-linux-gnu` | -| Windows Server 2025 x64 | `x86_64-pc-windows-msvc` | -| Windows 11 ARM64 | `aarch64-pc-windows-msvc` | -| macOS 15 Intel | `x86_64-apple-darwin` | -| macOS 15 Apple Silicon | `aarch64-apple-darwin` | - -Runner labels are listed in GitHub's -[hosted-runner reference](https://docs.github.com/en/actions/reference/runners/github-hosted-runners). - -Every leg verifies the runner architecture, runs the parser unit tests, and executes the public -parser contract suite in debug and release. These are native test executions, not cross-compile -checks. The release check disables LTO to keep CI small while retaining optimized arithmetic -behavior. At most three matrix legs run concurrently. Full workspace CI keeps its existing -platform selection; this separate job builds only the standalone PSD crate and its dependencies. - -The matrix tests CPU parsing and writing. It does not certify graphics drivers, GPU previews, -UI cancellation, ATN command conversion/playback, WebAssembly execution or Photoshop parity. -Those need their own execution and expected-result checks. Processor architecture coverage is -not an exhaustive inventory of CPU models or operating-system versions. - -## Contract and fixtures - -`crates/psd/tests/atn_contract.rs` exercises the public `photocraft_psd::atn::{parse, write}` APIs. -Its four committed synthetic fixtures are independently encoded, not generated by the writer -under test. The checks cover: - -- Byte-exact parsing/writing, explicit endianness, names, action/step order and empty actions. -- UTF-16 units, including supplementary characters and unpaired surrogates. -- Function keys, modifiers, panel state, enabled state, dialog flags and unknown event names. -- Every descriptor value variant and reference form, including opaque alias/path/raw bytes. -- Exact floating-point bits, including NaN payloads, signed zero and infinities. -- Every truncated fixture, invalid versions/tags, declared action/step limits and excessive - descriptor nesting, with passing valid controls. -- Deterministic generated metadata/float cases and seed mutations through the same invariant - called by the real fuzz target. - -The parser preserves file representation; native engine option semantics remain the -responsibility of `crates/engine/src/atn_cmds/tests.rs` and future playback regression suites. - -Run the focused checks from the repository root: - -```sh -cargo test --locked -p photocraft-psd --lib atn::tests:: -cargo test --locked -p photocraft-psd --test atn_contract -cargo test --locked --release -p photocraft-psd --test atn_contract -``` - -Fixtures and their provenance/checksums are documented in -`crates/psd/tests/fixtures/atn/README.md`. New Photoshop-authored interoperability oracles belong -in the separately pinned `photocraft-corpus` repository, following the existing corpus policy. - -## Fuzzing - -The dedicated ATN workflow runs the existing `psd/atn` cargo-fuzz target on Linux nightly and on -manual dispatch. It starts with the four valid fixtures, runs for 60 seconds nightly, and caps -input length at 64 KiB, resident memory at 512 MiB and a single input at five seconds. Manual -runs can select 60, 300 or 600 seconds. The general CI workflow's existing manual fuzz matrix -also includes `psd/atn`. - -The shared invariant in `tests/common/atn_roundtrip.rs` requires every successfully parsed set -to write and reparse with the same set/action metadata, descriptor structure and floating-point -bits. NaNs are compared by bits before normalization for AST equality, avoiding false failures -from `NaN != NaN`. Only trailing-input diagnostic warnings are excluded from serialized state. -Canonical output must remain byte-stable on another write. - -```sh -mkdir -p crates/psd/fuzz/corpus/atn -cp crates/psd/tests/fixtures/atn/*.atn crates/psd/fuzz/corpus/atn/ -cd crates/psd -cargo +nightly fuzz run atn -- -max_total_time=60 -max_len=65536 -rss_limit_mb=512 -timeout=5 -``` - -Run cargo-fuzz where its sanitizer/runtime are supported; the deterministic contract suite runs -on every matrix platform. Every real failure gets a minimized committed regression fixture and -an expected behavior assertion. Do not hide a failure by adding an ignore or weakening a check. - -## Evidence and scope - -Parser legs upload the exact commit, runner/image, architecture, Rust/Cargo versions, and test -logs, including failures. Fuzz runs upload environment details, fixture checksums, the evolved -corpus, logs and crash artifacts. Evidence is retained for 14 days. A failed, cancelled or -skipped parser matrix prevents the aggregate gate from succeeding. - -Local validation on one platform does not establish that hosted jobs have passed. Confirm all -six native legs in GitHub Actions before describing the change as verified on that matrix. diff --git a/docs/development.md b/docs/development.md index 12cf20da74..dee2b99638 100644 --- a/docs/development.md +++ b/docs/development.md @@ -631,8 +631,9 @@ PhotoCraft equivalent. Events outside the table load as `actions.photoshopStep` steps that keep the descriptor verbatim (so Save Actions writes them back) and fail at playback with the step's Photoshop name. -The focused native parser matrix, public-API contract fixtures, scheduled fuzzing and local -commands are described in [ATN parser tests and CI](atn-testing.md). +`crates/psd/tests/it/atn_contract.rs` checks the parser's public API against four small, +independently encoded fixtures in `crates/psd/tests/fixtures/atn/` (provenance and checksums in +its README): `cargo test -p photocraft-psd --test it atn_contract::`. ## Remove Tool quality