Skip to content

feat(testing): bootstrap 8 rule blocks in go-testing-guide#10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bborbe merged 1 commit into from
Jun 2, 2026
Merged

feat(testing): bootstrap 8 rule blocks in go-testing-guide #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 89 additions & 15 deletions docs/go-testing-guide.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,18 +15,29 @@ Key principles:

## Critical Rules

**MUST NOT use stdlib `testing` table-driven tests.** Always use Ginkgo `DescribeTable`/`Entry`. If a `*_suite_test.go` with Ginkgo imports exists in the package, all tests must use Ginkgo.
### RULE go-testing/no-stdlib-table-tests (MUST)

**Owner**: go-test-quality-assistant
**Applies when**: a Go test file in a package that has a Ginkgo `*_suite_test.go` uses `t.Run` inside a `for _, tt := range tests` loop instead of `DescribeTable`/`Entry`.
**Enforcement**: judgment (ast-grep follow-up)
**Why**: Mixed Ginkgo + stdlib tables produce inconsistent reporter output, fragmented runs, and surprises with `--focus` / `--label-filter`. Single-framework enforcement keeps test runs predictable.

#### Bad

```go
// BAD — stdlib table-driven test
// stdlib table-driven test in a Ginkgo-suite package
func TestFoo(t *testing.T) {
tests := []struct{ input, want string }{ ... }
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { ... })
}
}
```

#### Good

// GOOD — Ginkgo DescribeTable
```go
// Ginkgo DescribeTable
var _ = DescribeTable("foo",
func(input, expected string) {
Expect(foo(input)).To(Equal(expected))
Expand All@@ -36,22 +47,60 @@ var _ = DescribeTable("foo",
)
```

**MUST NOT use `testing.T` directly** in packages that have a Ginkgo test suite. Use `Describe`/`Context`/`It`/`DescribeTable`/`Entry` instead.
### RULE go-testing/no-testing-t-direct (MUST)

**Owner**: go-test-quality-assistant
**Applies when**: a Go file in a package that has a Ginkgo `TestSuite` entry-point uses `*testing.T` directly inside test functions other than the suite entry-point itself.
**Enforcement**: judgment (ast-grep follow-up)
**Why**: Direct `testing.T` use bypasses the Ginkgo lifecycle (`BeforeEach`/`AfterEach`/`JustBeforeEach`), produces flaky setup ordering, and breaks `--focus` filtering. Use `Describe`/`Context`/`It`/`DescribeTable`/`Entry` so the suite runs as one coherent test plan.

**MUST NOT call an error-returning function bare in an `It` block.** `errcheck` (run by `make precommit`) will fail. Wrap with a matcher that documents intent:
#### Bad

```go
func TestUserService(t *testing.T) { // direct testing.T in a Ginkgo-suite package
t.Run("Create", func(t *testing.T) {
// ...
})
}
```

- Expecting success: `Expect(someFunc(ctx)).To(Succeed())`
- Expecting failure: `Expect(someFunc(ctx)).To(HaveOccurred())`
- Need the error: `err := someFunc(ctx); Expect(err).To(MatchError(...))`
#### Good

```go
// BAD — errcheck: "Error return value not checked"
var _ = Describe("UserService", func() {
Context("Create", func() {
It("creates a user with valid data", func() {
// ...
})
})
})
```

### RULE go-testing/no-bare-error-call (MUST)

**Owner**: go-test-quality-assistant
**Applies when**: a Go test file inside an `It` / `BeforeEach` / `JustBeforeEach` / `AfterEach` block calls an error-returning function whose return value is discarded.
**Enforcement**: judgment (ast-grep follow-up — errcheck-equivalent scoped to Ginkgo blocks)
**Why**: `errcheck` (run by `make precommit`) flags discarded errors and breaks the build. Wrapping every error-returning call in a Gomega matcher (`Succeed()` / `HaveOccurred()` / `MatchError(...)`) documents the test's intent at the assertion site instead of relying on silent fall-through.

