Skip to content

fix(ci): repair the lint job, make the test job actually run tests#76

Merged
bashandbone merged 3 commits into
mainfrom
fix/ci-hk-pkl-and-test-job
Jul 26, 2026
Merged

fix(ci): repair the lint job, make the test job actually run tests#76
bashandbone merged 3 commits into
mainfrom
fix/ci-hk-pkl-and-test-job

Conversation

@bashandbone

Copy link
Copy Markdown
Owner

Why

CI has been red on main, and the jobs that were green weren't testing anything.

  • lint never ran a step. hk.pkl targets the hk v1.2.0 pkl schema while the workflow installs hk 1.48, which can't evaluate it (Eval error: field not found: staticcheck). Everything downstream — fmt, clippy, typos, cargo-deny — has been silently skipped.
  • Test Suite reported success on stable, beta, and nightly while running zero tests. It checked out the repo, installed Rust, warmed three caches, and stopped. There was no test step.
  • Security Audit was failing on live advisories.

This is the same class of problem #62 is about — a check that narrates success without performing it — one layer up, in the harness.

What changed

hk / lint

  • Retarget hk.pkl at the hk 1.48 pkl schema; pin mise.toml to hk = "1.48" so local and CI agree. The latest-vs-pinned drift is what let this break unnoticed. Verified both hk 1.41.1 and 1.48 accept the new schema.
  • The 1.48 schema rejects .toMap().filter(...).toMapping() in the test hook (unknown method 'filter' on Object); select the step directly instead.
  • Replace hk check || cd ../ && hk check — which parses as (hk check || cd ../) && hk check, so the second invocation always ran, in the parent directory when the first failed — with hk check --all --skip-step cargo_test. --all because hk defaults to changed files, which on a fresh CI checkout leaves most steps with nothing to do; --skip-step because the test job now owns the suite.
  • typos: exclude CHANGELOG.md at the step level. It's generated by git-cliff and abbreviated commit hashes trip the spellchecker; _typos.toml's extend-exclude can't help because it only applies when typos walks a directory, and hk passes explicit file paths.

Test Suite

  • Install cargo-nextest, set a git identity for the integration harness, and actually run cargo nextest run --all-features --no-fail-fast.
  • beta/nightly are continue-on-error so an upstream toolchain regression is visible without gating merges.

