Skip to content

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

Merged
bborbe merged 1 commit into
masterfrom
feat/bootstrap-testing
Jun 2, 2026
Merged

feat(testing): bootstrap 8 rule blocks in go-testing-guide#10
bborbe merged 1 commit into
masterfrom
feat/bootstrap-testing

Conversation

@bborbe

Copy link
Copy Markdown
Owner

Summary

Bootstraps `docs/go-testing-guide.md` — converts its 8 MUST sections into canonical `### RULE` blocks. Mirrors PRs #2-5, #8 template.

`rules/index.json`: 27 → 42 entries (8 new `go-testing/` + 7 `agent-cmd/*` that were forgotten in PR #9's commit).

Rules added

idlevelenforcement
`go-testing/no-stdlib-table-tests`MUSTjudgment
`go-testing/no-testing-t-direct`MUSTjudgment
`go-testing/no-bare-error-call`MUSTjudgment
`go-testing/suite-test-file-required`MUSTjudgment (file-existence)
`go-testing/main-test-with-compiles`MUSTjudgment (file-existence + body)
`go-testing/suite-timeout-required`MUSTjudgment
`go-testing/counterfeiter-mocks-required`MUSTjudgment
`go-testing/libtime-injection-required`MUSTjudgment (cross-refs `go-time/no-time-now-direct`)

All are mechanically enforceable in principle — ast-grep YAMLs deferred to focused follow-up PRs rather than blocking this bootstrap (same approach as PR #8 / `go-prometheus/counter-total-suffix`).

PR #9 cleanup

PR #9 trimmed the agent-cmd-guide and added 7 `agent-cmd/*` rule blocks but didn't include the `rules/index.json` walker output regen. Local `make build-index` here picked them up; included in this PR's diff. Net new for this PR specifically: 8 go-testing rules.

Pre-emptive checks (lessons from #6 + #8)

  • `grep -nEi "personal/|/users/bborbe|~/documents/obsidian"` → clean
  • Trading-term grep (`candle|epic|broker|signalstore`) → clean
  • No internal contradictions (e.g. Good examples don't violate sibling rules)
  • All cross-references (rule id ↔ index anchor ↔ rule YAML path) resolve

Test plan

Restructures the 8 MUST sections in docs/go-testing-guide.md into
canonical `### RULE` blocks. Same template as PRs #2-5, #8.
Rules added (all judgment for this PR; mechanical ast-grep follow-ups
tracked separately):
- go-testing/no-stdlib-table-tests (MUST)
Ginkgo-suite packages must use DescribeTable/Entry, not for/t.Run.
- go-testing/no-testing-t-direct (MUST)
No bare testing.T in Ginkgo-suite packages — use Describe/It.
- go-testing/no-bare-error-call (MUST)
Wrap error-returning calls in Gomega matchers (Succeed/HaveOccurred/
MatchError) — errcheck enforces this in precommit.
- go-testing/suite-test-file-required (MUST)
Every test-containing package needs *_suite_test.go or Ginkgo silently
discovers no specs.
- go-testing/main-test-with-compiles (MUST)
Every binary needs main_test.go with a Compiles It-block backed by
gexec.Build.
- go-testing/suite-timeout-required (MUST)
suiteConfig.Timeout must be set so hung tests fail fast.
- go-testing/counterfeiter-mocks-required (MUST)
No hand-written mocks — they drift silently.
- go-testing/libtime-injection-required (MUST)
Cross-references go-time/no-time-now-direct; test-quality scope.
rules/index.json: 27 → 42 entries
This also picks up 7 agent-cmd/* entries from PR #9 (#9 merged the
doc but didn't include the walker output regen — caught locally via
make build-index). Net new in this PR: 8 go-testing rules.
Pre-emptive checks (lessons from PRs #6, #8): no personal vault paths,
no trading-domain terms, no internal contradictions (rule examples
don't violate sibling rules).

@ben-s-pull-request-reviewerben-s-pull-request-reviewerBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
"verdict": "approve",
"summary": "The PR bootstraps 8 RULE blocks in docs/go-testing-guide.md and adds matching entries to rules/index.json. All four concerns from the plan were investigated: (1) the 7 agent-cmd/* entries are not duplicates — they were omitted from index.json during the PR #9 trim and are added here retroactively; (2) the cross-rule reference `go-time/no-time-now-direct` exists in index.json; (3) the new RULE blocks are structurally sound as documentation with enforcement marked as judgment/ast-grep follow-up; (4) the new entries are intentionally placed at the top of the array (agent-cmd before go-*, go-testing before go-time), consistent with logical grouping. No critical issues found.",
"comments": [
{
"file": "rules/index.json",
"line": 1,
"severity": "nit",
"message": "Should Fix: PR title says 'bootstrap 8 rule blocks in go-testing-guide' but this PR also adds 7 agent-cmd/* entries to rules/index.json (corresponding to the 7 RULE blocks added in the PR #9 trim of agent-command-development-guide.md). These agent-cmd entries were omitted from index.json in PR #9 and are added here retroactively. Consider splitting into separate commits or clarifying the title to mention both."
},
{
"file": "docs/go-testing-guide.md",
"line": 47,
"severity": "nit",
"message": "Nice to Have: All 8 new RULE blocks have enforcement marked as 'judgment (ast-grep follow-up)'. The rules are well-structured as documentation but mechanical enforcement via ast-grep YAML has not been implemented yet. This is expected for a bootstrap pass but should be tracked as follow-up work."
}
],
"concerns_addressed": [
"correctness: rules/index.json 7 agent-cmd/* entries — verified NOT duplicates from PR #9; PR #9 only touched docs/agent-command-development-guide.md, these index.json entries were never added and are correctly added now",
"correctness: go-testing/libtime-injection-required cross-rule reference 'go-time/no-time-now-direct' — verified exists in rules/index.json at line 367",
"correctness: 8 new RULE blocks reference 'ast-grep follow-up' — confirmed, rules are documentation-quality, enforcement not yet implemented (expected for bootstrap)",
"correctness: new entries inserted at top of JSON array — intentional; agent-cmd/* entries go before go-context/* (alphabetically first by domain), go-testing/* entries go before go-time/* (consistent with prior go-testing position)"
]
}

@bborbe
bborbe merged commit 00ea390 into masterJun 2, 2026
1 check passed
@bborbe
bborbe deleted the feat/bootstrap-testing branch June 2, 2026 09:12
bborbe added a commit that referenced this pull request Jun 2, 2026
PR #9 trimmed a doc that contained pre-canonicalized rule blocks but
shipped without running 'make build-index'. The 7 new agent-cmd/*
entries were silently absent from rules/index.json until PR #10 picked
them up retroactively. The lesson: walker-output drift is silent at
review time; rules/index.json passes JSON validation regardless.
Fix: add 'check-index' to the precommit chain. The target regenerates
the walker output to a tmp file, diffs against the committed index,
and fails with the diff + a 'run make build-index and commit' message
if they differ.
Behavior verified:
- Clean state (master HEAD c07b809): precommit passes
- Synthetic drift (extra line appended to rules/index.json): precommit
fails with diff output and non-zero exit code
This closes the gap that turned a one-PR change (PR #9) into a two-PR
walker-regen catch-up (PR #10). Future trim/bootstrap PRs that touch
rule blocks will fail precommit locally if the walker output isn't
also committed.
bborbe added a commit that referenced this pull request Jun 2, 2026
Restructures the 4 MUST sections in docs/go-licensing-guide.md into
canonical `### RULE` blocks. Same template as PRs #2-5, #8, #10.
Rules added:
- go-licensing/license-file-required (MUST, judgment)
Public Go repos MUST have a root LICENSE file. Private/internal repos
exempt. Detection requires file-existence check — not ast-grep.
- go-licensing/readme-license-section-required (MUST, judgment)
Public README must have a ## License H2 section pointing to LICENSE.
Markdown structure check — not ast-grep.
- go-licensing/source-file-header-required (MUST, addlicense)
All non-vendor *.go files need the 3-line BSD-2 header. Enforcement
delegates to 'addlicense -check' via make precommit (canonical tool;
already wired into the toolchain).
- go-licensing/copyright-year-discipline (MUST, judgment)
No bulk year-updates for trivial changes; no future/non-numeric
years. PR-scope diff inspection — partial ast-grep detection
possible for the future-year case but bulk-update trigger needs
diff-scope reasoning.
rules/index.json: 42 -> 46
Walker drift detected by PR #13's check-index target during first
precommit run — regenerated rules/index.json and re-ran precommit
clean. New guard working as designed.
Pre-emptive checks (lessons from #6, #8): no personal vault paths,
no trading-domain terms, no internal contradictions, all cross-refs
resolve. License-assistant agent exists at agents/license-assistant.md.
bborbe added a commit that referenced this pull request Jun 2, 2026
Restructures the godoc style guide with canonical `### RULE` blocks
for its enforceable conventions. Mirrors PRs #2-5, #8, #10, #14.
Rules added (rules/index.json: 46 -> 50):
- go-doc/exported-item-must-have-comment (MUST, judgment)
Every exported function/type/const/var needs a doc comment. Mechanical
follow-up: revive's 'exported' rule.
- go-doc/comment-starts-with-name (MUST, judgment)
Doc comment first word must match the identifier name. Mechanical
follow-up: revive (or ast-grep via the PR #11 recipe).
- go-doc/package-comment-in-doc-go (SHOULD, judgment)
Package comment belongs in doc.go to survive refactors of feature files.
- go-doc/third-person-no-signature-repeat (SHOULD, judgment)
No first-person prose; don't restate the signature.
All examples generic (Order, Customer, Discount, Add) — no trading domain.
All rule IDs unique against the 46 existing entries. godoc-assistant
agent exists at agents/godoc-assistant.md.
Pre-emptive checks (lessons from PRs #6, #8, #14):
- No personal vault paths
- No trading-domain terms
- All cross-refs resolve
- make build-index regenerated; check-index target passes
bborbe added a commit that referenced this pull request Jun 2, 2026
Restructures the 6 enforceable conventions in docs/go-architecture-patterns.md
into canonical `### RULE` blocks. Mirrors PRs #2-5, #8, #10, #14, #15.
Rules added (rules/index.json: 50 -> 56, new go-architecture/* family):
- go-architecture/counterfeiter-directive-on-interface (MUST)
Every substitutable interface needs a //counterfeiter:generate directive
so go generate regenerates the fake when the interface drifts.
- go-architecture/new-prefix-constructor-naming (MUST)
Constructors start with New (not Create/Make/etc). Partial overlap with
go-factory/no-impl-in-factory-pkg for Create*.
- go-architecture/constructor-returns-interface (MUST)
New* returns the interface, not *concrete struct. Hides implementation,
keeps dependency direction one-way.
- go-architecture/private-struct-matches-interface (SHOULD)
UserService -> userService (first letter lowered). godoc pairs them;
IDE outlines pair them; refactors find them with one rename.
- go-architecture/no-globals-or-singletons (MUST)
Service deps via constructor injection, never package-level vars.
Globals break parallel tests, hide the dep graph, make refactors fragile.
- go-architecture/business-logic-not-in-main (MUST)
main.go is wiring; domain operations live in pkg/. Mixed main.go is
untestable and unreachable from other binaries.
Cross-references to existing rules (already in index, not duplicated):
- go-errors/no-context-background-in-business-logic — covers context discipline
- go-context/cancel-check-in-loop — covers infinite-loop ctx.Done() checks
- go-time/* family — covers libtime injection
All examples generic (User, UserService, Worker, log/db deps).
No trading-domain terms (KafkaBrokers grep hit is server infrastructure,
not trading-broker terminology — pre-existing in main.go example).
No personal vault paths.
Pre-emptive checks (lessons from PRs #6, #8, #14, #15): clean grep,
unique rule IDs (6 new vs 50 existing), go-architecture-assistant
agent exists at agents/go-architecture-assistant.md, make build-index
regenerated, check-index passes.
Sign up for freeto 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

@bborbe