Skip to content

test: benchmark the installation token cache hit - #71

Merged
jferrl merged 1 commit into
mainfrom
perf/cache-hit-benchmark
Sep 19, 2026
Merged

jferrl merged 1 commit into
mainfrom
perf/cache-hit-benchmark

Conversation

@jferrl

@jferrl jferrl commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Description

Almost every Token() call returns a cached token, and nothing measured that path. A
change that reintroduced work there — an allocation per call, or a lost cache — would not
fail any existing test, because they all assert behaviour rather than cost.

Baseline on an 18-core M-series machine:

BenchmarkInstallationTokenSource_CacheHit-18
    31613367    34.90 ns/op    0 B/op    0 allocs/op

The benchmark asserts the test server saw exactly one request after the loop. Without that
check it would still print a number if caching broke — it would just be timing HTTP round
trips and calling them cache hits. Verified against a build with the cache removed:

benchmark made 30813 installation-token requests, want 1: the loop was not measuring cache hits

What is deliberately not here

No concurrency tests. I wrote a set that counted HTTP requests under 200 concurrent
callers, then checked whether they earned their place by mutating the source:

Mutation Caught by the existing suite
Caching disabled entirely Yes — TestReuseTokenSourceWithSkew
Cache kept, lock dropped across the refresh Yes — TestReuseTokenSourceWithSkew
Wrapper correct but not wired into NewInstallationTokenSource Yes — Test_WithInstallationExpirySkew_Wiring

All three were already covered, so the new tests were redundant and were dropped. The
existing collapse test is also the stronger assertion: it checks every goroutine receives
the same token, not just that one request was made.

Type of Change

  • 🔧 Chore (dependency updates, tooling, etc.)

Testing

  • New and existing unit tests pass locally with my changes
  • I have tested this change manually

go test ./... passes and golangci-lint run ./... reports 0 issues. The benchmark does
not run during go test, so CI time is unchanged.

Documentation

  • I have updated the documentation accordingly
  • I have updated the CHANGELOG.md file
  • I have added/updated code comments where necessary

CHANGELOG.md is untouched: this adds no library behaviour for a consumer to act on.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have checked my code and corrected any misspellings
  • My changes are compatible with the Go versions in go.mod and the CI matrix

Almost every Token() call returns a cached token, and nothing measured
that path. A change that reintroduced work there -- an allocation per
call, or a lost cache -- would not fail any test, since the existing
ones all assert behaviour rather than cost.

The benchmark asserts the server saw exactly one request after the
loop. Without that check it would still report a number if caching
broke; it would just be timing HTTP round trips and calling them cache
hits.

Current baseline on an 18-core M-series machine:

  BenchmarkInstallationTokenSource_CacheHit-18
      31613367    34.90 ns/op    0 B/op    0 allocs/op

No new concurrency tests: TestReuseTokenSourceWithSkew already pins
the collapse to one upstream fetch, and Test_WithInstallationExpirySkew_Wiring
pins that the constructor uses the wrapper. Mutation testing confirmed
both catch a disabled cache, a lock dropped across the refresh, and an
unwrapped installation source.
@codecov

codecov Bot commented Sep 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.88%. Comparing base (69b5dd9) to head (487cbe9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #71   +/-   ##
=======================================
  Coverage   98.88%   98.88%           
=======================================
  Files           5        5           
  Lines         447      447           
=======================================
  Hits          442      442           
  Misses          5        5           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jferrl
jferrl merged commit 9204502 into main Sep 19, 2026
10 checks passed
@jferrl
jferrl deleted the perf/cache-hit-benchmark branch September 19, 2026 11:46
Sign up for free to 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