cargo-deny / advisories

  • The cargo_deny step could not have passed even with a loadable config: it passed --exclude-dev to the check subcommand (it's been a top-level flag since 0.18) and piped -f json through jq -e '.[].vulnerabilities | length == 0', but cargo-deny streams one object per diagnostic rather than an array, so jq exited 5 every run. It now gates on cargo-deny's own exit code with the informational classes allowed, preserving the original intent of failing on vulnerabilities only.
  • Fixed by upgrade: anyhow 1.0.102 → 1.0.104 (RUSTSEC-2026-0190, unsound Error::downcast_mut) and crossbeam-epoch 0.9.18 → 0.9.20 (RUSTSEC-2026-0204).
  • RUSTSEC-2026-0044 / RUSTSEC-2026-0048 (aws-lc-sys 0.38.0) are unreachable by upgrade: aws-lc-rs pins aws-lc-sys exactly, and it arrives transitively via gix[blocking-http-transport-curl-rustls] → curl-sys → rustls-ffi → rustls → aws-lc-rs. Added to deny.toml's ignore list with the reasoning and the upstream blocker recorded, mirrored into the audit-check ignore list. Worth revisiting: switching gix to blocking-http-transport-curl drops rustls and aws-lc-sys entirely (TLS would come from OpenSSL, which git2/libgit2-sys already links), but that changes the TLS backend and would complicate musl builds, so it's left as a deliberate follow-up.

First commit (58613a6) is the polish that was sitting uncommitted in the working tree from the P2/P3 work: behaviourbehavior per the declared en-us locale, two clippy idioms, and rustfmt over the test files.

Verification

  • hk check --all --skip-step cargo_test passes end to end under hk 1.48 locally (pkl, typos, cargo_fmt, cargo_deny, cargo_check, cargo_clippy all green).
  • cargo audit with the ignore list reports no vulnerabilities.
  • Full suite: 569/569 pass, 0 skipped.

Refs #62 — see #62 (comment) for the audit of what that issue still has open.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XocU6NjgZricrRWMLyLXDn

bashandbone and others added 2 commits July 25, 2026 21:55
Follow-up polish on the P2/P3 test-improvement work (#62) that landed in
ffe76d8 and 1c22370:

- Spelling: "behaviour" -> "behavior" in CLI help, doc comments, and the
  README, matching the en-us locale declared in _typos.toml.
- Clippy idioms: map(..).unwrap_or(false) -> is_ok_and, map(to_string) ->
  map(clone) on an owned String.
- rustfmt over the test files touched by the P3 work.
- _typos.toml: recognize the HTML comment form of the spellchecker:off/on
  fence, and exclude the generated CHANGELOG.md.
- hk.pkl: drop the redundant --config flag from the typos step; typos
  already discovers _typos.toml from the workspace indicator.

Refs #62

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XocU6NjgZricrRWMLyLXDn
CI has been red on main, and the green jobs were not testing anything.

**lint** died before running a single step: hk.pkl targets the hk v1.2.0
pkl schema while the workflow installs hk 1.48, which fails to evaluate it
("field not found: staticcheck"). Retarget hk.pkl at the 1.48 schema and
pin mise.toml to hk 1.48 so local and CI agree — the drift is what let this
break unnoticed. The 1.48 schema also rejects the `.toMap().filter()
.toMapping()` expression in the `test` hook, so select the step directly.

The workflow's fallback, `hk check || cd ../ && hk check`, parses as
`(hk check || cd ../) && hk check`: the second invocation always ran, in the
parent directory when the first failed. Replaced with a single
`hk check --all --skip-step cargo_test` — `--all` because hk defaults to
changed files, which on a fresh CI checkout leaves most steps with nothing
to do, and `--skip-step` because the test job now owns running the suite.

**Test Suite** checked out the repo, installed Rust, warmed three caches,
and stopped — there was no test step, so it reported success across all
three toolchains while running zero tests. Added cargo-nextest, a git
identity for the integration harness, and the actual run. beta/nightly are
marked continue-on-error so an upstream toolchain regression reports
without gating merges.

**Security Audit** was failing on live advisories, and once lint could run,
cargo-deny surfaced the same ones. Two fixes by upgrade: anyhow 1.0.102 ->
1.0.104 (RUSTSEC-2026-0190) and crossbeam-epoch 0.9.18 -> 0.9.20
(RUSTSEC-2026-0204). The two aws-lc-sys advisories are unreachable by
upgrade — aws-lc-rs pins aws-lc-sys exactly and it arrives transitively
through gix's curl-rustls transport — so they are ignored in deny.toml with
the reasoning and the upstream blocker recorded, and mirrored into the
audit-check ignore list.

Also repaired the cargo_deny step itself, which could not have passed: it
still passed --exclude-dev to the `check` subcommand (now a top-level flag)
and piped `-f json` output through `jq -e '.[].vulnerabilities | length ==
0'`, but cargo-deny streams one object per diagnostic rather than an array,
so jq exited 5 every run. It now gates on cargo-deny's own exit code,
allowing the informational classes to preserve the original intent of
failing on vulnerabilities only.

Verified locally: `hk check --all --skip-step cargo_test` passes end to end
under hk 1.48, `cargo audit` reports no vulnerabilities, and the full suite
is 569/569.

Refs #62

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XocU6NjgZricrRWMLyLXDn
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.39535% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/utilities.rs 68.42% 6 Missing ⚠️
src/git_ops/git2_ops.rs 81.81% 2 Missing ⚠️
Files with missing lines Coverage Δ
src/config.rs 89.36% <ø> (ø)
src/git_manager.rs 90.91% <100.00%> (+0.02%) ⬆️
src/git_ops/gix_ops.rs 67.11% <100.00%> (ø)
src/git_ops/simple_gix.rs 98.61% <100.00%> (ø)
src/options.rs 94.65% <100.00%> (ø)
src/git_ops/git2_ops.rs 84.52% <81.81%> (-0.13%) ⬇️
src/utilities.rs 76.96% <68.42%> (+0.02%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…nt failures

With the config finally loading, the lint job got far enough to reveal that
mise installs the Rust toolchain without optional components, so cargo_fmt
and cargo_clippy had no binary to call. Add them explicitly.

Also pass --no-fail-fast so a run reports every lint problem at once. The
previous run aborted the remaining six steps the instant cargo_fmt failed,
which made an aborted cargo_deny look like a second, independent failure.

Refs #62

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XocU6NjgZricrRWMLyLXDn
@bashandbone
bashandbone merged commit 3f25280 into main Jul 26, 2026
8 of 9 checks passed
@bashandbone
bashandbone deleted the fix/ci-hk-pkl-and-test-job branch July 26, 2026 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant