Files
substrate/docs/code-style-guide.md
T
Steven Shriver 7c0c70c7a2 CI: create JUnit artifacts and gate on all tests passing (#1873)
This change wires the `gotestsum` seam into the actual CI and ensures
that all tests _run_ and _pass_ using a new "gate" that verifies the
JUnit artifacts they produce. Specifically, the new gate works as
follows:

1. _registers_ that a test _should_ run and produce an artifact
2. _verifies_ that the artifact was produced and _all_ tests passed
(none were skipped).

The registration phase happens when a test is run in CI and requires
that the test be provided a `E2E_JUNIT_FILE` env variable.

Partial work for #1871

- [x] Tests pass
   ```
  Run make build-junittool
  make build-junittool
  bin/junittool verify -manifest "${ARTIFACTS}/expected-junit.txt"
  shell: /usr/bin/bash -e {0}
  env:
    E2E_ATENET_DATAPLANE: agentgateway
    ARTIFACTS: /home/runner/work/substrate/substrate/_artifacts
go -C tools/junittool build -o
/home/runner/work/substrate/substrate/bin//junittool .
FILE TESTS FAILURES ERRORS SKIPPED
/home/runner/work/substrate/substrate/_artifacts/e2e-gvisor.xml 84 0 0 4
/home/runner/work/substrate/substrate/_artifacts/e2e-microvm.xml 84 0 0
5
/home/runner/work/substrate/substrate/_artifacts/e2e-mitm.xml 1 0 0 0
/home/runner/work/substrate/substrate/_artifacts/e2e-mitm-microvm.xml 1
0 0 0

/home/runner/work/substrate/substrate/_artifacts/e2e-networking-mitm.xml
11 0 0 1

/home/runner/work/substrate/substrate/_artifacts/e2e-networking-mitm-microvm.xml
11 0 0 1
  TOTAL   
  ```
- [x] Appropriate changes to documentation are included in the PR
2026-09-28 22:01:26 +00:00

2.3 KiB

Code Style Guide

Go conventions for this repository — written for humans and coding agents alike. The API style guide governs the proto surface; this document governs the Go code behind it. Repository layout is covered by code layout.

The baseline is Effective Go, the Google Go style guide, and gofmt. This document records only decisions that are specific to this project.

Proto field access: check presence, don't default it

Generated getters (e.g. req.GetActor().GetName()) return zero values when any message in the chain is nil. That is convenient and dangerous: a required message the caller forgot to set silently becomes "", and the bug surfaces far from its cause.

  • A getter chain must never stand in for a presence check. Whenever a field is required at this point in the code, check it explicitly and fail loudly: INVALID_ARGUMENT at the API boundary, a real error (not a guessed zero value) elsewhere.

  • Once the boundary has validated presence, getters downstream are fine, as is the guarded form for optional messages:

    if wass := worker.Assignment; wass != nil {
        // Fields of wass were validated on write; use them directly.
    }
    
  • Absence of a group of fields that are set and cleared together is represented by a nil message (worker.Assignment == nil), never by probing a scalar field inside it for its zero value.

Testing

  • Standard library testing only — no assertion or mocking frameworks.
  • Table-driven tests with t.Run subtests are the default shape.
  • Prefer a real test implementation when one exists: the PostgreSQL test fixture for the store, envtest for the Kubernetes API. Release resources with t.Cleanup.
  • A test that skips on a missing precondition must fail on it in CI instead — a skipped test and a passing one are the same exit code. Resolve strictness through a named predicate (dockerenv.Required()), and prove the strict branch red before merging. See CI fail-closed preconditions.

TODOs

Deferred decisions are recorded in code as TODO(<issue-number>): ..., placed where the decision will eventually have to be made.