diff --git a/.build.yml b/.build.yml index b6bb7aff8cfd1c1d4bd73570829eefeb0de14e03..658f47b9af80a9b67592f81b9a3681c7ffc4de51 100644 --- a/.build.yml +++ b/.build.yml @@ -1,6 +1,7 @@ # builds.sr.ht manifest for diff.sr.ht. One linear pipeline: install the # cache helper, assemble the shared SCSS, restore caches, package with abuild, -# publish the apk, save the caches. +# publish the apk, save the caches, and upload this build's own coverage and +# benchmarks to cov.sr.ht and bench.sr.ht. # # The reasoning behind every task lives in docs/ci.md, not here: builds.sr.ht # stores the submitted manifest in a varchar(16384), so a manifest over 16 KiB @@ -24,8 +25,10 @@ # S3 credentials for the cacher CI cache (Garage `docker-cache` bucket), # same pair the bencher and ci-cacher builds use. - 7dde4219-0783-4581-a67d-c94749de3600 # ~/.s3-cache-key-id - 0e5b3530-6f19-4f30-9b73-9339dd382e46 # ~/.s3-cache-key-secret - # A tokens.sr.ht working token carrying artifacts:upload, the same secret the - # sibling services mount. It is what publish_artifacts sends. + # A tokens.sr.ht working token, the same secret the sibling services mount. + # It must carry artifacts:upload (publish_artifacts), cov:upload (coverage) + # and bench:upload (bench); missing one fails that upload and no other. + # See docs/ci.md#secrets. - c7968415-1a6d-4ca0-a188-150fb7f57b65 # ~/.srht-token sources: - https://git.srht.bigb.es/~bigbes/sr-ht-compare @@ -44,6 +47,18 @@ # CORE_VER must track the deployment's SRHT_CORE_VER; BOOTSTRAP_REV is the # submodule commit core.sr.ht pins at that tag. See docs/ci.md#environment. CORE_VER: "0.84.5" BOOTSTRAP_REV: 779ad9f174ea5ab7e755f6df0ec9e5912d67dd16 + # Where this build reports on itself. Both repository names are the one on the + # `sources:` line above and not the service's: the daemon was renamed to + # diff.sr.ht, the git repository was not. docs/ci.md#coverage, #bench. + COVER_ORIGIN: https://cov.srht.bigb.es + COVER_REPO: "~bigbes/sr-ht-compare" + BENCH_ORIGIN: https://bench.srht.bigb.es + BENCH_REPO: "~bigbes/sr-ht-compare" +# Literal paths relative to $HOME; `artifacts:` has no globbing, which is why +# the apk is not here. See docs/ci.md#artifacts. +artifacts: + - cover.out + - bench.txt submitter: git.sr.ht: allow-refs: @@ -151,13 +166,13 @@ # the log, where they are the whole diagnosis. See docs/ci.md#test. gofmt -l . | tee /tmp/fmt test ! -s /tmp/fmt || { echo "gofmt would change the files above" >&2; exit 1; } go vet ./... - # `make test` and not a bare `go test ./...`: the test command lives in - # one place. There is no DSN guard and no service to reach — every suite - # here is hermetic (gitx builds a bare repo with the local git, web is - # httptest + ecoretest) — so nothing can skip, and a failure is a non-zero - # exit that fails this task. That is what the APKBUILD's `!check` now - # rests on. docs/ci.md#test. - make test + # `make cover` and not a bare `go test ./...`: the test command lives in + # one place, and `cover` is `test` with two flags in it. There is no DSN + # guard and no service to reach — every suite here is hermetic — so + # nothing can skip, and a failure fails this task. That is what the + # APKBUILD's `!check` rests on. The profile goes to $HOME, not into the + # checkout, which an untracked file would stamp "+dirty". docs/ci.md#test. + make cover COVERPROFILE="$HOME/cover.out" - build: | cd "$REPO" # -d: makedepends come from `packages:` above, so skip abuild's own @@ -241,3 +256,61 @@ # purpose. Without --force an upload skips a key already there, so no # `cacher exists ||` guard is needed. See docs/ci.md#cache_save. cacher dir upload "$KEY_MOD" ~/go/pkg/mod cacher dir upload "$KEY_GOC" ~/.cache/go-build + - coverage: | + # This build's own coverage, to the instance's cov.sr.ht, and nothing + # after it depends on it. docs/ci.md#coverage. + cd "$REPO" + if [ ! -r ~/.srht-token ]; then + echo "no ~/.srht-token: nothing uploaded; the profile is this build's" + echo "cover.out artifact" + exit 0 + fi + # GIT_REF is absent on a manual submission and ref is optional; key is the + # idempotency key. BOTH prefixes are stripped — tags build too. + ref="${GIT_REF#refs/heads/}" + ref="${ref#refs/tags/}" + url="$COVER_ORIGIN/api/v1/repos/$COVER_REPO/reports" + url="$url?commit=$(git rev-parse HEAD)&ref=$ref&key=$JOB_ID&job_url=$JOB_URL" + echo "uploading cover.out to $url" + # set +x so the header never reaches the log; no Content-Type (the service + # sniffs); --fail-with-body prints the JSON error AND exits non-zero. + # docs/ci.md#the-request. + set +x + curl -sS --fail-with-body -X POST \ + -H "Authorization: Bearer $(cat ~/.srht-token)" \ + --data-binary "@$HOME/cover.out" \ + "$url" + echo + - bench: | + # This build's own benchmarks — the diffing path a request is spent in — + # to bench.sr.ht. A VM this small measures a shape, not a number. + # docs/ci.md#bench. + cd "$REPO" + # -s keeps the recipe out of the body; a redirect and a cat and NOT + # `| tee`, which would hand the task tee's exit status and let a failing + # benchmark pass. $HOME is where artifacts look. + make -s bench > "$HOME/bench.txt" + cat "$HOME/bench.txt" + # `go test -bench` that matches NOTHING prints "ok" and exits 0, and an + # empty file is valid benchfmt: without these a renamed benchmark uploads + # nothing and reports success. Nothing here can skip. docs/ci.md#the-greps. + grep -q '^BenchmarkDiff/' "$HOME/bench.txt" + grep -q '^BenchmarkCommitPatch' "$HOME/bench.txt" + grep -q '^BenchmarkCutPatch/' "$HOME/bench.txt" + if [ ! -r ~/.srht-token ]; then + echo "no ~/.srht-token: the benchmarks ran and are above; nothing was" + echo "uploaded. The file is this build's bench.txt artifact." + exit 0 + fi + # visibility acts only on the POST that creates $BENCH_REPO. + ref="${GIT_REF#refs/heads/}"; ref="${ref#refs/tags/}" + url="$BENCH_ORIGIN/api/v1/repos/$BENCH_REPO/runs" + url="$url?commit=$(git rev-parse HEAD)&ref=$ref&key=$JOB_ID&job_url=$JOB_URL" + url="$url&visibility=public" + echo "uploading bench.txt to $url" + set +x + curl -sS --fail-with-body -X POST \ + -H "Authorization: Bearer $(cat ~/.srht-token)" \ + --data-binary "@$HOME/bench.txt" \ + "$url" + echo diff --git a/.gitignore b/.gitignore index f6c3741c94a4f345f7d0e74f44998101ed139ec2..8b543416626460cb9fec5a588129505d4b6d5bfd 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,13 @@ # Transient build artifacts *.tmp +# What `make cover` and `make bench` write when they are run with the defaults, +# i.e. from a laptop. CI puts both in $HOME instead, outside the checkout — for +# the reason the paths above are listed: an untracked file here is what makes +# the next `go build` stamp its binary "+dirty". +/cover.out +/bench.txt + # Local instance config /config.ini diff --git a/Makefile b/Makefile index a02d750410bc6a1177a61f24839bf6847f2b4d59..590d80a9a87434525aefdd7fb7b355eaa54cd3ed 100644 --- a/Makefile +++ b/Makefile @@ -23,6 +23,32 @@ # `css`, which removes the previous build's file, and `check-css`, which counts # what is left, cannot drift apart. CSS=web/static/main.min.*.css +# TESTFLAGS is the hole `cover` reaches through, so that the suite has exactly +# one spelling: `cover` is `test` with the profile flags in it and nothing else +# changed. Do not add a second `go test ./...` line to this file. +TESTFLAGS?= + +# Where `cover` writes the profile. CI passes an absolute path OUTSIDE the +# checkout ($HOME/cover.out): a profile written into the working tree would be +# an untracked file, and Go reads vcs.modified from `git status --porcelain`, +# which counts those — the packaged binary would stamp itself "+dirty". +# The default is gitignored for the same reason. +COVERPROFILE?=cover.out + +# BENCH_COUNT is `go test -count` for `bench`, and ten is the family's number: +# bench.sr.ht's confidence interval for a point becomes finite at six +# repetitions and a comparison becomes significant at four, so a -count under +# six uploads points the service can only mark "low n". A laptop that only wants +# to know the benchmarks still run says `make bench BENCH_COUNT=1`. +BENCH_COUNT?=10 + +# `bench` names its own timeout because it is the one target that can outrun +# go test's 10m default: ten counts of every benchmark, each of which builds a +# real git repository first, on a builds.sr.ht VM that is some multiple slower +# than a laptop. The failure would be `panic: test timed out` over a goroutine +# dump, which reads like a hang in the code rather than an unlucky number here. +BENCH_TIMEOUT?=20m + all: build # Compile the service into ./comparesrht. @@ -30,7 +56,26 @@ build: go build -o $(BIN) ./cmd/$(BIN) test: - go test ./... + go test $(TESTFLAGS) ./... + +# The same suite, plus the coverage profile .build.yml uploads to cov.sr.ht. +# +# It goes through `test` rather than repeating the command, so that CI's run and +# a developer's `make test` cannot become two different suites — the only +# difference between them is the two flags below. -covermode=atomic because the +# service is a concurrent HTTP server and the default `set` mode records "this +# statement ran" rather than how often; cov.sr.ht stores the counts. +cover: + @$(MAKE) test TESTFLAGS="-covermode=atomic -coverprofile=$(COVERPROFILE)" + go tool cover -func=$(COVERPROFILE) | tail -1 + +# The benchmarks of the diffing path, as benchfmt on stdout — the format +# .build.yml uploads to bench.srht.bigb.es. -run='^$$' because this is a run +# whose point is the benchmarks and the suite has already run in `test`; +# -benchmem because B/op and allocs/op are half of what a diff benchmark means +# and they cost nothing to collect. +bench: + go test -run='^$$' -bench=. -benchmem -count=$(BENCH_COUNT) -timeout $(BENCH_TIMEOUT) ./... # CSS pipeline: sassc -> minify -> content-hashed filename. The running # service globs web/static/main.min.*.css at startup, so the hash in the name @@ -152,4 +197,4 @@ install -Dm755 $(BIN) $(DESTDIR)$(BINDIR)/$(BIN) mkdir -p $(DESTDIR)$(STATICDIR) install -Dm644 -t $(DESTDIR)$(STATICDIR) web/static/* -.PHONY: all build test css check-css bundle run-dev install install-files +.PHONY: all build test cover bench css check-css bundle run-dev install install-files diff --git a/README.md b/README.md index b2f3899055e6adc87a1ef8e6c542171cb185274b..94ed2b455f24caee807dbe71f621f3edd33b3f32 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,8 @@ ```sh make build # compile ./comparesrht make test # go test ./... +make cover # the same suite plus ./cover.out (what CI uploads to cov.sr.ht) +make bench # the diffing benchmarks as benchfmt (what CI uploads to bench.sr.ht) make css # build the hashed stylesheet (sassc + minify) make bundle # rebuild the vendored frontend bundle (esbuild) ``` diff --git a/docs/ci.md b/docs/ci.md index 5657c69859a9771db005c947fce14e263d0e320a..d9bbf074397fd4fbeb0c86fc7933063311504c0b 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -6,10 +6,15 @@ submitted at all — the failure is at submission time and reads like nothing in particular: the branch simply has no CI. Rationale therefore lives here, and the manifest carries pointers. +That headroom is nearly spent: the manifest is ~15.8 KiB of the 16 KiB a +submission may carry, which is about eight comment lines. A paragraph added +there is a paragraph that has to come out again — write it here instead. + The pipeline is one linear job on `alpine/edge`: install the cache helper, assemble the shared SCSS, make a throwaway signing key, decide the version, restore the Go caches, test, package with `abuild`, publish the apk to -`repo.bigb.es/alpine/v3.22/bigbes`, save the caches. +`repo.bigb.es/alpine/v3.22/bigbes`, save the caches, and report on itself to +cov.sr.ht and bench.sr.ht. It is triggered by a push to the **sourcehut** side. A push to sourcecraft cannot reach builds.sr.ht; the gitsync mirror is what puts the commit on @@ -40,7 +45,7 @@ |---|---|---| | `apk-ci-s3` | `~/.apk-ci.env` | `publish` | | `7dde4219-…` | `~/.s3-cache-key-id` | `cacher_init` | | `0e5b3530-…` | `~/.s3-cache-key-secret` | `cacher_init` | -| `c7968415-…` | `~/.srht-token` | `publish_artifacts` | +| `c7968415-…` | `~/.srht-token` | `publish_artifacts`, `coverage`, `bench` | They are file secrets. Listing them is what turns `publish` and the cache tasks on; a manual submission that asks for no secrets still runs the interesting part @@ -48,6 +53,16 @@ of the pipeline and stops at `cacher_init`, which is the right place to notice. `apk-ci-s3` is referenced by name and the other two by UUID, which is only because that is how they were written in the donor manifests; both forms work. + +`~/.srht-token` is a tokens.sr.ht **working token**, minted once for a person and +carrying the grants of every service its builds reach: `artifacts:upload` for +`publish_artifacts`, `cov:upload` for `coverage`, `bench:upload` for `bench`. It +is one secret shared with the sibling pipelines rather than three per-service +ones — that is what centralising issuance buys. Grants are compared **literally**, +so a token minted before cover.sr.ht was renamed carries `cover:upload` and +uploads nothing to cov; re-mint it and replace the secret. A token missing one +grant fails that upload and no other, because each of the three tasks presents +it independently. ## environment @@ -238,14 +253,40 @@ `gofmt -l` exits 0 whether or not it printed anything, so the only way to fail on its output is to look at the output. The `tee` keeps the offending filenames in the build log, where they are the entire diagnosis. -`go vet ./...` next, then `make test` — the Makefile, not a bare `go test ./...`, -so that the test command lives in one place. +`go vet ./...` next, then `make cover COVERPROFILE="$HOME/cover.out"` — the +Makefile, not a bare `go test ./...`, so that the test command lives in one +place. + +`cover` is not a second suite. It is `test` with two flags reached through +`TESTFLAGS`, so CI's run and a developer's `make test` cannot become two +different commands: + +```make +test: + go test $(TESTFLAGS) ./... + +cover: + @$(MAKE) test TESTFLAGS="-covermode=atomic -coverprofile=$(COVERPROFILE)" + go tool cover -func=$(COVERPROFILE) | tail -1 +``` + +`-covermode=atomic` because this is a concurrent HTTP server: the default `set` +mode records that a statement ran and not how often, and cov.sr.ht stores the +counts. + +The profile is written to **`$HOME`, outside the checkout**. A profile written +into the working tree would be an untracked file, and Go reads `vcs.modified` +from `git status --porcelain`, which counts untracked files — the apk built two +tasks later would stamp itself `+dirty`. The `COVERPROFILE` default (`cover.out` +in the repo root, for a local run) is gitignored for the same reason. No Postgres, no network and **no guard**. Every suite in this tree is hermetic: `gitx/fixture_test.go` builds a bare repository with the local `git` binary, and `web/web_test.go` is `httptest` plus `ecoretest`. Nothing here is gated on an environment variable, so nothing can skip itself into a green build, and the -only thing this task owes is a non-zero exit when `make test` fails. +only thing this task owes is a non-zero exit when the suite fails. That is also +what makes the uploaded coverage honest: there is no configuration under which +a package silently contributes zero. That last point is load-bearing for the APKBUILD. `options="!check"` used to say the tests could not run in the VM — a claim that was never true — and now says @@ -359,6 +400,120 @@ dead code: without `--force`, `cacher dir upload` already skips a key that is present, so the guard asked the same question twice and its only real effect was to make a failure of the `exists` call look like a decision not to upload. +## artifacts + +Two, both literal paths relative to `$HOME`: `cover.out` and `bench.txt`. + +`artifacts:` has no globbing, which is why the apk is still not listed — its +name carries the version and changes every commit. These two do not. + +They are declared **because** the two uploads below can fail. An upload that +404s or 401s leaves the build red and the file downloadable, so the report can +be POSTed by hand instead of re-running a fifteen-minute pipeline for it. That +is also the whole content of the "no token" branch in each task: the work was +done, the result is attached, nothing was sent. + +## coverage + +The profile `test` produced, POSTed to the instance's cov.sr.ht — this service +reporting on itself, to a service it does not otherwise talk to. + +``` +POST $COVER_ORIGIN/api/v1/repos/$COVER_REPO/reports + ?commit=&ref=&key=$JOB_ID&job_url=$JOB_URL +``` + +`COVER_REPO` is `~bigbes/sr-ht-compare`, taken from the `sources:` line and not +from the service name: the daemon was renamed to diff.sr.ht, the git repository +was not, and it is the repository a report is filed under. + +Near-last in the pipeline and nothing after it depends on it. A cov.sr.ht outage +must not be able to lose an apk that was already built, tested and published. + +## bench + +`make -s bench` into `$HOME/bench.txt`, then the same shape of POST: + +``` +POST $BENCH_ORIGIN/api/v1/repos/$BENCH_REPO/runs + ?commit=&ref=&key=$JOB_ID&job_url=$JOB_URL&visibility=public +``` + +`visibility` acts only on the POST that *creates* `$BENCH_REPO`; it is ignored +afterwards. + +`-s` keeps make from echoing the recipe into the body, which is not decoration: +the body is parsed as benchfmt and a `go test …` line at the top of it is not. + +A **redirect and a `cat`**, never `| tee`. A pipeline takes the exit status of +its last command, so `make bench | tee bench.txt` would hand this task `tee`'s +status and let a failing benchmark upload its own wreckage as a green run. + +### What is measured + +`gitx`, the diffing path, which is what a request to this service is spent in: +`BenchmarkDiff` (both the two-dot and three-dot grammars a compare URL can +carry), `BenchmarkDiffStat`, `BenchmarkCommitPatch`, and the two pure functions +underneath them, `BenchmarkMapFilePatches` and `BenchmarkCutPatch`. + +They build their own bare repository with the local `git` — 40 files of 120 +lines, a branch that diverged, and a head commit that edits, adds, deletes and +renames — because the *test* fixture's whole history is a dozen lines, over +which a benchmark measures opening a repository rather than diffing one. + +`BENCH_COUNT` is 10, the family's number: bench.sr.ht's confidence interval for +a point becomes finite at six repetitions and a comparison becomes significant +at four, so a lower count uploads points it can only mark "low n". + +`bench` is the one target that names its own `-timeout` (`BENCH_TIMEOUT`, 20m). +Ten counts of every benchmark, each preceded by building a real git repository, +on a VM some multiple slower than a laptop, can outrun `go test`'s 10m default — +and what that looks like is `panic: test timed out` over a goroutine dump, which +reads like a hang in the service rather than an unlucky number in a Makefile. + +### The greps + +```sh +grep -q '^BenchmarkDiff/' "$HOME/bench.txt" +grep -q '^BenchmarkCommitPatch' "$HOME/bench.txt" +grep -q '^BenchmarkCutPatch/' "$HOME/bench.txt" +``` + +These are not belt-and-braces. **`go test -bench` that matches nothing prints +`ok` and exits 0**, and an empty file is valid benchfmt — so a renamed, moved or +deleted benchmark would upload nothing at all and the build would go green over +it. The three names span the three shapes in the file (a sub-benchmark, a +top-level one, and the pure-function pair), so a package that stopped compiling +its benchmarks cannot slip through. + +Nothing here is gated on an environment variable, so unlike the sibling services +there is no DSN guard to write: no benchmark in this tree can skip itself, and a +short file therefore means something went wrong rather than something was +unavailable. + +### The request + +Both uploads share the same three rails, and each of them is here because it +failed once somewhere in this family: + +- **`set +x` immediately before the `curl`.** builds.sr.ht traces task bodies, + and `-H "Authorization: Bearer $(cat ~/.srht-token)"` traced is a working + token in a build log anyone can read. +- **`curl -sS --fail-with-body`.** `--fail` alone throws the body away, and the + body is the service's JSON explanation; `--fail-with-body` prints it *and* + still exits non-zero. +- **Both ref prefixes stripped**, `refs/heads/` and `refs/tags/`. This pipeline + builds tags too, and a tag build would otherwise file its report under a ref + literally named `refs/tags/v0.3.0`. + +`GIT_REF` is absent on a manually submitted build; `ref` is optional for both +APIs, so it goes up empty. `key=$JOB_ID` is the idempotency key: re-running a +job replaces its report rather than adding a second one. + +The `[ ! -r ~/.srht-token ]` guard is **not** a fallback for a failed upload. +With the file present the upload is fatal on purpose. It covers exactly one +case — a submission that asked for no secrets — and it says so in the log. + ## css The stylesheet is compiled from `scss/main.scss` against the shared partials and @@ -437,12 +592,12 @@ parallel — the copying would start beside the build it is supposed to follow. ## What is not here -- **No coverage upload.** The sibling services POST their profile to cover.sr.ht - at the end of the pipeline. Adding it here needs a token secret and a - repository on cover. -- **No artifacts.** There is nothing to download: the apk goes to S3, and its - name changes every commit, which `artifacts:` cannot express (it has no - globbing). +- **No Postgres.** This service has no database and its suites are hermetic by + design; there is nothing for a `postgres` task to do, and adding one would + buy an idle daemon in the VM. +- **No apk in `artifacts:`.** Its name carries the version and changes every + commit, which `artifacts:` cannot express — it has no globbing. The apk's two + destinations are S3 and artifacts.sr.ht. - **No matrix.** One architecture, one image. ## Known wart diff --git a/gitx/bench_test.go b/gitx/bench_test.go new file mode 100644 index 0000000000000000000000000000000000000000..86ddddaaa389bd8b918f305d565fcb62a3c0b9d5 --- /dev/null +++ b/gitx/bench_test.go @@ -0,0 +1,293 @@ +package gitx + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/go-git/go-git/v5/plumbing/format/diff" + + "sourcecraft.dev/bigbes/sr-ht-compare/core" +) + +// The benchmarks of the diffing path — what this service spends a request in. +// Every page it serves is one of these calls plus a template render: Diff and +// DiffStat behind /~owner/repo/compare/base..head, CommitPatch behind +// /~owner/repo/commit/rev, mapFilePatches and cutPatch inside all three. +// +// They are uploaded to bench.srht.bigb.es by the `bench` task of .build.yml, so +// their names are part of that pipeline: the task greps for them before it +// POSTs, because `go test -bench` that matches nothing still prints "ok" and +// exits 0 (docs/ci.md#bench). +// +// A builds.sr.ht VM measures a shape rather than a number — it is a shared +// virtual machine with no CPU pinning — so what is worth reading off the series +// is a step, not a millisecond. + +const ( + // benchFiles and benchLines size the fixture repository. They are what makes + // this a diff benchmark rather than a go-git-open benchmark: the existing + // test fixture's whole history is a dozen lines, which the render finishes + // before the tree walk has paid for itself. + // + // The pair produces a ~30 000-line base tree and a head commit touching a + // quarter of it, which lands a two-dot patch in the tens of kilobytes — the + // size of an ordinary change under review, and small enough that a run at + // the family's -count=10 stays inside a CI VM's patience. + benchFiles = 40 + benchLines = 120 + + // benchModified is how many of the files the feature branch edits. + benchModified = 12 +) + +// benchRepo is the fixture the diff benchmarks measure against: +// +// seed ── main-work (branch: main) +// \ +// feature-work (branch: feature) +// +// seed : benchFiles files of benchLines lines each +// main-work : edits 4 of them, so main and feature have really diverged +// feature-work: edits benchModified, adds 3, deletes 2, renames 1 +// +// The divergence is the point: with main advanced past the fork, "main..feature" +// and "main...feature" are different comparisons, and the three-dot form (the +// one a review page uses) has a merge base to find. +type benchRepo struct { + root string // reposRoot, the directory Open takes + main string // SHA of main + head string // SHA of feature +} + +// benchLineFile renders one fixture file. The seed and the edit differ in a +// handful of lines out of benchLines, so the hunks are hunks and not whole-file +// rewrites — a rewrite would measure go-git's line differ on its easiest input. +func benchLineFile(name string, edited bool) []byte { + var b strings.Builder + for i := range benchLines { + switch { + case edited && i%17 == 0: + fmt.Fprintf(&b, "%s line %d edited by the feature branch\n", name, i) + default: + fmt.Fprintf(&b, "%s line %d with enough text to look like source\n", name, i) + } + } + return []byte(b.String()) +} + +func benchFileName(i int) string { return fmt.Sprintf("src/f%02d.txt", i) } + +// newBenchRepo builds the bare fixture repository under a temporary directory +// owned by b, and returns the handles the benchmarks resolve against. +func newBenchRepo(b *testing.B) benchRepo { + b.Helper() + if _, err := exec.LookPath("git"); err != nil { + b.Skipf("git not available: %v", err) + } + + root := b.TempDir() + work := b.TempDir() + + d1 := "2024-03-01T00:00:00Z" + d2 := "2024-03-02T00:00:00Z" + d3 := "2024-03-03T00:00:00Z" + + gitTest(b, work, d1, "init", "-b", "main") + + for i := range benchFiles { + writeFile(b, work, benchFileName(i), benchLineFile(benchFileName(i), false)) + } + gitTest(b, work, d1, "add", "-A") + gitTest(b, work, d1, "commit", "-m", "seed: the tree under comparison") + + gitTest(b, work, d2, "branch", "feature") + + // main advances past the fork point, so the merge base is not main. + for i := range 4 { + name := benchFileName(i) + writeFile(b, work, name, benchLineFile(name+" on main", true)) + } + gitTest(b, work, d2, "add", "-A") + gitTest(b, work, d2, "commit", "-m", "main-work: edits on the base branch") + + gitTest(b, work, d3, "checkout", "feature") + for i := range benchModified { + name := benchFileName(i) + writeFile(b, work, name, benchLineFile(name, true)) + } + for i := range 3 { + name := fmt.Sprintf("src/added%02d.txt", i) + writeFile(b, work, name, benchLineFile(name, false)) + } + for i := benchFiles - 2; i < benchFiles; i++ { + if err := os.Remove(filepath.Join(work, benchFileName(i))); err != nil { + b.Fatal(err) + } + } + gitTest(b, work, d3, "mv", benchFileName(20), "src/renamed.txt") + gitTest(b, work, d3, "add", "-A") + gitTest(b, work, d3, "commit", "-m", "feature-work: edits, additions, deletions, a rename") + + if err := os.MkdirAll(filepath.Join(root, "~"+fxOwner), 0o755); err != nil { + b.Fatal(err) + } + gitTest(b, work, d3, "clone", "--bare", work, filepath.Join(root, "~"+fxOwner, "benchrepo")) + + return benchRepo{ + root: root, + main: fxRevIn(b, root, "benchrepo", "main"), + head: fxRevIn(b, root, "benchrepo", "feature"), + } +} + +// openBench opens the fixture through the code under test. +func openBench(b *testing.B, fx benchRepo) *Repo { + b.Helper() + repo, err := Open(fx.root, fxOwner, "benchrepo") + if err != nil { + b.Fatalf("Open: %v", err) + } + return repo +} + +// BenchmarkDiff renders the unified diff a compare page shows, in both of the +// grammars ParseCompareSpec accepts. Three-dot is the costlier of the two and +// the one a review reaches for: it resolves a merge base before it can diff. +func BenchmarkDiff(b *testing.B) { + fx := newBenchRepo(b) + repo := openBench(b, fx) + ctx := context.Background() + + for _, tc := range []struct { + name string + threeDot bool + }{ + {"two-dot", false}, + {"three-dot", true}, + } { + b.Run(tc.name, func(b *testing.B) { + spec := core.CompareSpec{Base: fx.main, Head: fx.head, ThreeDot: tc.threeDot} + b.ReportAllocs() + for b.Loop() { + p, err := repo.Diff(ctx, spec) + if err != nil { + b.Fatalf("Diff: %v", err) + } + if p.Truncated { + b.Fatal("the fixture patch hit the byte cap: the benchmark is measuring cutPatch") + } + } + }) + } +} + +// BenchmarkDiffStat measures the file list of the same comparison. It is not a +// cheaper Diff and is not expected to read as one: the patch is generated in +// full either way, and what this leaves out is only the String() of it. +func BenchmarkDiffStat(b *testing.B) { + fx := newBenchRepo(b) + repo := openBench(b, fx) + ctx := context.Background() + spec := core.CompareSpec{Base: fx.main, Head: fx.head, ThreeDot: true} + + b.ReportAllocs() + for b.Loop() { + files, err := repo.DiffStat(ctx, spec) + if err != nil { + b.Fatalf("DiffStat: %v", err) + } + if len(files) == 0 { + b.Fatal("no file changes: the fixture is not what the benchmark thinks it is") + } + } +} + +// BenchmarkCommitPatch measures the single-commit view: resolve a revision, +// diff it against its first parent, render and map. It is the whole of what +// /~owner/repo/commit/ does. +func BenchmarkCommitPatch(b *testing.B) { + fx := newBenchRepo(b) + repo := openBench(b, fx) + ctx := context.Background() + + b.ReportAllocs() + for b.Loop() { + patch, files, info, err := repo.CommitPatch(ctx, fx.head) + if err != nil { + b.Fatalf("CommitPatch: %v", err) + } + if patch.Text == "" || len(files) == 0 || info == nil { + b.Fatal("empty commit patch: the fixture is not what the benchmark thinks it is") + } + } +} + +// BenchmarkMapFilePatches measures the pure projection every diff surface runs +// over go-git's result. It uses the synthetic file patches of mapping_test.go +// rather than a repository, because it is the tallying of chunks that is being +// measured and a real diff would bury it under its own generation. +func BenchmarkMapFilePatches(b *testing.B) { + line := strings.Repeat("a line of content that is about this long\n", 8) + fps := make([]diff.FilePatch, 0, benchFiles) + for i := range benchFiles { + name := benchFileName(i) + fps = append(fps, fakeFilePatch{ + from: file(name), + to: file(name), + chunks: []diff.Chunk{ + fakeChunk{content: line, op: diff.Equal}, + fakeChunk{content: line, op: diff.Delete}, + fakeChunk{content: line, op: diff.Add}, + fakeChunk{content: line, op: diff.Equal}, + }, + }) + } + + b.ReportAllocs() + for b.Loop() { + if got := mapFilePatches(fps); len(got) != benchFiles { + b.Fatalf("mapped %d file changes, want %d", len(got), benchFiles) + } + } +} + +// BenchmarkCutPatch measures the byte cap on the two inputs that behave +// differently: text under the limit, which must be returned untouched, and text +// over it, which is scanned backwards for a file boundary. The second is the +// one a pathological diff reaches, and it is the reason the cap exists. +func BenchmarkCutPatch(b *testing.B) { + var sb strings.Builder + for i := range benchFiles { + name := benchFileName(i) + fmt.Fprintf(&sb, "diff --git a/%s b/%s\n--- a/%s\n+++ b/%s\n", name, name, name, name) + sb.Write(benchLineFile(name, false)) + } + text := sb.String() + + for _, tc := range []struct { + name string + limit int64 + }{ + {"under-the-cap", int64(len(text)) + 1}, + {"over-the-cap", int64(len(text) / 2)}, + } { + b.Run(tc.name, func(b *testing.B) { + wantTruncated := tc.limit < int64(len(text)) + b.ReportAllocs() + for b.Loop() { + out, truncated := cutPatch(text, tc.limit) + if truncated != wantTruncated { + b.Fatalf("truncated=%v, want %v", truncated, wantTruncated) + } + if out == "" { + b.Fatal("cutPatch returned nothing") + } + } + }) + } +} diff --git a/gitx/fidelity_test.go b/gitx/fidelity_test.go index 2c9da1815d75e17ed79c4b7664f81cef06b40368..1f852f5732f9edee0683656e61e505e01c26259e 100644 --- a/gitx/fidelity_test.go +++ b/gitx/fidelity_test.go @@ -49,8 +49,9 @@ head = fxRevIn(t, reposRoot, "fidelity", "main") return reposRoot, base, head } -// fxRevIn is fxRev for an arbitrary repo name under fxOwner. -func fxRevIn(t *testing.T, reposRoot, name, rev string) string { +// fxRevIn is fxRev for an arbitrary repo name under fxOwner. Like gitTest it +// takes a testing.TB, because bench_test.go resolves its landmarks with it. +func fxRevIn(t testing.TB, reposRoot, name, rev string) string { t.Helper() bare := filepath.Join(reposRoot, "~"+fxOwner, name) cmd := exec.Command("git", "-C", bare, "rev-parse", rev) diff --git a/gitx/fixture_test.go b/gitx/fixture_test.go index ecdb87a796ca41737e595fb1560e6886eb240a6c..c3dd1ee041277ac6a51067365b48f72b027efde3 100644 --- a/gitx/fixture_test.go +++ b/gitx/fixture_test.go @@ -31,8 +31,9 @@ fxName = "demo" ) // gitTest runs git in dir with a hardened, deterministic environment, failing -// the test on error. -func gitTest(t *testing.T, dir string, date string, args ...string) string { +// the test on error. It takes a testing.TB rather than a *testing.T because +// bench_test.go builds its own fixture repository with the same helper. +func gitTest(t testing.TB, dir string, date string, args ...string) string { t.Helper() cmd := exec.Command("git", args...) cmd.Dir = dir @@ -55,7 +56,7 @@ } return string(out) } -func writeFile(t *testing.T, dir, name string, data []byte) { +func writeFile(t testing.TB, dir, name string, data []byte) { t.Helper() p := filepath.Join(dir, name) if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil {