Skip to content

continuous integration

Kadyapam edited this page Oct 4, 2026 · 2 revisions

Continuous Integration

.github/workflows/test.yml is the pre-merge gate, on pull_request and pushes to main.

step command
member versions ./ci/check-member-versions.sh
build cargo build --workspace --all-targets --locked
test cargo test --workspace --all-targets --locked
clippy cargo clippy --workspace --all-targets --locked -- -D warnings -A clippy::large_enum_variant -A clippy::result_large_err

⚠ member versions runs first, and a failure there hides everything else

ci/check-member-versions.sh (Release Member Versions, noetl/ai-meta#331) runs before the toolchain is installed. When it fails, build, test and clippy are all reported skipped — not green, unavailable.

This is worth knowing because a PR in that state looks like whatever you were expecting to fail. One clippy-gate PR read as a clippy failure and was not: it had changed executor/ while leaving executor/Cargo.toml unmoved, so the release would have cut a tag and published a noetl-executor that did not contain the change. Check which step failed before diagnosing.

If you touch executor/, bump executor/Cargo.toml in the same PR. The script will not guess patch-vs-minor for you, by design.

clippy gates, with two documented exceptions

-D warnings is live as of noetl/ai-meta#378. It previously ran under || true, so the step cost its full runtime on every PR and could not fail.

Clearing the way took 39 lints — and note the measurement trap: a -D warnings run reports only 15 of them, because warnings-as-errors aborts compilation at the first failing crate, so noetl-executor fails and the main noetl crate (holding 23 of the 39) is never linted at all. Use a non-gating cargo clippy to inventory; the gating run is not an inventory.

The two -A entries, and why they are not just lints to fix

lint why
large_enum_variant EhdbQueryCommand (src/lib.rs), Tool and ToolSpecDoc (executor/src/playbook.rs) are clap Subcommand and serde tagged/untagged derive enums. Boxing the large variants is clippy's fix, but Tool is also derived under the json-schema feature that generates the published DSL schema, so boxing changes that schema's output — a downstream contract change.
result_large_err the large Err is tonic::Status; clippy's advice is to shrink tonic::Status, which is upstream. Single test-helper site in arrow-flight-client/src/tests.rs.

Any new -A must carry its reason on the same line. An unexplained -A reads as configuration rather than debt and never gets revisited.

clippy.toml, and why a threshold beats an -A

too-many-arguments-threshold = 8

Two functions in src/lib.rs take 8 arguments against clippy's default ceiling of 7. Raising the threshold keeps the lint live: a function growing to 9 arguments still fails the gate (verified with a 9-arg probe, which fired (9/8)). -A clippy::too_many_arguments would have disabled it everywhere. Prefer a threshold or a site-scoped #[allow] over a workflow-level -A.

⚠ clippy --fix is a suggestion, not a correctness proof

Reviewing its output is not optional. On this crate its manual_flatten fix rewrote

for line in reader.lines() { if let Ok(line) = line { … } }

into reader.lines().flatten() — which is lines_filter_map_ok: flatten() loops forever if the reader repeatedly returns Err. It traded a style lint for a potential hang. Both sites are now map_while(Result::ok), which stops at the first error exactly as the original did.

The toolchain is pinned

rust-toolchain.toml pins channel = "1.99.0"; the workflow installs @stable but the pin wins, so the lint set only moves when that file changes. Every Rust repo in the fleet now carries the same pin, so a Rust release cannot turn a gate red on its own. Raised from 1.91.1 on 2026-10-04 (noetl/cli#94, noetl/ai-meta#398).

The cost was measured before the pin moved rather than discovered after: exactly one lint, clippy::needless_borrow in src/playbook_runner.rs — a & on an already-borrowed rhai inside the truncation format!.

Corollary: a local clippy green is not evidence about CI unless your toolchain matches the pin. Read the version from the lint's own rust-clippy/rust-<x.y.z> documentation URL in the CI output — the dtolnay/rust-toolchain banner reports what it installed, not what the pin then selected.

The release pipeline builds two architectures

build-deb in release.yml is a matrix, not a single job:

runner Architecture:
ubuntu-latest amd64
ubuntu-24.04-arm arm64

Native runners, not a cross-compile. The apt job downloads both with pattern: deb-* and merge-multiple: true, discovers the architectures present from the filenames, and indexes each separately:

ARCHES="$(find pool/main -maxdepth 1 -name '*.deb' \
  | sed -nE 's/.*_([a-z0-9]+)\.deb$/\1/p' | sort -u | tr '\n' ' ')"

for ARCH in ${ARCHES}; do
  dpkg-scanpackages --multiversion --arch "${ARCH}" pool/main /dev/null \
    > "dists/${DIST}/main/binary-${ARCH}/Packages"
done

Three details here are load-bearing and easy to undo (noetl/ai-meta#390):

  • --multiversion — without it only the newest version of each package is indexed, so apt-get install noetl=<older> cannot resolve. 22 of 23 published .debs were unreachable this way.
  • --arch "${ARCH}" — without it dpkg-scanpackages writes every .deb into whichever index it is told to produce, ignoring Architecture:. Harmless while only amd64 shipped; it would serve arm64 binaries to amd64 clients the moment arm64 exists.
  • Release is written to $RUNNER_TEMP and mvd in. apt-ftparchive release D > D/Release scans the directory it writes into, so a direct redirect makes Release list itself.

sed -nE … p, not sed -E s///: the substitution form passes non-matching filenames through unchanged, turning a stray file in pool/ into an "architecture" named after its path.

noetl/apt's PR gate fails on all of the above — see repository integrity.

Related

Clone this wiki locally