mirror of
https://github.com/coder/coder.git
synced 2026-06-04 05:28:20 +00:00
Ethan
c650aabbef
My agent added `//nolint:testpackage` to a test file on one of my PRs. Again. This PR cleans it up across the entire repo and updates the in-repo conventions so future agents stop doing it. The repo already has a precedent for white-box tests that need to touch unexported symbols: `*_internal_test.go` (145+ existing files). The `testpackage` linter's default `skip-regexp` exempts that filename suffix, so the `//nolint:testpackage` directive is unnecessary in every case where someone reached for it. This PR renames 51 such files to `*_internal_test.go` via `git mv` so blame and history follow, and strips the dead directive from 2 files that were already correctly named (`coderd/oauth2provider/authorize_internal_test.go`, `coderd/x/chatd/advisor_internal_test.go`). `.claude/docs/TESTING.md` now documents the rule explicitly under *Test Package Naming*, which is imported into the root `AGENTS.md` via `@.claude/docs/TESTING.md`. The rule: prefer `package foo_test`; if you need internal access, rename the file to `*_internal_test.go` rather than adding a nolint directive.
114 lines
3.2 KiB
Go
114 lines
3.2 KiB
Go
package chatdebug
|
|
|
|
import (
|
|
"context"
|
|
"testing"
|
|
|
|
"github.com/google/uuid"
|
|
"github.com/stretchr/testify/require"
|
|
"go.uber.org/mock/gomock"
|
|
|
|
"github.com/coder/coder/v2/coderd/database/dbmock"
|
|
"github.com/coder/coder/v2/testutil"
|
|
)
|
|
|
|
func TestBeginStepReuseStep(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
t.Run("reuses handle under ReuseStep", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
chatID := uuid.New()
|
|
ownerID := uuid.New()
|
|
runID := uuid.New()
|
|
t.Cleanup(func() { CleanupStepCounter(runID) })
|
|
|
|
ctrl := gomock.NewController(t)
|
|
db := dbmock.NewMockStore(ctrl)
|
|
expectDebugLoggingEnabled(t, db, ownerID)
|
|
expectCreateStepNumberWithRequestValidity(
|
|
t,
|
|
db,
|
|
runID,
|
|
chatID,
|
|
1,
|
|
OperationStream,
|
|
false,
|
|
)
|
|
expectDebugLoggingEnabled(t, db, ownerID)
|
|
|
|
svc := NewService(db, testutil.Logger(t), nil)
|
|
ctx := ContextWithRun(context.Background(), &RunContext{RunID: runID, ChatID: chatID})
|
|
ctx = ReuseStep(ctx)
|
|
opts := RecorderOptions{ChatID: chatID, OwnerID: ownerID}
|
|
|
|
firstHandle, firstEnriched := beginStep(ctx, svc, opts, OperationStream, nil)
|
|
secondHandle, secondEnriched := beginStep(ctx, svc, opts, OperationStream, nil)
|
|
|
|
require.NotNil(t, firstHandle)
|
|
require.Same(t, firstHandle, secondHandle)
|
|
require.Same(t, firstHandle.stepCtx, secondHandle.stepCtx)
|
|
require.Same(t, firstHandle.sink, secondHandle.sink)
|
|
require.Equal(t, runID, firstHandle.stepCtx.RunID)
|
|
require.Equal(t, chatID, firstHandle.stepCtx.ChatID)
|
|
require.Equal(t, int32(1), firstHandle.stepCtx.StepNumber)
|
|
require.Equal(t, OperationStream, firstHandle.stepCtx.Operation)
|
|
require.NotEqual(t, uuid.Nil, firstHandle.stepCtx.StepID)
|
|
|
|
firstStepCtx, ok := StepFromContext(firstEnriched)
|
|
require.True(t, ok)
|
|
secondStepCtx, ok := StepFromContext(secondEnriched)
|
|
require.True(t, ok)
|
|
require.Same(t, firstStepCtx, secondStepCtx)
|
|
require.Same(t, firstHandle.stepCtx, firstStepCtx)
|
|
require.Same(t, attemptSinkFromContext(firstEnriched), attemptSinkFromContext(secondEnriched))
|
|
})
|
|
|
|
t.Run("creates new handles without ReuseStep", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
chatID := uuid.New()
|
|
ownerID := uuid.New()
|
|
runID := uuid.New()
|
|
t.Cleanup(func() { CleanupStepCounter(runID) })
|
|
|
|
ctrl := gomock.NewController(t)
|
|
db := dbmock.NewMockStore(ctrl)
|
|
expectDebugLoggingEnabled(t, db, ownerID)
|
|
expectCreateStepNumberWithRequestValidity(
|
|
t,
|
|
db,
|
|
runID,
|
|
chatID,
|
|
1,
|
|
OperationStream,
|
|
false,
|
|
)
|
|
expectDebugLoggingEnabled(t, db, ownerID)
|
|
expectCreateStepNumberWithRequestValidity(
|
|
t,
|
|
db,
|
|
runID,
|
|
chatID,
|
|
2,
|
|
OperationStream,
|
|
false,
|
|
)
|
|
|
|
svc := NewService(db, testutil.Logger(t), nil)
|
|
ctx := ContextWithRun(context.Background(), &RunContext{RunID: runID, ChatID: chatID})
|
|
opts := RecorderOptions{ChatID: chatID, OwnerID: ownerID}
|
|
|
|
firstHandle, _ := beginStep(ctx, svc, opts, OperationStream, nil)
|
|
secondHandle, _ := beginStep(ctx, svc, opts, OperationStream, nil)
|
|
|
|
require.NotNil(t, firstHandle)
|
|
require.NotNil(t, secondHandle)
|
|
require.NotSame(t, firstHandle, secondHandle)
|
|
require.NotSame(t, firstHandle.sink, secondHandle.sink)
|
|
require.Equal(t, int32(1), firstHandle.stepCtx.StepNumber)
|
|
require.Equal(t, int32(2), secondHandle.stepCtx.StepNumber)
|
|
require.NotEqual(t, firstHandle.stepCtx.StepID, secondHandle.stepCtx.StepID)
|
|
})
|
|
}
|