Uh oh!
There was an error while loading. Please reload this page.
ci: run e2e on develop pushes; ratchet + extend coverage floors - #301
Conversation
Scheduled workflows execute the default branch's tree (main), which trails develop by ~46 commits — discovery_e2e_test.go, delete_e2e_test.go, and the delete-teardown job don't exist there, so the nightly never actually ran them. Add push: branches: [develop] to e2e.yml so the suite runs against the tree it was written for. Ratchet the coverage floors to just under current develop numbers and add the two missing load-bearing packages: internal/cli 68 -> 80 (current 82.9%) internal/submit 72 -> 78 (current 80.4%) internal/push NEW 87 (current 89.0%) internal/cluster NEW 75 (current 78.5%) Verified locally: ./scripts/coverage-floor.sh passes all four floors. Fixes#293Fixes#294 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LukasWodka
commented
Jul 14, 2026
👋 Heads-up — Code review queue is at 37 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
…8.5% internal/cluster coverage is environment-dependent: the kubeconfig-resolution paths only execute on a machine that has a real ~/.kube/config, so a dev laptop reads 78.5% while the bare ubuntu runner reads 74.6% (first CI run on this PR). The floor gates on CI, so it must be set against the CI number: 73 (= actual - 1), not the ticket's proposed 75. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LukasWodka
commented
Jul 14, 2026
@BugBot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d1d82e2. Configure here.
saadqbal
left a comment
There was a problem hiding this comment.
Nice — floors ratchet up (all set just under the CI numbers, and Test is green so they pass), and the push: develop trigger is the right fix for the schedule-runs-on-main gap. Good catch on the CI-vs-local cluster coverage delta. 👍
Uh oh!
There was an error while loading. Please reload this page.
…findings Pinned staticcheck@2025.1.1 joins the standalone lint set in both the Makefile lint target and build.yml's lint job (-checks all,-ST1005), mirroring the errcheck/ineffassign pin pattern. The golangci-lint OOM story that originally disabled it is stale for the standalone binary: a full run is ~12s wall locally. Findings fixed (the full non-ST1005 set): - ST1008: runSet (resources_set_test) returns (string, error), error last - ST1003: CheckMaskIdColumn -> CheckMaskIDColumn (+ call sites/comments) - ST1003: errors_as -> errorsAs, io_eof_or_similar -> ioEOFOrSimilar - ST1020: CoreFloorText doc comment starts with the function name ST1005 stays excluded: it flags ~58 customer-visible error strings that need a wording review, not a mechanical sweep — separate follow-up. Rebased onto develop: union-merged the lint job with develop's govulncheck job (#278), coverage-floor step (#301) and deadcode advisory gate; kept the audit comment #286 added above CheckMaskID*Column. The 'going back to the action' comment in build.yml is KEPT (not deleted) — #6 is still open on develop, so golangci-lint-action stays disabled and that rationale still holds. Detached the four data_ingest_*.go header comments from the package clause (#303 split) so the newly-enabled staticcheck gate is ST1000-clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…findings Pinned staticcheck@2025.1.1 joins the standalone lint set in both the Makefile lint target and build.yml's lint job (-checks all,-ST1005), mirroring the errcheck/ineffassign pin pattern. The golangci-lint OOM story that originally disabled it is stale for the standalone binary: a full run is ~12s wall locally. Findings fixed (the full non-ST1005 set): - ST1008: runSet (resources_set_test) returns (string, error), error last - ST1003: CheckMaskIdColumn -> CheckMaskIDColumn (+ call sites/comments) - ST1003: errors_as -> errorsAs, io_eof_or_similar -> ioEOFOrSimilar - ST1020: CoreFloorText doc comment starts with the function name ST1005 stays excluded: it flags ~58 customer-visible error strings that need a wording review, not a mechanical sweep — separate follow-up. Rebased onto develop: union-merged the lint job with develop's govulncheck job (#278), coverage-floor step (#301) and deadcode advisory gate; kept the audit comment #286 added above CheckMaskID*Column. The 'going back to the action' comment in build.yml is KEPT (not deleted) — #6 is still open on develop, so golangci-lint-action stays disabled and that rationale still holds. Detached the four data_ingest_*.go header comments (#303 split) plus exitcodes.go's (#284, which develop merged after this branch was cut) from the package clause so the newly-enabled staticcheck gate is ST1000-clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…findings Pinned staticcheck@2025.1.1 joins the standalone lint set in both the Makefile lint target and build.yml's lint job (-checks all,-ST1005), mirroring the errcheck/ineffassign pin pattern. The golangci-lint OOM story that originally disabled it is stale for the standalone binary: a full run is ~12s wall locally. Findings fixed (the full non-ST1005 set): - ST1008: runSet (resources_set_test) returns (string, error), error last - ST1003: CheckMaskIdColumn -> CheckMaskIDColumn (+ call sites/comments) - ST1003: errors_as -> errorsAs, io_eof_or_similar -> ioEOFOrSimilar - ST1020: CoreFloorText doc comment starts with the function name ST1005 stays excluded: it flags ~58 customer-visible error strings that need a wording review, not a mechanical sweep — separate follow-up. Rebased onto develop: union-merged the lint job with develop's govulncheck job (#278), coverage-floor step (#301) and deadcode advisory gate; kept the audit comment #286 added above CheckMaskID*Column. The 'going back to the action' comment in build.yml is KEPT (not deleted) — #6 is still open on develop, so golangci-lint-action stays disabled and that rationale still holds. Detached the four data_ingest_*.go header comments (#303 split) plus exitcodes.go's (#284, which develop merged after this branch was cut) from the package clause so the newly-enabled staticcheck gate is ST1000-clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…findings (#302) * ci: re-enable staticcheck (pinned standalone) + fix the 5 non-ST1005 findings Pinned staticcheck@2025.1.1 joins the standalone lint set in both the Makefile lint target and build.yml's lint job (-checks all,-ST1005), mirroring the errcheck/ineffassign pin pattern. The golangci-lint OOM story that originally disabled it is stale for the standalone binary: a full run is ~12s wall locally. Findings fixed (the full non-ST1005 set): - ST1008: runSet (resources_set_test) returns (string, error), error last - ST1003: CheckMaskIdColumn -> CheckMaskIDColumn (+ call sites/comments) - ST1003: errors_as -> errorsAs, io_eof_or_similar -> ioEOFOrSimilar - ST1020: CoreFloorText doc comment starts with the function name ST1005 stays excluded: it flags ~58 customer-visible error strings that need a wording review, not a mechanical sweep — separate follow-up. Rebased onto develop: union-merged the lint job with develop's govulncheck job (#278), coverage-floor step (#301) and deadcode advisory gate; kept the audit comment #286 added above CheckMaskID*Column. The 'going back to the action' comment in build.yml is KEPT (not deleted) — #6 is still open on develop, so golangci-lint-action stays disabled and that rationale still holds. Detached the four data_ingest_*.go header comments (#303 split) plus exitcodes.go's (#284, which develop merged after this branch was cut) from the package clause so the newly-enabled staticcheck gate is ST1000-clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: goimports -local gate + Dependabot config (gomod weekly, actions monthly) (#304) * ci: goimports -local gate + Dependabot config (gomod weekly, actions monthly) goimports -local github.com/tracebloc/cli now blocks in both loops: build.yml's lint job (pinned goimports@v0.48.0, same x/tools version as the deadcode pin) and the Makefile's fmt-check target, with make fmt extended to auto-fix. .golangci.yml already declared this grouping via local-prefixes but nothing enforced it — 4 files had drifted (data.go, data_test.go, ingestion_run_test.go, resources_set_test.go), fixed here with import-grouping-only diffs. .github/dependabot.yml extends the org's backend-only Dependabot pilot: gomod weekly (k8s.io/* + sigs.k8s.io/* grouped, golang.org/x/* grouped), github-actions monthly. Unlike backend's security-only config, the CLI takes real version updates — customers install this binary, so staying current is security posture (see #276). Org-wide rollout decision flagged to Asad on the PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: canonicalize import grouping in data_test.go for the goimports gate Regroups imports into the layout goimports -local github.com/tracebloc/cli emits (stdlib / third-party / tracebloc-local). gofmt and goimports only sort within existing blank-line groups, so the manual groups passed fmt-check as-is but were not the canonical single-block output; Bugbot and our precheck both flagged the divergence. Verified idempotent under the pinned goimports v0.48.0 and green on make fmt-check + go build ./... Rebased onto ci/279 (post-#303 data.go split): data.go's import block is already canonical from the #303 split, so its canonicalization here is a no-op and dropped — this commit now regroups data_test.go only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * ci: flip deadcode gate to blocking; delete the 3 dead #127 leftovers (#313) * ci: goimports -local gate + Dependabot config (gomod weekly, actions monthly) goimports -local github.com/tracebloc/cli now blocks in both loops: build.yml's lint job (pinned goimports@v0.48.0, same x/tools version as the deadcode pin) and the Makefile's fmt-check target, with make fmt extended to auto-fix. .golangci.yml already declared this grouping via local-prefixes but nothing enforced it — 4 files had drifted (data.go, data_test.go, ingestion_run_test.go, resources_set_test.go), fixed here with import-grouping-only diffs. .github/dependabot.yml extends the org's backend-only Dependabot pilot: gomod weekly (k8s.io/* + sigs.k8s.io/* grouped, golang.org/x/* grouped), github-actions monthly. Unlike backend's security-only config, the CLI takes real version updates — customers install this binary, so staying current is security posture (see #276). Org-wide rollout decision flagged to Asad on the PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: canonicalize import grouping in data_test.go for the goimports gate Regroups imports into the layout goimports -local github.com/tracebloc/cli emits (stdlib / third-party / tracebloc-local). gofmt and goimports only sort within existing blank-line groups, so the manual groups passed fmt-check as-is but were not the canonical single-block output; Bugbot and our precheck both flagged the divergence. Verified idempotent under the pinned goimports v0.48.0 and green on make fmt-check + go build ./... Rebased onto ci/279 (post-#303 data.go split): data.go's import block is already canonical from the #303 split, so its canonicalization here is a no-op and dropped — this commit now regroups data_test.go only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: flip deadcode gate to blocking; delete the 3 dead #127 leftovers The deadcode CI step loses continue-on-error (and the Makefile target its '|| true'): both now run scripts/deadcode-check.sh, which fails on any function unreachable from ./cmd/tracebloc that isn't declared in scripts/deadcode-allowlist.txt. The tool itself always exits 0, so the old advisory step never could have blocked — the gate keys on output. Deleted (verified still dead with the pinned deadcode@v0.48.0): - config.clearAll + its three dedicated tests (TestClearAll, TestClearAll_HomeError, TestClear) — logout uses Save, nothing else ever called it - push.allCategoryIDs — moved verbatim into category_registry_test.go (the registry-pinning tests legitimately iterate every id; the shipped binary shouldn't carry the helper) - submit.isSubmitError — moved into client_test.go as the test-local assertion helper it always was (orphaned 'errors' import dropped) Allowlisted with reasons (the 4 legit findings): Status.String + JobOutcome.String (fmt-reflection Stringers) and ReadLabelValues + inferColumnType (di#349 test-only parity harnesses). Stale allowlist entries warn without failing; line numbers are stripped so edits that shift code don't red the gate. Coverage floors still clear after the test deletions (cli 82.9% >= 68, submit 80.3% >= 72). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
e2e on develop pushes (#293). Scheduled workflows always execute the default branch's tree —
main, currently ~46 commits behinddevelop.discovery_e2e_test.go,delete_e2e_test.go, and the wholedelete-teardownjob don't exist onmain, so the nightly has never actually run them (the TokenRequest/offboard e2e included). This addspush: branches: [develop]toe2e.yml(existing schedule / dispatch / labeled-PR triggers kept), so the suite runs against the tree the tests were written for.Coverage floor ratchet (#294). Bumps the two existing floors to just under current develop numbers and adds the two load-bearing packages that had no floor (
internal/pushis the 6.3k-LOC money path):coverage-floor.shmeasures, ubuntu CI runner)internal/cliinternal/submitinternal/pushinternal/clusterDeviation from the ticket:
internal/clustergets 73, not the proposed 75. Its coverage is environment-dependent — the kubeconfig-resolution paths only execute on a machine with a real~/.kube/config, so a dev laptop reads 78.5% while the bare CI runner reads 74.6% (caught by this PR's own first CI run). The floor gates on CI, so it's set to CI-actual − 1, per the ratchet policy. Documented in the script header.Also updates the one-line step name in
build.yml("cli, submit, push, cluster must not rot") so it doesn't misname the floor set — expected trivial rebase vs sibling PRs touching this file — merge order flexible.Test plan
Ran locally:
./scripts/coverage-floor.sh— all four floors pass locally; first CI run then failed oninternal/cluster(74.6% on the runner vs 78.5% locally), floor corrected to 73 — CI green is the real verification here.e2e.yml+build.yml.NOT run locally: the e2e suite itself (needs kind/k3d); the new push trigger will exercise it on the first merge to
develop.Fixes#293
Fixes#294
🤖 Generated with Claude Code
Note
Low Risk
CI-only workflow and coverage-threshold changes; no runtime product behavior.
Overview
e2e on
developpushes (#293). Adds apushtrigger ondevelopto the integration workflow so kind/k3d suites run against the branch where newer tests (discovery, delete, offboard teardown) actually live. Scheduled runs still use the default branch (main), which had been skipping that work; schedule, manual dispatch, and label-gated PR runs are unchanged.Coverage ratchet (#294). Raises floors for
internal/cli(80) andinternal/submit(78) and adds floors forinternal/push(87) andinternal/cluster(73). The script header documents current CI percentages and setsinternal/clusterfrom the bare Ubuntu runner (not a laptop with kubeconfig). The Build workflow step label is updated to mention all four gated packages.Reviewed by Cursor Bugbot for commit d1d82e2. Bugbot is set up for automated code reviews on this repo. Configure here.