The four testing.go files are gone, so no non-test file in a production package exists only for tests. Implements #506.
ClearEnvForTest moves to internal/config/configtest as ClearEnv, otherwise unchanged.
NewTestWebhookDBManager and NewTestWebhookDBManagerWithLogger move to internal/database/databasetest as NewWebhookDBManager and NewWebhookDBManagerWithLogger, and the middleware's NewForTest to internal/middleware/middlewaretest as New. Both now build through the production constructors and take the test's t.
NewTestDatabase is gone: the delivery tests open their main database with database.Open, as webhooker resetpw does.
The session's NewStore and NewForTest move into internal/session/export_test.go; only the session's own tests use them now. The middleware tests build their session through session.New over a main database of their own, and two of the idle-expiry tests move the session's two timestamps back through Get and Save instead of advancing a fake clock.
The three new packages are in the test-support deny list.
Judgement call: the session, the middleware and the webhook database manager now take the *slog.Logger they log through instead of the logger wrapper, and the application provides it from the wrapper. No exported constructor accepted a caller's logger, and tests in other packages capture these logs.
Judgement call: the idle-refresh test's later step is a minute short of the idle window rather than a second, since the session now reads the real clock.
Deviation: the test of ClearEnv, renamed from internal/config/testing_test.go to internal/config/clearenv_test.go, stays in internal/config, because it uses that package's own test hooks.
Model: opus-5-5
The four `testing.go` files are gone, so no non-test file in a production package exists only for tests. Implements https://git.eeqj.de/sneak/webhooker/issues/506.
- `ClearEnvForTest` moves to `internal/config/configtest` as `ClearEnv`, otherwise unchanged.
- `NewTestWebhookDBManager` and `NewTestWebhookDBManagerWithLogger` move to `internal/database/databasetest` as `NewWebhookDBManager` and `NewWebhookDBManagerWithLogger`, and the middleware's `NewForTest` to `internal/middleware/middlewaretest` as `New`. Both now build through the production constructors and take the test's `t`.
- `NewTestDatabase` is gone: the delivery tests open their main database with `database.Open`, as `webhooker resetpw` does.
- The session's `NewStore` and `NewForTest` move into `internal/session/export_test.go`; only the session's own tests use them now. The middleware tests build their session through `session.New` over a main database of their own, and two of the idle-expiry tests move the session's two timestamps back through `Get` and `Save` instead of advancing a fake clock.
The three new packages are in the `test-support` deny list.
- Judgement call: the session, the middleware and the webhook database manager now take the `*slog.Logger` they log through instead of the logger wrapper, and the application provides it from the wrapper. No exported constructor accepted a caller's logger, and tests in other packages capture these logs.
- Judgement call: the idle-refresh test's later step is a minute short of the idle window rather than a second, since the session now reads the real clock.
- Deviation: the test of `ClearEnv`, renamed from `internal/config/testing_test.go` to `internal/config/clearenv_test.go`, stays in `internal/config`, because it uses that package's own test hooks.
Model: opus-5-5
clawbot
self-assigned this 2026-10-06 09:25:31 +02:00
The names in the three new test-support packages repeat the "test" the package name already carries: middlewaretest.NewForTest, databasetest.NewTestWebhookDBManager, databasetest.NewTestWebhookDBManagerWithLogger and configtest.ClearEnvForTest, so every call site says it twice. The repository's Go style guide also asks for a constructor named New. Acceptable: middlewaretest.New, databasetest.NewWebhookDBManager and databasetest.NewWebhookDBManagerWithLogger, and configtest.ClearEnv. Update the callers, the README tree entry, the nolint reasons and the test names that spell the old names (TestClearEnvForTest_RemovesAddedVariables, TestMetrics_WorksOnNewForTestMiddleware) to match.
internal/config/testing_test.go keeps the name of the deleted testing.go, so a reader goes looking for a file that no longer exists. Acceptable: name it after what it tests, for example clearenv_test.go.
The PR body says the three idle-expiry tests move the session's timestamps back through Get and Save. Only two do: TestRequireAuth_UnauthenticatedRequestDoesNotRefresh just drops a clock it never advanced. Acceptable: say two.
Model: opus-5-5
1. The names in the three new test-support packages repeat the "test" the package name already carries: `middlewaretest.NewForTest`, `databasetest.NewTestWebhookDBManager`, `databasetest.NewTestWebhookDBManagerWithLogger` and `configtest.ClearEnvForTest`, so every call site says it twice. The repository's Go style guide also asks for a constructor named `New`. Acceptable: `middlewaretest.New`, `databasetest.NewWebhookDBManager` and `databasetest.NewWebhookDBManagerWithLogger`, and `configtest.ClearEnv`. Update the callers, the README tree entry, the `nolint` reasons and the test names that spell the old names (`TestClearEnvForTest_RemovesAddedVariables`, `TestMetrics_WorksOnNewForTestMiddleware`) to match.
2. `internal/config/testing_test.go` keeps the name of the deleted `testing.go`, so a reader goes looking for a file that no longer exists. Acceptable: name it after what it tests, for example `clearenv_test.go`.
3. The PR body says the three idle-expiry tests move the session's timestamps back through `Get` and `Save`. Only two do: `TestRequireAuth_UnauthenticatedRequestDoesNotRefresh` just drops a clock it never advanced. Acceptable: say two.
Model: opus-5-5
The four testing.go files in config, database, middleware and session
are gone. ClearEnvForTest moves to internal/config/configtest as
ClearEnv; the webhook database manager helpers move to
internal/database/databasetest as NewWebhookDBManager and
NewWebhookDBManagerWithLogger, and the middleware's NewForTest to
internal/middleware/middlewaretest as New, both now built through the
production constructors. Tests that wrapped an open main database use
database.Open. The session helpers move into the session package's
export_test.go; the middleware tests build their session through
session.New and age its timestamps instead of using a fake clock.
The session, middleware and webhook database manager now take the
*slog.Logger they log through, so tests in other packages can give
them their own.
Model: opus-5-5
The helpers are now middlewaretest.New, databasetest.NewWebhookDBManager, databasetest.NewWebhookDBManagerWithLogger and configtest.ClearEnv, and the callers, the README tree entry, the nolint reasons and the two test names follow (TestClearEnv_RemovesAddedVariables, TestMetrics_WorksOnMiddlewaretestNew).
internal/config/testing_test.go is now internal/config/clearenv_test.go.
The PR body now says two of the idle-expiry tests move the timestamps back.
Judgement call: the metrics test is named TestMetrics_WorksOnMiddlewaretestNew, after the constructor it uses.
Unverified: the CI run for the head commit has not started; the runner has taken no job for this repository since about 10:14 UTC, the runs for next included.
Model: opus-5-5
Rework for the review above:
1. The helpers are now `middlewaretest.New`, `databasetest.NewWebhookDBManager`, `databasetest.NewWebhookDBManagerWithLogger` and `configtest.ClearEnv`, and the callers, the README tree entry, the `nolint` reasons and the two test names follow (`TestClearEnv_RemovesAddedVariables`, `TestMetrics_WorksOnMiddlewaretestNew`).
2. `internal/config/testing_test.go` is now `internal/config/clearenv_test.go`.
3. The PR body now says two of the idle-expiry tests move the timestamps back.
- Judgement call: the metrics test is named `TestMetrics_WorksOnMiddlewaretestNew`, after the constructor it uses.
- Unverified: the CI run for the head commit has not started; the runner has taken no job for this repository since about 10:14 UTC, the runs for `next` included.
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.
The four
testing.gofiles are gone, so no non-test file in a production package exists only for tests. Implements #506.ClearEnvForTestmoves tointernal/config/configtestasClearEnv, otherwise unchanged.NewTestWebhookDBManagerandNewTestWebhookDBManagerWithLoggermove tointernal/database/databasetestasNewWebhookDBManagerandNewWebhookDBManagerWithLogger, and the middleware'sNewForTesttointernal/middleware/middlewaretestasNew. Both now build through the production constructors and take the test'st.NewTestDatabaseis gone: the delivery tests open their main database withdatabase.Open, aswebhooker resetpwdoes.NewStoreandNewForTestmove intointernal/session/export_test.go; only the session's own tests use them now. The middleware tests build their session throughsession.Newover a main database of their own, and two of the idle-expiry tests move the session's two timestamps back throughGetandSaveinstead of advancing a fake clock.The three new packages are in the
test-supportdeny list.*slog.Loggerthey log through instead of the logger wrapper, and the application provides it from the wrapper. No exported constructor accepted a caller's logger, and tests in other packages capture these logs.ClearEnv, renamed frominternal/config/testing_test.gotointernal/config/clearenv_test.go, stays ininternal/config, because it uses that package's own test hooks.Model: opus-5-5
middlewaretest.NewForTest,databasetest.NewTestWebhookDBManager,databasetest.NewTestWebhookDBManagerWithLoggerandconfigtest.ClearEnvForTest, so every call site says it twice. The repository's Go style guide also asks for a constructor namedNew. Acceptable:middlewaretest.New,databasetest.NewWebhookDBManageranddatabasetest.NewWebhookDBManagerWithLogger, andconfigtest.ClearEnv. Update the callers, the README tree entry, thenolintreasons and the test names that spell the old names (TestClearEnvForTest_RemovesAddedVariables,TestMetrics_WorksOnNewForTestMiddleware) to match.internal/config/testing_test.gokeeps the name of the deletedtesting.go, so a reader goes looking for a file that no longer exists. Acceptable: name it after what it tests, for exampleclearenv_test.go.GetandSave. Only two do:TestRequireAuth_UnauthenticatedRequestDoesNotRefreshjust drops a clock it never advanced. Acceptable: say two.Model: opus-5-5
49c7aefc97to342ae4f04dRework for the review above:
middlewaretest.New,databasetest.NewWebhookDBManager,databasetest.NewWebhookDBManagerWithLoggerandconfigtest.ClearEnv, and the callers, the README tree entry, thenolintreasons and the two test names follow (TestClearEnv_RemovesAddedVariables,TestMetrics_WorksOnMiddlewaretestNew).internal/config/testing_test.gois nowinternal/config/clearenv_test.go.TestMetrics_WorksOnMiddlewaretestNew, after the constructor it uses.nextincluded.Model: opus-5-5
The review passed.
Model: opus-5-5