Matcher choice by intent:
- Expecting success: `Expect(fn(ctx)).To(Succeed())`
- Expecting failure: `Expect(fn(ctx)).To(HaveOccurred())`
- Need the error for further assertions: `err := fn(ctx); Expect(err).To(MatchError(...))`

#### Bad

```go
// errcheck: "Error return value not checked"
It("calls Save exactly twice", func() {
service.Process(ctx)
Expect(store.SaveCallCount()).To(Equal(2))
})
```

// GOOD — error explicitly accounted for
#### Good

```go
It("calls Save exactly twice", func() {
Expect(service.Process(ctx)).To(HaveOccurred())
Expect(store.SaveCallCount()).To(Equal(2))
Expand All@@ -60,7 +109,12 @@ It("calls Save exactly twice", func() {

## Test Suite Setup

**MUST provide a `*_suite_test.go` file in every package with tests.** Without it, Ginkgo specs are not discovered and `make test` silently misses coverage.
### RULE go-testing/suite-test-file-required (MUST)

**Owner**: go-test-quality-assistant
**Applies when**: a Go package contains test files (`*_test.go`) but no `*_suite_test.go` file with a `TestSuite` entry-point and `RunSpecs`.
**Enforcement**: judgment (file-existence check; ast-grep follow-up)
**Why**: Without a suite file, Ginkgo specs are not discovered. `make test` exits 0 even though no specs ran — silent coverage loss. The suite file is the single entry-point Go's `testing` package invokes.

### Standard Package Suite

Expand DownExpand Up@@ -101,7 +155,12 @@ Requirements:

### Main Package Suite (special case)

**MUST include `main_test.go` for every binary project.** Without it, build failures are not caught by `make test`.
### RULE go-testing/main-test-with-compiles (MUST)

**Owner**: go-test-quality-assistant
**Applies when**: a Go binary project (package `main` with `main.go`) does not have a `main_test.go` containing a `Compiles` It-block backed by `gexec.Build`.
**Enforcement**: judgment (file-existence + body check; ast-grep follow-up)
**Why**: Without `main_test.go` + a `Compiles` check, build failures in `main.go` are not caught by `make test`. The CI greenlight then deploys a binary that doesn't link.

```go
// Copyright (c) 2026 Benjamin Borbe All rights reserved.
Expand DownExpand Up@@ -177,7 +236,12 @@ var _ = Describe("Product", func() {

## Test Timeouts

**MUST set a suite-level timeout.** Every suite file includes `suiteConfig.Timeout` as a safety net against hanging tests.
### RULE go-testing/suite-timeout-required (MUST)

**Owner**: go-test-quality-assistant
**Applies when**: a `*_suite_test.go` file calls `GinkgoConfiguration()` without setting `suiteConfig.Timeout` before `RunSpecs`.
**Enforcement**: judgment (ast-grep follow-up — pattern over suite body)
**Why**: Without a suite-level timeout, a hung test holds the test runner indefinitely. CI eventually kills the job — but only after the job-level timeout (often 30+ minutes), wasting CI minutes and delaying feedback. Suite-level timeout is the safety net that fails fast.

Per-spec timeout:

Expand All@@ -199,7 +263,12 @@ Describe("slow subsystem", func() {

## Mock Generation

**MUST use Counterfeiter-generated mocks.** Never hand-write mocks — they drift from the interface and break silently.
### RULE go-testing/counterfeiter-mocks-required (MUST)

**Owner**: go-test-quality-assistant
**Applies when**: a test file declares a hand-written struct that satisfies a production interface and is used in place of a real implementation under test, instead of importing a `mocks/<Name>` fake produced by Counterfeiter.
**Enforcement**: judgment (presence of `//counterfeiter:generate` directive + hand-written fake detection; ast-grep follow-up)
**Why**: Hand-written mocks drift from the interface — when the production interface gains a method, the hand-written mock silently keeps satisfying the old surface (test still compiles, doesn't exercise the new contract). Counterfeiter-generated fakes regenerate from the interface, so any drift surfaces at `go generate` time.

### Generate Directive

Expand DownExpand Up@@ -237,7 +306,12 @@ Expect(actualUser.Name).To(Equal("test"))

## Time Handling

**MUST inject time via `libtime.CurrentDateTimeGetter` from `github.com/bborbe/time`.** Never call `time.Now()` directly in business logic — tests cannot control it.
### RULE go-testing/libtime-injection-required (MUST)

**Owner**: go-test-quality-assistant
**Applies when**: a Go business-logic file (outside `main.go`, `cmd/**`, `*_test.go`, `vendor/`) reads the current time. Tests cannot control `time.Now()` directly, so dependent code is unverifiable.
**Enforcement**: cross-rule — overlaps with `go-time/no-time-now-direct` (already in `rules/index.json`). This rule scopes the same constraint to test-coverage assessments: a service that doesn't inject time has no testable time-dependent paths.
**Why**: Without `libtime.CurrentDateTimeGetter` injection, every test that depends on time becomes flaky or impossible. `libtime.NewCurrentDateTime()` + `SetNow(fixedTime)` produces deterministic, fast tests for date math, expiry windows, scheduling, etc.

```go
import libtime "github.com/bborbe/time"
Expand Down
135 changes: 135 additions & 0 deletions rules/index.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,67 @@
[
{
"anchor": "agent-cmd/agent-frontmatter",
"applies_when": "any `agents/*.md` file is created.",
"doc_path": "docs/agent-command-development-guide.md",
"enforcement": "judgment",
"id": "agent-cmd/agent-frontmatter",
"level": "MUST",
"owner": "agent-auditor"
},
{
"anchor": "agent-cmd/command-frontmatter",
"applies_when": "any `commands/*.md` file is created.",
"doc_path": "docs/agent-command-development-guide.md",
"enforcement": "judgment",
"id": "agent-cmd/command-frontmatter",
"level": "MUST",
"owner": "slash-command-auditor"
},
{
"anchor": "agent-cmd/command-thin",
"applies_when": "any new `commands/*.md` file is added or substantially changed.",
"doc_path": "docs/agent-command-development-guide.md",
"enforcement": "judgment",
"id": "agent-cmd/command-thin",
"level": "MUST",
"owner": "slash-command-auditor"
},
{
"anchor": "agent-cmd/gap-driven-feedback",
"applies_when": "an agent depends on documented information that may be incomplete.",
"doc_path": "docs/agent-command-development-guide.md",
"enforcement": "judgment",
"id": "agent-cmd/gap-driven-feedback",
"level": "SHOULD",
"owner": "agent-auditor"
},
{
"anchor": "agent-cmd/no-user-prompts",
"applies_when": "any agent or command performs work that could prompt the user (writing to `/tmp/`, requesting permissions, asking confirmation) during normal execution.",
"doc_path": "docs/agent-command-development-guide.md",
"enforcement": "judgment",
"id": "agent-cmd/no-user-prompts",
"level": "MUST",
"owner": "agent-auditor"
},
{
"anchor": "agent-cmd/scripts-in-claude-dir",
"applies_when": "an agent depends on executable scripts (Python, shell) to do real work.",
"doc_path": "docs/agent-command-development-guide.md",
"enforcement": "judgment",
"id": "agent-cmd/scripts-in-claude-dir",
"level": "MUST",
"owner": "agent-auditor"
},
{
"anchor": "agent-cmd/single-source-of-truth",
"applies_when": "an agent's domain has multiple potential data sources (config files, APIs, generated artifacts, documentation) and one of them is the authoritative, human-maintained source.",
"doc_path": "docs/agent-command-development-guide.md",
"enforcement": "judgment",
"id": "agent-cmd/single-source-of-truth",
"level": "SHOULD",
"owner": "agent-auditor"
},
{
"anchor": "go-context/cancel-check-in-loop",
"applies_when": "Go `for` loop body lacks a non-blocking `select { case <-ctx.Done(): ...; default: }` check, outside `*_test.go` and `vendor/`.",
Expand DownExpand Up@@ -215,6 +278,78 @@
"level": "MUST",
"owner": "go-security-specialist"
},
{
"anchor": "go-testing/counterfeiter-mocks-required",
"applies_when": "a test file declares a hand-written struct that satisfies a production interface and is used in place of a real implementation under test, instead of importing a `mocks/<Name>` fake produced by Counterfeiter.",
"doc_path": "docs/go-testing-guide.md",
"enforcement": "judgment (presence of `//counterfeiter:generate` directive + hand-written fake detection; ast-grep follow-up)",
"id": "go-testing/counterfeiter-mocks-required",
"level": "MUST",
"owner": "go-test-quality-assistant"
},
{
"anchor": "go-testing/libtime-injection-required",
"applies_when": "a Go business-logic file (outside `main.go`, `cmd/**`, `*_test.go`, `vendor/`) reads the current time. Tests cannot control `time.Now()` directly, so dependent code is unverifiable.",
"doc_path": "docs/go-testing-guide.md",
"enforcement": "cross-rule — overlaps with `go-time/no-time-now-direct` (already in `rules/index.json`). This rule scopes the same constraint to test-coverage assessments: a service that doesn't inject time has no testable time-dependent paths.",
"id": "go-testing/libtime-injection-required",
"level": "MUST",
"owner": "go-test-quality-assistant"
},
{
"anchor": "go-testing/main-test-with-compiles",
"applies_when": "a Go binary project (package `main` with `main.go`) does not have a `main_test.go` containing a `Compiles` It-block backed by `gexec.Build`.",
"doc_path": "docs/go-testing-guide.md",
"enforcement": "judgment (file-existence + body check; ast-grep follow-up)",
"id": "go-testing/main-test-with-compiles",
"level": "MUST",
"owner": "go-test-quality-assistant"
},
{
"anchor": "go-testing/no-bare-error-call",
"applies_when": "a Go test file inside an `It` / `BeforeEach` / `JustBeforeEach` / `AfterEach` block calls an error-returning function whose return value is discarded.",
"doc_path": "docs/go-testing-guide.md",
"enforcement": "judgment (ast-grep follow-up — errcheck-equivalent scoped to Ginkgo blocks)",
"id": "go-testing/no-bare-error-call",
"level": "MUST",
"owner": "go-test-quality-assistant"
},
{
"anchor": "go-testing/no-stdlib-table-tests",
"applies_when": "a Go test file in a package that has a Ginkgo `*_suite_test.go` uses `t.Run` inside a `for _, tt := range tests` loop instead of `DescribeTable`/`Entry`.",
"doc_path": "docs/go-testing-guide.md",
"enforcement": "judgment (ast-grep follow-up)",
"id": "go-testing/no-stdlib-table-tests",
"level": "MUST",
"owner": "go-test-quality-assistant"
},
{
"anchor": "go-testing/no-testing-t-direct",
"applies_when": "a Go file in a package that has a Ginkgo `TestSuite` entry-point uses `*testing.T` directly inside test functions other than the suite entry-point itself.",
"doc_path": "docs/go-testing-guide.md",
"enforcement": "judgment (ast-grep follow-up)",
"id": "go-testing/no-testing-t-direct",
"level": "MUST",
"owner": "go-test-quality-assistant"
},
{
"anchor": "go-testing/suite-test-file-required",
"applies_when": "a Go package contains test files (`*_test.go`) but no `*_suite_test.go` file with a `TestSuite` entry-point and `RunSpecs`.",
"doc_path": "docs/go-testing-guide.md",
"enforcement": "judgment (file-existence check; ast-grep follow-up)",
"id": "go-testing/suite-test-file-required",
"level": "MUST",
"owner": "go-test-quality-assistant"
},
{
"anchor": "go-testing/suite-timeout-required",
"applies_when": "a `*_suite_test.go` file calls `GinkgoConfiguration()` without setting `suiteConfig.Timeout` before `RunSpecs`.",
"doc_path": "docs/go-testing-guide.md",
"enforcement": "judgment (ast-grep follow-up — pattern over suite body)",
"id": "go-testing/suite-timeout-required",
"level": "MUST",
"owner": "go-test-quality-assistant"
},
{
"anchor": "go-time/inject-getter-not-create",
"applies_when": "a factory or constructor file outside `main.go` calls `libtime.NewCurrentDateTime()`.",
Expand Down
Loading