Skip to content

Run the tests on every push - #3

Merged
dcadolph merged 1 commit into
mainfrom
ci/basics
Sep 28, 2026
Merged

dcadolph merged 1 commit into
mainfrom
ci/basics

Conversation

@dcadolph

Copy link
Copy Markdown
Owner

preen had release.yml and nothing else. Twenty-five test files, none of them run
by anything. For a tool whose central claim is that it rewrites history and
provably does not lose a byte, shipping no visible evidence that the tests pass is
the wrong gap to have.

The workflow

Build, vet, test, and a gofmt check, on push to main and on every pull request.
Go version comes from go.mod rather than being pinned separately, so it cannot
drift from what the module declares. Git is given an identity and a default branch
name, because the tests drive a real git rather than a fake.

A test isolation bug found while wiring it up

The suite did not pass on my own machine. Three hook tests failed:

--- FAIL: TestHookRewriteRejectedByDefault   the conservation check did not catch the hook
--- FAIL: TestNoVerifySkipsARejectingHook    a rejecting hook did not stop the run
--- FAIL: TestHookRewriteAcceptedWhenAllowed the hook's edit was accepted without being reported

Not a real failure, and not a CI problem either, which is the interesting part. I
have core.hooksPath set in my global git config, which is the ordinary way to
install shared hooks. While it is set, git ignores repository-local hooks, so the
hook each test installs never runs and the test is left asserting against nothing.

newHarness already passed GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM to the git
commands it runs itself. The engine under test spawns git on its own, and those
children inherit the process environment rather than the harness's, so the
isolation stopped at the boundary that mattered. The run package now sets both
in TestMain, which covers every git the package causes to run.

This would have passed CI either way, on a clean runner with no global config.
Which is the worst version of the problem: green in CI and red for anyone who
installs hooks the normal way, including me. The full suite is now green locally
with my real configuration in place.

preen had a release workflow and nothing that ran the twenty-five test
files it already carries, so a tool whose pitch is that it provably does
not lose your work shipped no visible evidence that anything was checked.

Build, vet, test and a gofmt check, with git given an identity because the
tests drive a real one.

The run package now clears GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM for
itself. Its harness already did that for the commands it runs, but the
engine spawns git of its own and those children read the process
environment, so anyone with core.hooksPath set globally watched three hook
tests fail against hooks they never wrote. The suite was red on my machine
and green everywhere else, which is the wrong way round.
@dcadolph
dcadolph merged commit 3813a98 into main Sep 28, 2026
1 check passed
@dcadolph
dcadolph deleted the ci/basics branch September 28, 2026 01:08
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