Files
meshcore-analyzer/cmd/server/perf_io_carmack_test.go
T
40f664c587 chore(#1859): gofmt sweep + gofmt/go vet CI gate (rebase of #1881) (#1941)
Rebase of #1881 by @SaarMesh-Bot onto current master. Their three
commits are preserved, two of them cherry-picked with authorship intact;
the sweep itself had to be regenerated. Opened as a new PR rather than
force-pushing their branch.

Closes #1881 once merged. Addresses parts 1 and 3 of #1859; part 2
landed as #1937.

## Why regenerated rather than merged

The sweep in #1881 was cut on 2026-09-02 07:13 and roughly forty PRs
landed after it, so it went `CONFLICTING/DIRTY`. Re-running `gofmt` on
current master is cheaper and less error-prone than resolving 72
conflicts that are all whitespace. The drift it fixes also grew in the
meantime: 66 files now, against 72 then, but spread differently.

## The three commits

1. **`style(#1859)`** — `gofmt -w` across the 14 modules. 66 files.
2. **`test(#1859)`** — @SaarMesh-Bot's fix for the one `go vet`
copylocks finding, `cmd/ingestor/coverage_boost_test.go`: the range
variable copied a `Config` embedding `sync.Once`. Cherry-picked
unchanged.
3. **`ci(#1859)`** — @SaarMesh-Bot's CI step that fails on gofmt drift
or vet findings, plus `.git-blame-ignore-revs`. Cherry-picked with one
change, noted in the commit message: the ignore file pointed at
`04bc80ee`, the sweep commit on their branch, which does not exist on
this base and would make `git blame --ignore-revs-file` error. Repointed
at `d3a02599`, the sweep here.

## Verification

The claim "formatting only" is checked twice rather than asserted:

- Every changed file is byte-identical to `gofmt(previous content)`. 0
of 66 deviate.
- With line comments and all whitespace stripped, 0 of 66 files differ,
so no code outside comments changed.

14 of the 66 also show doc-comment reflow. Since Go 1.19 `gofmt`
re-indents indented comment blocks to tabs and inserts a blank comment
line before them; the behavior matrix above `resolveHopWithContext` in
`cmd/ingestor/path_resolver.go` is a clear example. That is gofmt's own
output, not an edit, but it is worth naming because it makes the diff
look larger than "whitespace" suggests.

The gate was run locally exactly as the workflow runs it: `gofmt` clean,
and `go vet` clean in all 14 modules, including `cmd/ingestor` which is
what commit 2 fixes.

Suites: `cmd/server` ok (80.7s), `internal/packetpath` ok (2.3s),
`cmd/ingestor` passes except
`TestWriteStatsAtomic_SymlinkAtDestIsReplaced`, which fails identically
on bare master with "A required privilege is not held by the client"
(Windows symlink privilege on my host, not code).

## Sequencing

This should go last in the queue. The sweep touches 66 files, so merging
it before the remaining open Go PRs gives each of them a conflict about
nothing but formatting. After it lands the gate is active, and any PR
with drift fails CI until it runs `gofmt -w`.

Excluded from the sweep: the misnamed `Dockerfile.go`, which is a
Dockerfile that gofmt cannot parse (the workflow excludes it too), and
`docs/DEPLOYMENT.md`, which a case-insensitive filesystem surfaces as a
spurious modification against `docs/deployment.md` and is unrelated.

---------

Co-authored-by: SaarMesh-Bot <300107934+SaarMesh-Bot@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-03 18:52:03 +02:00

141 lines
5.3 KiB
Go

package main
import (
"bufio"
"os"
"path/filepath"
"strings"
"testing"
"time"
)
// TestParseProcIO_EmptyDoesNotMarkOK — #1167 Carmack must-fix #6: the
// server-side parser was missing the parsedAny gate the ingestor's parser
// got in must-fix #3 of the original review. Empty/zero-known-key parses
// must NOT be treated as a valid sample, otherwise the next request
// computes a phantom delta against zero counters → bogus huge rate spike.
//
// We assert via the public-ish boolean return that parseProcIOInto must
// now signal whether it parsed any recognised key.
func TestParseProcIO_EmptyDoesNotMarkOK(t *testing.T) {
var s procIOSample
ok := parseProcIOInto(bufio.NewScanner(strings.NewReader("")), &s)
if ok {
t.Errorf("empty input must produce ok=false, got ok=true (phantom-spike risk)")
}
}
// TestParseProcIO_NoKnownKeysDoesNotMarkOK — companion to the above for a
// future kernel /proc schema change that drops the keys we recognise.
func TestParseProcIO_NoKnownKeysDoesNotMarkOK(t *testing.T) {
var s procIOSample
ok := parseProcIOInto(bufio.NewScanner(strings.NewReader("garbage_key: 42\nother: 99\n")), &s)
if ok {
t.Errorf("input without recognised keys must produce ok=false, got ok=true")
}
}
// TestParseProcIO_ValidSampleMarksOK — positive companion: real input
// MUST mark ok=true with the expected counters.
func TestParseProcIO_ValidSampleMarksOK(t *testing.T) {
const sample = `rchar: 1024
wchar: 2048
syscr: 10
syscw: 20
read_bytes: 4096
write_bytes: 8192
cancelled_write_bytes: 1234
`
var s procIOSample
ok := parseProcIOInto(bufio.NewScanner(strings.NewReader(sample)), &s)
if !ok {
t.Fatalf("valid sample must produce ok=true")
}
if s.readBytes != 4096 || s.writeBytes != 8192 || s.cancelledWrite != 1234 {
t.Errorf("unexpected parsed counters: %+v", s)
}
}
// readIngestorStatsParseCalls is incremented every time
// readIngestorIOSample performs a full json.Unmarshal of the stats file
// (i.e. cache miss). Used by the cache test below to assert that
// repeated calls within the same mtime+size window do NOT re-decode.
//
// The hook must be wired up in perf_io.go (Carmack must-fix #2).
//var readIngestorStatsParseCalls atomic.Int64 — defined in perf_io.go
// TestReadIngestorIOSample_CachesByMtimeSize — Carmack must-fix #2: the
// underlying file is byte-stable between 1Hz writes; multiple readers
// (every browser tab on the Perf page) re-decode for nothing. Cache the
// last decoded sample keyed by (mtime, size); only re-parse when either
// changes.
func TestReadIngestorIOSample_CachesByMtimeSize(t *testing.T) {
dir := t.TempDir()
statsPath := filepath.Join(dir, "ingestor-stats.json")
freshAt := time.Now().UTC().Format(time.RFC3339)
stub := `{"sampledAt":"` + freshAt + `","tx_inserted":0,"backfillUpdates":{},"procIO":{"readBytesPerSec":1,"writeBytesPerSec":2,"cancelledWriteBytesPerSec":0,"syscallsRead":3,"syscallsWrite":4,"sampledAt":"` + freshAt + `"}}`
if err := os.WriteFile(statsPath, []byte(stub), 0o600); err != nil {
t.Fatal(err)
}
t.Setenv("CORESCOPE_INGESTOR_STATS", statsPath)
// Reset counter + cache.
readIngestorStatsParseCalls.Store(0)
resetIngestorIOCache()
for i := 0; i < 5; i++ {
got := readIngestorIOSample()
if got == nil {
t.Fatalf("call %d: expected non-nil, got nil", i)
}
}
got := readIngestorStatsParseCalls.Load()
if got != 1 {
t.Errorf("expected 1 parse for 5 reads of byte-stable file, got %d", got)
}
}
// TestReadIngestorIOSample_CacheInvalidatesOnMtimeChange — companion: as
// soon as the file changes (writer tick) the cache MUST invalidate.
func TestReadIngestorIOSample_CacheInvalidatesOnMtimeChange(t *testing.T) {
dir := t.TempDir()
statsPath := filepath.Join(dir, "ingestor-stats.json")
write := func() {
freshAt := time.Now().UTC().Format(time.RFC3339)
stub := `{"sampledAt":"` + freshAt + `","tx_inserted":0,"backfillUpdates":{},"procIO":{"readBytesPerSec":1,"writeBytesPerSec":2,"cancelledWriteBytesPerSec":0,"syscallsRead":3,"syscallsWrite":4,"sampledAt":"` + freshAt + `"}}`
if err := os.WriteFile(statsPath, []byte(stub), 0o600); err != nil {
t.Fatal(err)
}
}
write()
t.Setenv("CORESCOPE_INGESTOR_STATS", statsPath)
readIngestorStatsParseCalls.Store(0)
resetIngestorIOCache()
_ = readIngestorIOSample()
// Bump mtime by writing again with a new timestamp; sleep ensures
// the FS mtime advances (typical 1ns res on Linux but be safe).
time.Sleep(10 * time.Millisecond)
// Touch with a different size by rewriting fresh content.
write()
// Force a clearly different mtime by setting it explicitly.
future := time.Now().Add(2 * time.Second)
if err := os.Chtimes(statsPath, future, future); err != nil {
t.Fatal(err)
}
_ = readIngestorIOSample()
got := readIngestorStatsParseCalls.Load()
if got != 2 {
t.Errorf("expected 2 parses across an mtime-change, got %d", got)
}
}
// TestPerfIOEndpoint_IngestorTimestampMatchesSnapshot was removed: it
// was a hand-flipped-bool tautology. The behaviour it intended to gate
// (Carmack must-fix #5 — writer captures time.Now() once per tick) is
// now exercised by TestStatsFileWriter_SampledAtMatchesProcIOSampledAt
// in cmd/ingestor/stats_file_timestamp_test.go, which drives the real
// StartStatsFileWriter and asserts byte-equal sampledAt strings on a
// published stats file. Removed per Kent Beck Gate review
// pullrequestreview-4254521304.