feat(ci): Implement robust rust CI. - #1
Closed
gavin-dunlap-luminar wants to merge 7 commits into
Closed
Conversation
gavin-dunlap-luminar
force-pushed
the
feat/implement-robust-ci
branch
from
June 12, 2026 14:38
46d91ad to
1908df7
Compare
This was referenced Sep 9, 2026
Contributor
|
Closing, #16 should supercede the work here post-nostd |
JustinKovacich
added a commit
that referenced
this pull request
Sep 10, 2026
Replaces this repo's seven hand-rolled jobs with a thin caller for `luminartech/rust_workflow/.github/workflows/rust-ci.yml@v1`, the same shape `uds_protocol` and `automotive_wire_codec` use. What the crate gains over the old `ci.yml`: pre-commit, a security audit (`cargo audit` + `cargo deny`), coverage, miri, and `cargo-semver-checks` -- the last being the gate that stops an accidental breaking release once the crate is published. Four inputs are deliberately off or overridden, each for a reason that will expire: - `publish-crate: false`. `release.yml` owns publishing, through trusted publishing rather than the registry token this workflow expects. The publish *dry run* stays on -- it needs no credentials and checks the crate still packages. - `run-semver-checks: false`. It diffs against the published baseline, and there is none until the first release. Flip it in the change that enables publishing. - `run-fuzz-tests: false`. `cargo fuzz build` needs a `fuzz/` directory. #1 adds four targets; this flips when that lands. - `run-property-tests: false` with `unit-test-filter: 'all()'`. There are no `prop_` tests, the filter would select nothing, and `cargo nextest` exits 4 on an empty selection -- so the job would fail rather than skip. And two overrides that are not temporary: `no-std-target: thumbv7em-none-eabihf`, the target this crate's bare-metal support is written against and `examples/bare_metal_codec` is built for, rather than the workflow's `thumbv6m` default; and `miri-args: '--lib'`, because `tests/golden_vectors.rs` reads its fixtures off disk and miri's isolation refuses `open`. The library tests are where the zero-copy decode paths worth checking for UB live, and 21 of them pass under miri. MSRV is left unset so the job reads `rust-version` from Cargo.toml and cannot drift from what the manifest promises. Scaffolding the shared workflow expects: `.pre-commit-config.yaml`, `.typos.toml`, and `deny.toml` copied from `uds_protocol`. Two hooks the siblings run are deliberately absent, with the reasons in the config -- mdformat would mangle this crate's markdown tables and reference-style links, and check-json has no strict JSON to check here. Every enabled gate was verified locally, including miri and pre-commit; `cargo deny` and `cargo audit` are not installed on this machine, so this PR's own run is their first check. This overlaps #1, which proposed its own 297-line `main.yml` before `rust_workflow` was tagged. The scaffolding that PR adds -- fuzz targets especially -- is still wanted; its workflow file is superseded by this caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JustinKovacich
added a commit
that referenced
this pull request
Sep 10, 2026
Replaces this repo's seven hand-rolled jobs with a thin caller for `luminartech/rust_workflow/.github/workflows/rust-ci.yml@v1`, the same shape `uds_protocol` and `automotive_wire_codec` use. What the crate gains over the old `ci.yml`: pre-commit, a security audit (`cargo audit` + `cargo deny`), coverage, miri, and `cargo-semver-checks` -- the last being the gate that stops an accidental breaking release once the crate is published. Four inputs are deliberately off or overridden, each for a reason that will expire: - `publish-crate: false`. `release.yml` owns publishing, through trusted publishing rather than the registry token this workflow expects. The publish *dry run* stays on -- it needs no credentials and checks the crate still packages. - `run-semver-checks: false`. It diffs against the published baseline, and there is none until the first release. Flip it in the change that enables publishing. - `run-fuzz-tests: false`. `cargo fuzz build` needs a `fuzz/` directory. #1 adds four targets; this flips when that lands. - `run-property-tests: false` with `unit-test-filter: 'all()'`. There are no `prop_` tests, the filter would select nothing, and `cargo nextest` exits 4 on an empty selection -- so the job would fail rather than skip. And two overrides that are not temporary: `no-std-target: thumbv7em-none-eabihf`, the target this crate's bare-metal support is written against and `examples/bare_metal_codec` is built for, rather than the workflow's `thumbv6m` default; and `miri-args: '--lib'`, because `tests/golden_vectors.rs` reads its fixtures off disk and miri's isolation refuses `open`. The library tests are where the zero-copy decode paths worth checking for UB live, and 21 of them pass under miri. MSRV is left unset so the job reads `rust-version` from Cargo.toml and cannot drift from what the manifest promises. Scaffolding the shared workflow expects: `.pre-commit-config.yaml`, `.typos.toml`, and `deny.toml` copied from `uds_protocol`. Two hooks the siblings run are deliberately absent, with the reasons in the config -- mdformat would mangle this crate's markdown tables and reference-style links, and check-json has no strict JSON to check here. Every enabled gate was verified locally, including miri and pre-commit; `cargo deny` and `cargo audit` are not installed on this machine, so this PR's own run is their first check. This overlaps #1, which proposed its own 297-line `main.yml` before `rust_workflow` was tagged. The scaffolding that PR adds -- fuzz targets especially -- is still wanted; its workflow file is superseded by this caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JustinKovacich
added a commit
that referenced
this pull request
Sep 10, 2026
simple_doip#1 carries 26 property tests written against the pre-`no_std` `write`/`read` API that 0.2.0 removed, on a branch whose merge base predates that migration, the error-taxonomy refactor and the 0.4.0 server break. The properties were sound; only the calls were stale. Rebasing that branch would have resolved textually and then failed to compile, so the cases are ported instead. 24 of the 26 land, all passing on the first run. The two that do not are byte roundtrips through serde, and this crate has no serde dependency to roundtrip through. They live in `tests/property.rs` rather than in `#[cfg(test)]` modules inside `src/` as the original did. proptest needs `std`, the library is `no_std`, and an integration test target gets `std` without any conditional-compilation gymnastics -- at the cost of only exercising the public API, which is all these properties touch anyway. What they check that `golden_vectors.rs` does not: the golden fixtures pin the exact bytes the crate emits, so they catch the wire format changing. These check that `encode` and `decode` agree with each other across the whole input space, which catches a field written in one order and read in another. Neither finds a misreading of the standard that both directions share symmetrically. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JustinKovacich
added a commit
that referenced
this pull request
Sep 10, 2026
…mation (#11) ## What The two things standing between `simple_doip` and a first crates.io release: the license texts, and a way to cut a release. ## Why The crate turned out to be much closer to publishable than the Phase A notes suggested. Checked against the live repo: - `Cargo.toml` on `main` already carries the **full publish metadata** — `description`, `license`, `repository`, `readme`, `keywords`, `categories`, `exclude`. No `publish = false`. - CI already runs **`cargo publish --dry-run`** in the `package` job, so publishability is verified on every run. - The only first-party dependency, **`automotive-wire-codec 0.3.0`, is already on crates.io** (published 2026-07-17). That was the hard prerequisite — `cargo publish` refuses a crate whose dependencies aren't on a registry. - The name **`simple_doip` is unclaimed** on crates.io (both spellings 404). - DFT's submodule pointer is **identical** to `main` (0 ahead / 0 behind, both at 0.5.2), so there's no fork to reconcile first. What was actually missing was the license files and the release plumbing. ## Commits 1. **`docs(license)`** — adds `LICENSE-MIT` + `LICENSE-APACHE`, copied verbatim from `uds_protocol` so the protocol libraries carry identical wording. The manifest has declared `MIT OR Apache-2.0` for a while with no text in the repo; GitHub's API reported `license: null` for exactly that reason. Both files land inside the published `.crate`. 2. **`build(release)`** — originally cargo-release plus a hand-rolled tag-driven `release.yml`. **Superseded:** see "Release tooling" below. Both files are deleted; `release-plz.toml` replaces them. ## Release tooling: release-plz, matching the sibling repos `uds_protocol` and `automotive_wire_codec` both release with release-plz, off a `release-plz.toml` that is **byte-identical between them**, and neither has a release workflow of its own — the jobs live in `luminartech/rust_workflow`. This repo was the odd one out. It now carries the same config verbatim, and `release.toml` + `.github/workflows/release.yml` are gone. The switch itself (`use-release-plz: true`, permissions, secrets) lands one layer up in **#13**, which owns `main.yml`. What this changes in practice: **no version is bumped by hand and no tag is pushed by hand.** A push to `main` maintains an open release PR; merging that PR publishes, tags, and cuts the GitHub release. ### The one real regression The deleted `release.yml` published through **crates.io trusted publishing** — OIDC, no stored registry credential anywhere. The reusable workflow's release-plz job takes a `CARGO_REGISTRY_TOKEN` secret instead. Matching the org is the point of this change, so that's the trade accepted here, but it is a step backwards on that one axis and the right place to fix it is `rust_workflow`, not this repo: one `id-token: write` plus a `crates-io-auth-action` step would give trusted publishing to every repo on the shared workflow at once. **Merging this still publishes nothing** — the release-plz jobs need secrets and a `crates-io` environment this repo does not have yet (listed in #13). ## `CONTRIBUTING.md` gains a Releases section Because the repo squash-merges with `squash_merge_commit_title: PR_TITLE`, the **PR title** is the commit subject on `main` — so PR titles, not branch commits, are what release-plz computes the version from. The new section spells that out with the type-to-bump table, and calls out the trap that bit this stack: pre-1.0 a breaking change is a *minor* bump, `!` is the only thing that produces one from a `fix:`, and `cargo-semver-checks` reads the API surface so it will not catch a behavioral break for you. ## How it was tested - `cargo publish --dry-run` — packages 77 files / 106.6 KiB compressed, verifies the packaged crate, resolves `automotive-wire-codec 0.3.0` from crates.io. **This is the proof that 0.5.2 is publishable as-is.** - `cargo package --list` — confirms `LICENSE-APACHE` and `LICENSE-MIT` are in the `.crate`. - `cargo fmt -- --check` and `cargo clippy --all-features -- -D warnings -Dclippy::pedantic` — both clean. - `release-plz.toml` parses, and its values `diff` clean against `uds_protocol`'s and `automotive_wire_codec`'s copies. - `cargo package --list` confirms `release-plz.toml` and `rust-toolchain.toml` stay out of the packaged crate (the `exclude` entry was updated with the rename). - The three cargo-release 0.25 behaviors this depends on were verified empirically during Phase A: the `publish` subcommand overrides `publish = false`; `--allow-branch '*'` is required on a tag-push (detached-HEAD) run; and `--version '^0.25'` is needed because cargo rejects a bare `--version 0.25`. ## Still needs a decision (not in this PR) 1. **crates.io owner + token.** `simple_doip` has no repo secrets at all. The precedent next door is a personal account — both `uds_protocol` and `automotive-wire-codec` are owned on crates.io by `zheylmun`. A crates.io **team owner** (`github:luminartech:<team>`) would be the durable answer. `CARGO_REGISTRY_TOKEN` then goes in this repo's secrets. (Worth noting: `uds_protocol` is at 0.1.0 on `main` but still 0.0.2 on crates.io, and has no `CARGO_REGISTRY_TOKEN` in its secrets — its merged release tooling has never actually published either.) 2. **First-publish version.** 0.5.2 as-is works. Worth being deliberate, because the version number is spent permanently once published. 3. **`luminartech/rust_workflow`.** The org-wide reusable workflow (`rust-ci.yml@v1`) already does tag-gated publish, a `publish-repository` fork guard, a `publish-environment` approval gate and `cargo semver-checks` — `uds_protocol` and `automotive_wire_codec` are both thin callers of it. Adopting it here would replace this repo's hand-rolled `ci.yml` *and* this `release.yml`, and it needs scaffolding this repo doesn't have yet (pre-commit config, `deny.toml`, `.typos.toml`, `.config/nextest.toml`, fuzz targets) — much of which is what #1 adds. That migration is worth doing, but it's a bigger change than unblocking a first publish, so this PR deliberately doesn't touch `ci.yml`. 4. **#1 and #2.** #2 (mine) is superseded by this PR — `main` grew the publish metadata it was adding. #1 (@gavin-dunlap-luminar) is a different question: its hand-rolled 297-line `main.yml` is superseded by `rust_workflow`, but its scaffolding is a prerequisite for adopting it. Both are currently `CONFLICTING` against `main`. ## Pre-publication audit (commits 3-10) A pass over the whole repo for things that would ship visibly wrong. Commits 3-5 needed no decisions: 3. **`build(cargo)`** — **docs.rs would have published a nearly empty API reference.** `default = []`, and docs.rs builds default features only, so `client`, `server`, `codec`, `alloc` and `std` — most of what the README points a reader at — would have been absent from the docs page. Fixed with `[package.metadata.docs.rs] all-features = true`; verified that the `client`, `server` and `message_codec` modules now render. The same commit stops shipping `release.toml` and `rust-toolchain.toml` inside the `.crate` (77 files → 75). 4. **`chore(vscode)`** — all six debug configurations passed `--package=doip`, the crate's pre-rename name, so every one of them failed. 5. **`chore(deps)`** — `futures-util 0.3.28` is yanked on crates.io, so every `cargo publish` run warned about it; moved to 0.3.34. Lockfile only. Commits 6-8 then closed the documentation gaps the audit turned up, and 9-10 settled how the crate actually gets published: 6. **`docs`** — adds `CHANGELOG.md` (110 lines), reconstructed from the release history, so the crate does not arrive on crates.io with the 0.2.0 zero-copy core, the 0.4.0 server API break and the 0.5.x client fixes behind it and no record of any of them. 7. **`docs`** — adds `SECURITY.md` (a stated way to report a vulnerability in an automotive diagnostics library) and `CONTRIBUTING.md` (a stated position on outside contributions, plus the Releases section described above). 8. **`docs(readme)`** — replaces the 45-line "Status" known-gaps inventory with a "Scope and limitations" section. The README is the crate's front page on crates.io; it is now 144 lines. 9. **`ci(release)`** — moved the hand-rolled `release.yml` onto crates.io trusted publishing. **Superseded** by commit 10, which deletes that file. 10. **`build(release)`** — hands versioning and publishing to release-plz. See "Release tooling" above, including the trusted-publishing regression this trade accepts. Also updated the repo's GitHub **description** (was "Crate for Rust DoIP") to match the manifest, and added **topics** (`doip`, `iso13400`, `automotive`, `diagnostics`, `no-std`, `rust`). ### Clean Worth recording, since the audit went looking: no Luminar branding anywhere except the org name in the repository URL; no internal hostnames, IP addresses, ticket numbers or names in any file; no `dbg!`/`println!`/`#[allow(...)]` in `src/`; `#![warn(missing_docs, missing_debug_implementations)]` is on; all 16 symbols the README names exist; examples are clean. ### Still open, needs a decision Four bullets that stood here are now closed: `CHANGELOG.md`, `SECURITY.md` and `CONTRIBUTING.md` are added by commits 6-7, the README's known-gaps inventory is replaced by commit 8, and **`v0.5.2` is now tagged and pushed at `304d014`**, so the tag history no longer has a hole in front of a first publish. What is left: - **Six naked TODOs in `src/`** (`connection.rs`, `routing_activation_request.rs`, and four in `server.rs`), notably `server.rs:631` (`LogicalAddress(0x0000), // TODO fix this constant`) — the same defect the README documents as `ClientConnectionInfo::logical_address` always being `0x0000`. Fixed one layer up in #12; the rest are fix-or-drop. - **`strum` is a major behind** (0.27 vs 0.28.0); bumping likely forces a matching bump in dft's workspace. - **CI has no cargo-audit, cargo-deny, cargo-semver-checks or typos check.** semver-checks is the one that matters once published — it's what stops an accidental breaking release. All four come free with `rust_workflow`, which #13 adopts. One audit finding was **withdrawn**: `ARCHITECTURE.md` §7.1 is marked "RESOLVED in 0.4.0" while sitting under "Known issues and deferred work", which looked stale — but §7's own preamble states the policy deliberately ("Resolved entries are kept because the analysis that led to the fix is still the fastest way to understand the shape the API ended up with"). Left alone. Renumbering would also have broken the `§7.2` references in `src/client_inner.rs` and `tests/integration_test.rs`. ## Review status Marked ready for review; not reviewed by anyone yet. CI is green — 7 pass. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
JustinKovacich
added a commit
that referenced
this pull request
Sep 10, 2026
**Stacked on #12 — merge order: #11 → #12 → this.** `--base` is `fix/server-tracks-tester-logical-address`, so the diff here is only this PR's two commits. ## What Replaces this repo's seven hand-rolled CI jobs with a thin caller for `luminartech/rust_workflow/.github/workflows/rust-ci.yml@v1` — the same shape `uds_protocol` and `automotive_wire_codec` use. What the crate gains over the old `ci.yml`: **pre-commit**, a **security audit** (`cargo audit` + `cargo deny`), **coverage**, **miri**, and **`cargo-semver-checks`** — the last being the gate that stops an accidental breaking release once the crate is public. ## Publishing stays out of it `release.yml` (from #11) owns publishing, through **trusted publishing** rather than the registry token this workflow expects, so `publish-crate: false`. The publish *dry run* stays on — it needs no credentials and checks the crate still packages. ## The four inputs that are off, and when each expires | Input | Why | Flips when | |---|---|---| | `run-semver-checks: false` | Diffs against the published baseline; there is none yet | The change that enables publishing | | `run-fuzz-tests: false` | `cargo fuzz build` needs a `fuzz/` directory | #1 lands (it adds four targets) | | `run-property-tests: false` | No `prop_` tests, so the filter selects nothing — and `cargo nextest` **exits 4 on an empty selection**, so the job would fail rather than skip | A property test exists | | `publish-crate: false` | See above | Not intended to flip | Two overrides that are **not** temporary: - **`no-std-target: thumbv7em-none-eabihf`** rather than the workflow's `thumbv6m` default — it's the target this crate's bare-metal support is written against and `examples/bare_metal_codec` is built for. (`thumbv6m` does build, for what it's worth; I checked.) - **`miri-args: '--lib'`** — `tests/golden_vectors.rs` reads its `.hex` fixtures off disk, and miri's isolation refuses `open`. The library tests are where the zero-copy decode paths worth checking for UB live. MSRV is left unset so the job reads `rust-version` from `Cargo.toml` and cannot drift from what the manifest promises. ## Commit 1 is the prerequisite: 12 lints and 2 typos The shared workflow lints with `--all-targets` **and** with `--no-default-features`; this repo's CI ran clippy over the library with all features on. Twelve findings sat in that gap, all in test code or behind a feature combination nothing ever linted — six `assert!(a == b)` → `assert_eq!`, two truncating `as` casts in test fixtures, a missing `#[must_use]`, an unchecked `Duration` subtraction, two doc comments needing backticks on `DoIP`. `typos` found **two real ones**, and one matters: **`diagnotics` was in a user-visible warning string** in `logical_address.rs`, so it would have shipped to anyone using the crate. `recieve` was in a doc comment. It also caught something my earlier audit missed: a test doc comment named an **internal MicroVision application** as its example of a tester holding the `TCP_DATA` slot. This repository is public, so it now says "a diagnostic tool already polling the same ECU" — which is what the test actually exercises. (My branding sweep grepped for the company and sensor names but not the application's, which is how it slipped through.) ## How it was tested Every gate the caller enables, run locally: | | | |---|---| | `cargo fmt --all -- --check` | ok | | clippy `--all-targets --all-features` (pedantic) | ok | | clippy `--no-default-features` (pedantic) | ok | | clippy `--no-default-features --features alloc` | ok | | `cargo build --release --all-features` | ok | | no-std build, `thumbv7em-none-eabihf` (± `alloc`) | ok | | `cargo doc` with `RUSTDOCFLAGS=-D warnings` + doctests | ok | | `cargo +1.88 build --all-features` (MSRV) | ok | | `cargo test --all-features` | ok | | `cargo publish --dry-run` | ok | | **miri**, exactly as the job runs it (`--lib`) | ok — 21 tests, no UB | | `pre-commit run --all-files` | ok | | `typos` | ok | `cargo deny` and `cargo audit` aren't installed on this machine, so **this PR's own run is their first check** — the `deny.toml` is `uds_protocol`'s, and its allow-list may need widening for a dependency it doesn't share. ## Scaffolding `.pre-commit-config.yaml`, `.typos.toml` and `deny.toml`, copied from `uds_protocol`. Two hooks the siblings run are deliberately absent, with the reasons in the config: **mdformat** would mangle this crate's markdown tables and reference-style links, and **check-json** has no strict JSON to check here (the only JSON is `.vscode/launch.json`, which is JSONC — and excluding it instead makes `check-hooks-apply` fail a hook that matches nothing). ## First run failed three jobs — and finding out was the point The three that failed are exactly the three that could not be checked locally, and two shared one root cause I had missed. **`rust-toolchain.toml` pinned `channel = "stable"`, and a directory-local toolchain file overrides whatever toolchain CI installs** — `rustup default` does not beat it. So every job whose purpose is to run a *different* toolchain was either failing or lying: - **Miri** failed: the job installs nightly with the `miri` component, then the bare `cargo miri test` resolved to stable, which has no miri. - **The MSRV check never checked the MSRV.** It installs 1.88 and runs `cargo build`, which the file redirected to stable. That is not a regression from this PR — the previous hand-rolled `ci.yml` used the same pattern, so **the MSRV gate has been decorative for as long as the file existed.** 1.88 does hold; `cargo +1.88` verifies it explicitly. - **Pre-commit** failed on the file's `components = ["clippy", "rustfmt"]`: rustup tried to add clippy to a runner whose stable toolchain already ships `bin/cargo-clippy` and refused with a file conflict. Neither `uds_protocol` nor `automotive_wire_codec` carries a toolchain file, so removing it also stops this repo being the odd one out. `CONTRIBUTING.md` now says the absence is deliberate and why. **Worth being explicit about my own verification miss:** every local check passed because `cargo +nightly miri` and `cargo +1.88 build` name a toolchain explicitly, which *does* beat the file. The workflow runs bare `cargo`. I was testing a different thing than CI was. **`Security Audit` was a genuine finding, not a config problem.** `cargo audit` reported three vulnerabilities in the committed lockfile, all from versions years behind what the manifest already permits: | Crate | Locked | Advisory | Patched | |---|---|---|---| | `bytes` | 1.4.0 | RUSTSEC-2026-0007 | >= 1.11.1 | | `mio` | 0.8.8 | RUSTSEC-2024-0019 | >= 0.8.11 | | `tracing-subscriber` | 0.3.19 | RUSTSEC-2025-0055 | >= 0.3.20 | `cargo update` takes them to 1.12.1, 1.2.3 and 0.3.23 and moves 53 other packages — `tokio 1.30.0` → 1.53.1, `anyhow 1.0.75` → 1.0.104 among them, clearing three `unsound` advisories that were only warnings. The changelog gets a `Security` entry, since a consumer deciding whether to upgrade should see it. The real risk in a 56-package update is the MSRV, which is now actually enforced. `cargo +1.88` builds the refreshed graph with `--all-features` and with `--no-default-features`. Full battery re-run green after the fixes, including miri through `rustup run nightly` (what the job now does), `pre-commit run --all-files`, and `cargo publish --dry-run`. **One commit is mislabeled.** `chore(deps)` also carries the `rust-toolchain.toml` deletion — the `git rm` was already staged when I wrote its message, which is why that message points at a "next commit" that does not contain it. Left as-is rather than rewritten; the reason it was removed is in the following commit and in `CONTRIBUTING.md`. ## Relationship to #1 This overlaps @gavin-dunlap-luminar's #1, which proposed its own 297-line `main.yml` — written before `rust_workflow` was tagged `v1`. The scaffolding that PR adds is still wanted, **the fuzz targets especially**, since they're what unblocks `run-fuzz-tests` here. Its workflow file is superseded by this caller. Worth a conversation rather than a close. ## Review status Not reviewed by anyone yet. Draft. --- ## Release automation (added after the original review pass) This PR now also flips `use-release-plz: true`, so the reusable workflow's `Release-plz PR` and `Release-plz Release` jobs take over versioning, the changelog, tags, GitHub releases and the crates.io publish. That input also disables the workflow's tag-gated `Release & Publish` job, so the two paths cannot both fire. #11 carries the matching `release-plz.toml` and deletes the old cargo-release tooling. Wiring copied from `automotive_wire_codec`: - `contents: write` + `pull-requests: write` on the called workflow — the test jobs downscope themselves back to `contents: read`; this grant exists only so release-plz can push the release-PR branch and the release tag. - `cargo-registry-token`, `release-plz-app-id`, `release-plz-app-private-key` passed through as secrets. - `publish-repository: luminartech/simple_doip`, so a fork cannot publish under this crate's name. ### Repo configuration this needs, none of it in this diff | | | |---|---| | `CARGO_REGISTRY_TOKEN` | secret; does not exist yet | | `RELEASE_PLZ_APP_ID` / `RELEASE_PLZ_APP_PRIVATE_KEY` | secrets; do not exist yet | | `crates-io` environment | **this repo has no environments at all**; both siblings have this one, and the reusable workflow gates each release job on it | Until those exist the release-plz jobs run and no-op rather than publishing, which is why merging this is safe. `run-semver-checks` stays off — there is still no published baseline to diff against. The comment now also records what it will *not* do once enabled: it reads the public API surface, so a changed signature is caught and a changed behavior is not. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
JustinKovacich
added a commit
that referenced
this pull request
Sep 10, 2026
**Stacked on #13 — merge order: #11 → #12 → #13 → this.** ## Issue URL Closes #1 (supersedes it — see below). Files #15. ## What Brings simple_doip#1's test suites forward instead of rebasing that branch, and turns on the two CI gates that were off for want of them. #1's merge base is 2026-04-03, before the `no_std` migration, the error-taxonomy refactor and the 0.4.0 server break. Its 26 property tests and 3 of its 4 fuzz targets drive `Message::read`, `Message::write` and `Payload::read` — **none of which exist on `main`**, all removed in 0.2.0. A rebase resolves textually (12 conflicts on the first of 7 commits) and then fails to compile. The properties were sound; only the calls were stale. | | | |---|---| | **`tests/property.rs`** | 24 properties ported to `Encode`/`Decode`, all passing on the first run | | **`fuzz/`** | 4 cargo-fuzz targets, 3 rewritten and 1 unchanged | | **`.github/` templates** | PR + bug/epic/task, verbatim from #1 — no API coupling, no porting needed | | **`main.yml`** | `run-property-tests` and `run-fuzz-tests` both on | **24 of 26 properties land.** The two that don't are byte round trips through serde, and this crate has no serde dependency. They live in `tests/property.rs` rather than `#[cfg(test)]` modules inside `src/` as the original did: proptest needs `std`, the library is `no_std`, and an integration target gets `std` with no conditional-compilation gymnastics. The cost is only reaching the public API, which is all these properties touch. ## What they add over `golden_vectors.rs` The golden fixtures pin the exact bytes the crate emits, so they catch the wire format changing. These check `encode` and `decode` agree with **each other** across the whole input space, which catches a field written in one order and read in another. Neither finds a misreading of the standard that both directions share symmetrically — that's what the fixtures are for. ## `fuzz_roundtrip` found a real bug in under a second **#15**: `Message::encode` can emit a frame that `Message::decode` rejects. `decode` takes exactly `header.payload_length` bytes and lets `Payload::decode` consume fewer without complaint; the decoded `Message` keeps the declared length; `encode` then writes that stale length beside a payload of its real size. A NACK frame declaring 5 body bytes and carrying 1 decodes fine, re-encodes to 9 bytes with the header still claiming 5, and fails to re-decode with `Incomplete { needed: 5, available: 1 }`. The target **skips that specific shape**, with #15 referenced at the check, so it keeps hunting field-order asymmetries without asserting a property the crate violates today. The skip comes out with the fix, which is stacked on this PR. ## Testing | | | |---|---| | `cargo test --all-features` / `--no-default-features` | pass | | the 24 properties | pass | | `cargo nextest run -E 'test(~prop_)'` | **selects exactly 24**, not zero — the `exit 4` failure mode | | `cargo fuzz build` (real cargo-fuzz, nightly) | pass | | 8s per fuzz target | 5.4M / 6.1M / 1.6M / 4.1M execs, all clean | | `clippy --all-targets --all-features -Dclippy::pedantic` | clean | | `clippy --no-default-features -Dclippy::pedantic` | clean | | `cargo fmt --all --check`, `pre-commit run --all-files` | clean | | `cargo publish --dry-run` | pass — `fuzz/` does not enter the packaged crate | `unit-test-filter` stays `all()` rather than excluding `prop_`: the unit job measures coverage, and coverage should describe the whole suite. The property job re-runs the same 24 under their own name for a readable signal. ## Not included The **PR description lint** the sibling repos pair with the templates. It requires `## Issue URL` and `## Testing` sections and would fail every currently open PR, including this one's stack-mates. A `No Issue` label now exists as its escape hatch; the job belongs in a follow-up once bodies conform. ## On #1 Its `main.yml` predates `rust_workflow@v1` by six weeks, so it wasn't a misjudgment — the better option didn't exist. Its scaffolding was independently written in #13 before I'd read his closely enough, which is on me. What was uniquely valuable was the tests, and they're here. Recommend closing #1 with a pointer to this PR rather than leaving it to rot; @gavin-dunlap-luminar is credited in the commit and in `tests/property.rs`. ## Note on the commit This is one commit, not the four its message describes — everything was already staged when I wrote the first one, so it swept the lot. Since the repo squash-merges with the body taken from the PR description, the rationale that would have been in those messages is above instead. ## Review status Not reviewed by anyone yet. Draft. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.