Skip to content

ci: cut unit-test duplication, and stop a flaky download failing the build - #16

Closed
weiishann wants to merge 4 commits into
mainfrom
feature/optimise-cicd
Closed

weiishann wants to merge 4 commits into
mainfrom
feature/optimise-cicd

Conversation

@weiishann

Copy link
Copy Markdown
Collaborator

Why

PR #15 lost a whole unit-test matrix to a transient sbt-launch.jar download: one leg died in 70 ms, and fail-fast cancelled the other. Digging into that, a passing run turned out to spend most of its time doing work twice.

Measured from run 35453363517, per matrix leg (~22.5 min):

Step Time Share
integrationTests/test:compile 69s 5%
clean package publishM2 52s 4%
+jacoco 952s 70%
licenseCheck 265s 20%

Only spark and integrationTests read sparkVersion, but +jacoco ran the whole root aggregate in both legs — so ~7 min of server and cli tests, plus a ~4.4 min licenseCheck, ran twice per pull request, identically.

What changed

unit-tests.yml splits into two jobs. core runs the eight Spark-independent modules once; spark matrixes only the modules that actually depend on the Spark version. Together they cover exactly root's .aggregate(...) — nothing dropped, nothing run twice.

licenseCheck moves to its own workflow, gated on build.sbt, version.sbt and project/**, since dependency licences can only change when those do.

The launcher download retries. --retry covers 5xx and timeouts, --retry-all-errors extends that to connection resets, --show-error stops --silent hiding the HTTP status. dev/test-sbt-launch-lib.sh covers both failure modes against a local server, and was confirmed to fail before the change and pass after.

Spark's test logging is quieted. 226,580 of 240,773 lines in a passing job were Spark INFO messages on stderr, which sbt tags [error]. All of it came from the connector tests. Measured on SchemaOperationsTest: −77% log lines, same 5/5 result.

Also: dependency caching, fail-fast: false, concurrency groups, timeout-minutes on every job, jacoco reports uploaded instead of discarded, format and UI checks extended to branch-*, and a Dependabot config.

Projected from the measured step times: runner time ~45 → ~31 min, wall-clock ~22.5 → ~12.5 min.

Please note before merging

test (Spark 4.0.0) and test (Spark 4.1.0) keep their exact names, because both are required status checks — renaming them would leave every PR waiting on checks that never report.

core and license-check are new contexts and are not in branch protection. Until they're added, the server and CLI suites and the licence check all run and report but do not block a merge. They gate today via test (Spark X), so this is a real gap that should be closed right after merge.

For the same reason, path-filtering unit-tests.yml is not included here: a required check skipped by a path filter never reports, and the pull request hangs forever.

Verification

  • Both required contexts still produced, verified by expanding the matrix from the parsed YAML
  • core ∪ spark == root's .aggregate(...), verified against build.sbt
  • All 10 sbt task keys resolve, sbt exit 0
  • Launcher tests red before the fix, green after; real Maven Central fetch still works
  • shellcheck: 13 findings before, 13 after — identical, all pre-existing
  • Spark connector test passes 5/5 with the new logging config

🤖 Generated with Claude Code

weiishann and others added 4 commits September 20, 2026 22:54
The launcher jar is gitignored, so every CI job re-fetches it from Maven
Central before sbt can start, and the curl call had no retry. A single
transient error there failed the whole job: on PR #15 one matrix leg died
in 70ms on this download and fail-fast then cancelled the other, so the
pull request got no test signal at all.

--retry covers 5xx and timeouts, --retry-all-errors extends that to
connection resets, and --show-error stops --silent from swallowing the HTTP
status when every attempt fails.

dev/test-sbt-launch-lib.sh exercises both failure modes against a local
server and was confirmed to fail before this change and pass after.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…version

Measured against a passing run, each matrix leg took ~22.5 minutes: ~16 for
+jacoco and ~4.4 for licenseCheck. Only `spark` and `integrationTests` read
sparkVersion, yet `+jacoco` ran the whole root aggregate in both legs, so the
~7 minutes of server and cli tests were executed twice, identically.

unit-tests.yml now has a `core` job that runs the eight Spark-independent
modules once, and a `spark` job that matrixes only the modules that depend on
the Spark version. Together they cover exactly root's .aggregate(...) - no
module is dropped and none runs twice.

The `spark` job keeps the job names `test (Spark 4.0.0)` and
`test (Spark 4.1.0)` verbatim because both are required status checks on main;
renaming them would leave every pull request waiting on checks that never
report. `core` is deliberately not added to branch protection yet.

licenseCheck moves to its own workflow, gated on the build files, since
dependency licences can only change when those change.

Also, across the workflows:

- cache ~/.sbt, ~/.ivy2/cache and ~/.cache/coursier, plus the launcher jar
- fail-fast: false, so one leg's flake no longer cancels the other's results
- concurrency groups, matching the idiom already used elsewhere in the repo
- timeout-minutes on every job, sized to ~3-4x the measured runtime
- upload the jacoco reports, which were previously written and discarded
- run the format and UI checks on branch-* too, as the other workflows do

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
In a passing unit-test job, 226,580 of 240,773 log lines were Spark INFO
messages. Spark logs to stderr, which sbt tags [error], so a green run was
full of [error] lines and a real failure was effectively unfindable. All of
that volume came from the Spark connector tests - the server and cli phases
contributed none.

There was no log4j2 config on the test classpath, so Spark fell back to its
bundled defaults. This keeps the framework at WARN and io.unitycatalog at
INFO, so Unity Catalog's own diagnostics survive.

Measured on SchemaOperationsTest: log lines drop 77% (878 -> 203) with the
same result, 5 of 5 passing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Action pins age out quietly: the checkout and setup-java pins in use target
Node 20, which the runners have already deprecated.

sbt is not a supported ecosystem, so build.sbt still needs updating by hand.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@weiishann weiishann closed this Oct 3, 2026
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