Network-dependent tests make CI's red signal ambiguous #439

Activity

1. David-Noble-at-work

   self-assigned this

   on Aug 17, 2026
2. David-Noble-at-work

   added

   taskImplementation work itemImplementation work item

   Epic:WindowsCampaignThe Windows campaign epicThe Windows campaign epic

   on Aug 17, 2026
3. David-Noble-at-work commented on Aug 18, 2026

   

   David-Noble-at-work

   on Aug 18, 2026

   ContributorAuthor

   More actions

   A second offender, with a different failure mode (2026-08-18)

   Observed on PR #515, whose diff is one markdown file, 2 insertions and 1 deletion. test (ubuntu-latest) went red. The innocence of the change is unambiguous, which makes this a clean instance of
   exactly what this task describes.

   What happened

   No --- FAIL: lines anywhere in the log. The package hit its wall instead:

   panic: test timed out after 2m0s
   	running tests:
   		TestResolveWithConfidence_NativePackage (1m59s)
   
   FAIL	github.com/NobleFactor/devlore-cli/cmd/internal/lorepackage	120.047s
   

   The failing job.

   Why it widens this task's scope

   1. It is not TestRegistry_SyncIntegration. The charter names that test as the known case; this is a
      second one, in cmd/internal/lorepackage, and nothing had catalogued it.
   2. It fails by hanging, not by erroring. The original evidence was a macOS DNS failure on PR feat(op): make the session own filesystem access behind accessors #414 — a
      fast, loud error after a 30s timeout. This one sat for 119 seconds and was killed by the test binary's
      own alarm. A fix that only handles resolution or connection errors would not have caught this, which
      is a direct argument for the charter's insistence on enumerating by behavior rather than by name.
   3. One hung test destroys the whole package's verdict. The timeout is per test binary, so every other
      test in cmd/internal/lorepackage reported nothing — they neither passed nor failed. The blast radius of
      a single network-reaching test is larger than that test.

   The compounding cost

   The Windows leg carries a known-failure baseline verified name-for-name on every PR. That discipline works
   because a changed set means something real. An ubuntu leg that goes red for environmental reasons trains
   the opposite reflex on the other three legs — and this instance is worse than a wrong answer, because with
   no --- FAIL: lines there is no name-for-name comparison to make at all. The signal is simply absent.
4. David-Noble-at-work commented on Aug 18, 2026

   

   David-Noble-at-work

   on Aug 18, 2026

   ContributorAuthor

   More actions

   Third instance, and the first that is not a test (2026-08-18)

   quality-gate on PR #548 sat 15+ minutes against a typical 5–8, with every other leg finished. The job
   steps place it exactly:

    9 completed  success  Vet
   10 in_progress    -    Install lint tools     <-- here
   11 pending        -    Lint Go
   

   Not a test. A setup step.

   What that step does

   for attempt in 1 2 3; do
     curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/v2.12.2/install.sh | sh -s -- ... && break
     echo "golangci-lint install attempt $attempt failed; retrying in 5s" >&2
     sleep 5
   done
   go install mvdan.cc/sh/v3/cmd/shfmt@latest
   sudo apt-get update && sudo apt-get install -y shellcheck

   Four network operations: raw.githubusercontent.com, the Go module proxy, and two apt round trips.

   Two defects, and the first is this task's own lesson

   1. The retry loop is hang-blind. curl carries no --connect-timeout and no --max-time, so a stalled
   connection hangs indefinitely and the loop never reaches its break-or-retry decision. Retries handle
   failures; this is a hang.

   That is the same distinction the ubuntu instance taught earlier today:
   TestResolveWithConfidence_NativePackage sat for 119 seconds until the test binary's own alarm killed it.
   There at least an alarm existed. Here there is none — only the six-hour job ceiling.

   2. shfmt@latest is unpinned. A new shfmt release can change formatting verdicts with no commit on our
   side. Non-reproducible by construction, and squarely #252's subject.

   This task is scoped too narrowly

   The charter says "enumerate every network-reaching test — by behavior, not by name." That scope would
   have missed this entirely: a curl | sh, a module-proxy fetch and apt-get update are network-reaching CI
   operations, and none of them is a test.

   Three instances today, three different mechanisms, one of them a test:

   #	Where	Mechanism
   1	TestRegistry_SyncIntegration (PR #414, macOS)	DNS failure — fast, loud error
   2	TestResolveWithConfidence_NativePackage (ubuntu)	hang until the 120s package alarm
   3	Install lint tools (ubuntu, this PR)	hang with no alarm at all

   The enumeration should be of network-reaching operations, not tests.

   Mitigations available without answering the whole charter

   • Give curl a --max-time so a stall becomes a retryable failure — the existing loop then works as
     intended.
   • Pin shfmt instead of @latest.
   • Cache all three tools keyed on their versions; today every run re-downloads them.
5. David-Noble-at-work commented on Aug 18, 2026

   

   David-Noble-at-work

   on Aug 18, 2026

   ContributorAuthor

   More actions

   Another instance for the enumeration this charter asks for, found by the hang rather than by the survey.

   ResolveWithConfidence and searchNative both reached the network -- not by cloning, like
   TestRegistry_SyncIntegration, but by calling platform.Detect() and then a PackageManager query whose
   staleness gate ran apt-get update underneath it. TestResolveWithConfidence_NativePackage hung twice in one
   day, 119s each time, taking the whole package's verdict down with it and leaving no --- FAIL: line to read.

   Worth noting for the survey's method: neither test names the network. Nothing in the call site suggests a
   fetch; the network was three layers down, inside a method called Available. An enumeration by name, or by
   reading test bodies, would have missed both. The behavioral criterion this charter already specifies is the
   right one -- this is evidence for how far down it has to look.

   Fixed in the PR that reshapes the index gate: queries refresh only when the index is absent, never merely
   because it is old.
6. David-Noble-at-work

   added a commit that references this issue on Aug 18, 2026

   fix(platform): keep the network out of package-manager reads (#549)

   ...

   4976c10