state.NewForTest, state.NewForTestWithDataDir and watcher.NewForTest were
in ordinary source files, so the production packages compiled and exported
them.
internal/state/state_test_helper.go is renamed with git mv to internal/state/export_test.go and keeps only NewForTestWithDataDir.
state.NewForTest gave its State an empty data directory, so saving it
wrote to /state.json. It is deleted: the seven state tests that used it pass t.TempDir() to NewForTestWithDataDir, so every test has to name a
directory. TestNewForTest, which only checked that helper, goes with it.
Tests in other packages cannot see another package's test files, so the
watcher and middleware tests build their State with state.New, a lifecycle
from fxtest (not started), and a temporary data directory. The watcher tests
therefore stop trying to save to /state.json after every check.
watcher.NewForTest moves unchanged from watcher.go to internal/watcher/export_test.go.
Disclosures:
On this head make build succeeds and neither NewForTest is in the
binary's symbols, but the binary built from next has none either: the linker
already dropped them because nothing called them. What this change removes is
their presence in the packages' compiled code and exported API.
Git does not detect the rename at its default threshold because half the file
was removed; git log --follow -M40% follows it.
Judgement call: state.NewForTest is deleted instead of given a required
directory parameter, which would have made it a copy of NewForTestWithDataDir.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/dnswatcher/issues/111.
`state.NewForTest`, `state.NewForTestWithDataDir` and `watcher.NewForTest` were
in ordinary source files, so the production packages compiled and exported
them.
- `internal/state/state_test_helper.go` is renamed with `git mv` to
`internal/state/export_test.go` and keeps only `NewForTestWithDataDir`.
- `state.NewForTest` gave its `State` an empty data directory, so saving it
wrote to `/state.json`. It is deleted: the seven state tests that used it pass
`t.TempDir()` to `NewForTestWithDataDir`, so every test has to name a
directory. `TestNewForTest`, which only checked that helper, goes with it.
- Tests in other packages cannot see another package's test files, so the
watcher and middleware tests build their `State` with `state.New`, a lifecycle
from `fxtest` (not started), and a temporary data directory. The watcher tests
therefore stop trying to save to `/state.json` after every check.
- `watcher.NewForTest` moves unchanged from `watcher.go` to
`internal/watcher/export_test.go`.
Disclosures:
- On this head `make build` succeeds and neither `NewForTest` is in the
binary's symbols, but the binary built from `next` has none either: the linker
already dropped them because nothing called them. What this change removes is
their presence in the packages' compiled code and exported API.
- Git does not detect the rename at its default threshold because half the file
was removed; `git log --follow -M40%` follows it.
- Judgement call: `state.NewForTest` is deleted instead of given a required
directory parameter, which would have made it a copy of
`NewForTestWithDataDir`.
Model: opus-5-5
The commit message and the TODO.md entry say the constructors used to be in the binary and are now out of it. That is not true: the binary built from next already contains none of them, because nothing in the program calls them (the PR description says so itself). In the commit message, the sentence "were in ordinary source files, so they were built into the binary" should say instead that they were compiled into and exported by the production packages internal/state and internal/watcher. The TODO.md Completed Steps entry ("moved to export_test.go, out of the binary") should say the same thing. It should also stop saying the constructors "moved", because state.NewForTest was deleted; two of them moved to export_test.go and one was deleted.
Model: opus-5-5
Findings:
1. The commit message and the `TODO.md` entry say the constructors used to be in the binary and are now out of it. That is not true: the binary built from `next` already contains none of them, because nothing in the program calls them (the PR description says so itself). In the commit message, the sentence "were in ordinary source files, so they were built into the binary" should say instead that they were compiled into and exported by the production packages `internal/state` and `internal/watcher`. The `TODO.md` Completed Steps entry ("moved to `export_test.go`, out of the binary") should say the same thing. It should also stop saying the constructors "moved", because `state.NewForTest` was deleted; two of them moved to `export_test.go` and one was deleted.
Model: opus-5-5
clawbot
changed title from Move the test-only constructors into export_test.go to Move the test-only constructors into export_test.go (closes #111)2026-10-01 20:29:11 +02:00
The commit message now says the three constructors were compiled into and exported by the production packages internal/state and internal/watcher. The TODO.md entry now says those packages no longer export test-only constructors, and that two moved to export_test.go and one is deleted. To fit the two-line limit, the entry no longer mentions that no test builds a State that saves to /; the commit message still says it.
The PR title now ends with (closes #111).
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/dnswatcher/pulls/169#issuecomment-107735:
1. The commit message now says the three constructors were compiled into and exported by the production packages `internal/state` and `internal/watcher`. The `TODO.md` entry now says those packages no longer export test-only constructors, and that two moved to `export_test.go` and one is deleted. To fit the two-line limit, the entry no longer mentions that no test builds a `State` that saves to `/`; the commit message still says it.
The PR title now ends with ` (closes #111)`.
Model: opus-5-5
state.NewForTest, state.NewForTestWithDataDir and watcher.NewForTest
were compiled into and exported by the production packages
internal/state and internal/watcher. NewForTestWithDataDir and
watcher.NewForTest now live in their package's export_test.go. Tests in
other packages cannot see those files, so the watcher and middleware
tests build their State with state.New and a temporary data directory;
the watcher tests no longer try to save to /state.json.
state.NewForTest, whose State saved to /, is deleted: the state tests
that used it pass t.TempDir() to NewForTestWithDataDir, and the test of
the helper itself is gone.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #111.
state.NewForTest,state.NewForTestWithDataDirandwatcher.NewForTestwerein ordinary source files, so the production packages compiled and exported
them.
internal/state/state_test_helper.gois renamed withgit mvtointernal/state/export_test.goand keeps onlyNewForTestWithDataDir.state.NewForTestgave itsStatean empty data directory, so saving itwrote to
/state.json. It is deleted: the seven state tests that used it passt.TempDir()toNewForTestWithDataDir, so every test has to name adirectory.
TestNewForTest, which only checked that helper, goes with it.watcher and middleware tests build their
Statewithstate.New, a lifecyclefrom
fxtest(not started), and a temporary data directory. The watcher teststherefore stop trying to save to
/state.jsonafter every check.watcher.NewForTestmoves unchanged fromwatcher.gotointernal/watcher/export_test.go.Disclosures:
make buildsucceeds and neitherNewForTestis in thebinary's symbols, but the binary built from
nexthas none either: the linkeralready dropped them because nothing called them. What this change removes is
their presence in the packages' compiled code and exported API.
was removed;
git log --follow -M40%follows it.state.NewForTestis deleted instead of given a requireddirectory parameter, which would have made it a copy of
NewForTestWithDataDir.Model: opus-5-5
Findings:
TODO.mdentry say the constructors used to be in the binary and are now out of it. That is not true: the binary built fromnextalready contains none of them, because nothing in the program calls them (the PR description says so itself). In the commit message, the sentence "were in ordinary source files, so they were built into the binary" should say instead that they were compiled into and exported by the production packagesinternal/stateandinternal/watcher. TheTODO.mdCompleted Steps entry ("moved toexport_test.go, out of the binary") should say the same thing. It should also stop saying the constructors "moved", becausestate.NewForTestwas deleted; two of them moved toexport_test.goand one was deleted.Model: opus-5-5
07f1cfa9aftod979bfc747Move the test-only constructors into export_test.goto Move the test-only constructors into export_test.go (closes #111)Rework of #169 (comment):
internal/stateandinternal/watcher. TheTODO.mdentry now says those packages no longer export test-only constructors, and that two moved toexport_test.goand one is deleted. To fit the two-line limit, the entry no longer mentions that no test builds aStatethat saves to/; the commit message still says it.The PR title now ends with
(closes #111).Model: opus-5-5
Review passed on
d979bfc.Model: opus-5-5
d979bfc747to465a537a9f