Skip to content

Two tests report a build failure as a PASS (t.Skipf on 'cannot build cmd/rbgo'), plus the full env-var-gate inventory #687

Open

Description

@tannevaled

Two tests turn a build failure into a PASS:

  • internal/vm/nethttp_bind_test.go:387
  • internal/vm/socket_test.go:424

both:

build := exec.Command("go", "build", "-o", bin, "./cmd/rbgo")
build.Dir = repoRoot(t)
build.Env = append(os.Environ(), "GOWORK=off")
if out, err := build.CombinedOutput(); err != nil {
	t.Skipf("cannot build cmd/rbgo (%v): %s", err, out)
}

If cmd/rbgo stops compiling, both report skip, the lane is green, and the
binary-level HTTP and socket coverage silently stops being exercised. t.Skipf
here says "this machine cannot build it"; what it actually reports is "the
product is broken". Those are opposite meanings and only one of them should be
green.

They should be t.Fatalf. There is no environment in which this repo's own
cmd/rbgo legitimately fails to build — every CI lane builds it, and a developer
who cannot build it wants to know.

This is the same defect class as #682, found the same way

#682 was a guard that could not run (behind RBGO_BUILD_IT=1, which no workflow
set) and so failed open for three days. This is a guard that runs and reports
success on the failure it exists to catch
. Both are "a check whose silence is
indistinguishable from a pass".

The sweep that found these also turned up the rest of the picture, worth
recording here so it is not rediscovered one at a time:

  • -tags rbgo_closed was compiled by no workflow at all — addressed by internal/vm, ci: fix the js/wasm build, and make the guard that caught it reachable #684's
    new lane.

  • testing.Short() (3 sites): no workflow passes -short, so these do run.
    The inverse case, listed for completeness.

  • Capability-probe skips (11): symlink, /dev/zero, mkfifo,
    user.Current(), Geteuid()==0. Satisfied on today's runners; they would go
    quiet rather than red if that changed.

  • Platform skips (~24): all three OS lanes exist, so every branch is reached.

  • 2 "gap recorded" skips in scripts/conformance/webapp/webapp_test.go:131,177
    — that package is in ./..., so a removed binding presents as a skip.

  • 1 skip that can never fire, internal/vm/grape_internal_test.go:171:

    r := &GrapeRoute{rt: grape.NewRoute("GET", "/x", nil), handler: nil}
    if r.handler != nil {
        t.Skip("handler set")
    }

    handler is set to nil two lines above, so the skip is dead code. Harmless,
    but it makes the test read as conditional when it is not.

The two t.Skipf sites are the ones that matter; the rest is inventory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions