mirror of
https://github.com/agent-substrate/substrate.git
synced 2026-11-10 10:13:50 +08:00
Steven Shriver
7c0c70c7a2
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
52 lines
2.3 KiB
Markdown
52 lines
2.3 KiB
Markdown
# Code Style Guide
|
|
|
|
Go conventions for this repository — written for humans and coding agents
|
|
alike. The [API style guide](api-style-guide.md) governs the proto surface;
|
|
this document governs the Go code behind it. Repository layout is covered by
|
|
[code layout](dev/code-layout.md).
|
|
|
|
The baseline is [Effective Go](https://go.dev/doc/effective_go), the
|
|
[Google Go style guide](https://google.github.io/styleguide/go/), 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:
|
|
|
|
```go
|
|
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](dev/best-practices/ci-fail-closed.md).
|
|
|
|
## TODOs
|
|
|
|
Deferred decisions are recorded in code as `TODO(<issue-number>): ...`, placed where
|
|
the decision will eventually have to be made.
|