test(coderd): remove flaky audit transaction case (#28468)

The `TransactionAfterNoChange` case in
`TestChatRetentionDays_AuditInfrastructureFailureRejectsWrite` was flaky
in [Flake Check run
32717900411](https://github.com/coder/coder/actions/runs/32717900411).
Its generic `InTx` wrapper armed a shared fail-next flag, so an
unrelated background transaction could consume the failure and let the
target PUT return 204 instead of the expected 500.

Remove the shared transaction wrapper and replace the endpoint-level
case with deterministic unit coverage of
`auditedChatOperationalSettingWrite`. The mock transaction executes the
no-op callback successfully and then returns an error, preserving
coverage that audit suppression happens only after the transaction
succeeds without involving API background transactions.
This commit is contained in:
Ethan
2026-08-25 12:59:19 +01:00
committed by GitHub
parent 16e0c4dba7
commit 6284964b5f
2 changed files with 43 additions and 36 deletions
+43
View File
@@ -18,6 +18,7 @@ import (
"cdr.dev/slog/v3"
"cdr.dev/slog/v3/sloggers/slogtest"
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/database/dbmock"
@@ -29,6 +30,48 @@ import (
"github.com/coder/coder/v2/testutil"
)
func TestAuditedChatOperationalSettingWriteNoOpTransactionFailure(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitShort)
db := dbmock.NewMockStore(gomock.NewController(t))
txErr := xerrors.New("commit transaction")
db.EXPECT().AcquireLock(
gomock.Any(),
database.GenLockID(string(chatOperationalSettingChatRetentionDays)),
).Return(nil)
db.EXPECT().GetChatSiteConfigValue(
gomock.Any(),
string(chatOperationalSettingChatRetentionDays),
).Return(database.GetChatSiteConfigValueRow{}, nil)
db.EXPECT().InTx(gomock.Any(), nil).DoAndReturn(
func(fn func(database.Store) error, _ *database.TxOptions) error {
require.NoError(t, fn(db))
return txErr
},
)
auditCanceled := false
api := &API{Options: &Options{Database: db}}
err := api.auditedChatOperationalSettingWrite(
ctx,
&audit.Request[database.ChatOperationalSettings]{},
func(commit bool) {
require.False(t, commit)
auditCanceled = true
},
chatOperationalSettingChatRetentionDays,
"30",
func(database.Store) error {
return xerrors.New("write unexpectedly called")
},
)
require.ErrorIs(t, err, txErr)
require.False(t, auditCanceled)
}
// ExtractChatParam authorizes the read, then GetAIBridgeChatCost authorizes it
// again. A denial on the second check means the ACL changed in between (a
// read-authz race). Assert it surfaces as 404, not 500.
-36
View File
@@ -638,34 +638,6 @@ type failNextGetChatSiteConfigValueStore struct {
failNextGetChatSiteConfigValue *atomic.Bool
}
type failNextChatOperationalSettingTransactionStore struct {
database.Store
failNextTransaction *atomic.Bool
}
func newFailNextChatOperationalSettingTransactionStore(store database.Store) *failNextChatOperationalSettingTransactionStore {
return &failNextChatOperationalSettingTransactionStore{
Store: store,
failNextTransaction: &atomic.Bool{},
}
}
func (s *failNextChatOperationalSettingTransactionStore) InTx(
function func(database.Store) error,
txOpts *database.TxOptions,
) error {
return s.Store.InTx(func(tx database.Store) error {
if err := function(tx); err != nil {
return err
}
if s.failNextTransaction.CompareAndSwap(true, false) {
return stderrors.New("forced chat operational setting transaction failure")
}
return nil
}, txOpts)
}
type failNextUpsertChatRetentionDaysStore struct {
database.Store
@@ -16840,14 +16812,6 @@ func TestChatRetentionDays_AuditInfrastructureFailureRejectsWrite(t *testing.T)
return store, func() { store.failNextAcquireLock.Store(true) }
},
},
{
name: "TransactionAfterNoChange",
retentionDays: 30,
store: func(db database.Store) (database.Store, func()) {
store := newFailNextChatOperationalSettingTransactionStore(db)
return store, func() { store.failNextTransaction.Store(true) }
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {