diff --git a/frontend/src/__tests__/api.test.ts b/frontend/src/__tests__/api.test.ts index 3c8d5733..3a381be0 100644 --- a/frontend/src/__tests__/api.test.ts +++ b/frontend/src/__tests__/api.test.ts @@ -372,11 +372,7 @@ describe('API Requests', () => { json: () => Promise.reject(new Error('parse error')) }); - try { - await apiRequest('/test'); - } catch (error) { - expect((error as { status?: number }).status).toBe(500); - } + await expect(apiRequest('/test')).rejects.toMatchObject({ status: 500 }); }); test('resolves a 204 No Content response to null', async () => { diff --git a/go.mod b/go.mod index 5fb3a2f6..abcf4fb6 100644 --- a/go.mod +++ b/go.mod @@ -59,7 +59,7 @@ require ( go.opentelemetry.io/otel/metric v1.45.0 // indirect go.opentelemetry.io/otel/trace v1.45.0 // indirect golang.org/x/crypto v0.58.0 - golang.org/x/net v0.61.0 // indirect + golang.org/x/net v0.62.0 // indirect golang.org/x/oauth2 v0.37.0 golang.org/x/sync v0.24.0 golang.org/x/sys v0.49.0 // indirect diff --git a/go.sum b/go.sum index 06deac0b..83df52e6 100644 --- a/go.sum +++ b/go.sum @@ -367,8 +367,8 @@ golang.org/x/crypto v0.58.0 h1:COkYLr4k7nDI4r6QtMML2AfzeIswWHAJthLS6K/M/54= golang.org/x/crypto v0.58.0/go.mod h1:Xh+kl5A+M0gMiWDytH2Hhr5Z7WbNCpv/eB/EFgfSk+w= golang.org/x/mod v0.42.0 h1:ICeL3t2L30ljSPHHxDnR4fqVcBt/zR2zJAb9juavmQg= golang.org/x/mod v0.42.0/go.mod h1:u5fzIrRInkL8MaO1cbRkUHT64v303YIT5KEqpNr7jRs= -golang.org/x/net v0.61.0 h1:RR+6j/BTBCrggOLugxa+66R8FvoN6MHlzDlZIbN9YFw= -golang.org/x/net v0.61.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI= +golang.org/x/net v0.62.0 h1:18ELalfM5+XNRwzM2nYqXsZQ9ByS1uhj2O/U7+5jqRY= +golang.org/x/net v0.62.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI= golang.org/x/oauth2 v0.37.0 h1:JUlcxA8oAtauLfiH8FX2/FkAWHAdi0QtGCGc+hofE98= golang.org/x/oauth2 v0.37.0/go.mod h1:IxwZNxUULJmpBFf9K/9NTMSIfZZuvuTy1gGxhigP/58= golang.org/x/sync v0.24.0 h1:iqiDrGoDgyuvAKrhLQthUEzU/R378N78gXbBjxed7oA= diff --git a/internal/mocks/bare_panic_guard_test.go b/internal/mocks/bare_panic_guard_test.go new file mode 100644 index 00000000..7ed3c4c7 --- /dev/null +++ b/internal/mocks/bare_panic_guard_test.go @@ -0,0 +1,113 @@ +package mocks + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestMocks_NoBarePanicOutsideTestReporter keeps #353 closed for code not yet +// written. A mock that panics on a test-authoring error (a mistyped .Return, +// say) ends the whole test binary: the remaining tests go unrun, TestMain +// teardown is skipped, and the output is a stack trace instead of a failure +// filed under the test's name. Such errors go through testReporter (see +// report.go), whose fail method is the one place allowed to panic, and only +// when no test is registered to report to. +// +// The check parses the package's non-test files, so a panic added to any +// mock later fails here without editing this test. +func TestMocks_NoBarePanicOutsideTestReporter(t *testing.T) { + const allowed = "testReporter.fail" + + sites := barePanicSites(t) + var offending []string + foundAllowed := false + for _, s := range sites { + if s.owner == allowed { + foundAllowed = true + continue + } + offending = append(offending, s.String()) + } + + // Self-check: if the scan stops seeing the one panic it must allow, it + // has broken and would pass vacuously. + require.True(t, foundAllowed, + "the source scan did not find the panic in %s; it has broken (or that panic moved)", allowed) + assert.Empty(t, offending, + "bare panic( in internal/mocks: route test-authoring errors through m.reporter "+ + "(wrongReturnType or fail) so they fail the registered test instead of ending the test binary:\n%s", + strings.Join(offending, "\n")) +} + +// panicSite is one call to the panic builtin and the declaration enclosing it. +type panicSite struct { + pos token.Position + owner string // "Recv.Method", "Func", or "package scope" +} + +func (s panicSite) String() string { + return s.pos.String() + " in " + s.owner +} + +// barePanicSites returns every panic(...) call in the package's non-test files. +func barePanicSites(t *testing.T) []panicSite { + t.Helper() + entries, err := os.ReadDir(".") + require.NoError(t, err) + fset := token.NewFileSet() + var sites []panicSite + for _, e := range entries { + name := e.Name() + if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + f, err := parser.ParseFile(fset, name, nil, 0) + require.NoError(t, err) + for _, decl := range f.Decls { + owner := declOwner(decl) + ast.Inspect(decl, func(n ast.Node) bool { + if isPanicCall(n) { + sites = append(sites, panicSite{pos: fset.Position(n.Pos()), owner: owner}) + } + return true + }) + } + } + return sites +} + +func isPanicCall(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return false + } + id, ok := call.Fun.(*ast.Ident) + return ok && id.Name == "panic" +} + +// declOwner names a top-level declaration: "Recv.Method" for a method, +// the function name for a function, "package scope" for anything else. +func declOwner(decl ast.Decl) string { + fd, ok := decl.(*ast.FuncDecl) + if !ok { + return "package scope" + } + if fd.Recv == nil || len(fd.Recv.List) != 1 { + return fd.Name.Name + } + recv := fd.Recv.List[0].Type + if star, ok := recv.(*ast.StarExpr); ok { + recv = star.X + } + if id, ok := recv.(*ast.Ident); ok { + return id.Name + "." + fd.Name.Name + } + return fd.Name.Name +} diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go index ea6706bb..62ef53b2 100644 --- a/internal/mocks/stores.go +++ b/internal/mocks/stores.go @@ -959,7 +959,7 @@ func (m *MockConfigStore) GetMarketplaceListingAttempt(ctx context.Context, purc if v := args.Get(0); v != nil { a, ok := v.(*config.MarketplaceListingAttempt) if !ok { - panic(fmt.Sprintf("mock: expected *config.MarketplaceListingAttempt, got %T", v)) + m.reporter.wrongReturnType("GetMarketplaceListingAttempt", "*config.MarketplaceListingAttempt", v) } attempt = a } @@ -1010,7 +1010,7 @@ func (m *MockConfigStore) ListMarketplaceListingsToPoll(ctx context.Context) ([] if v := args.Get(0); v != nil { r, ok := v.([]config.PurchaseHistoryRecord) if !ok { - panic(fmt.Sprintf("mock: expected []config.PurchaseHistoryRecord, got %T", v)) + m.reporter.wrongReturnType("ListMarketplaceListingsToPoll", "[]config.PurchaseHistoryRecord", v) } rows = r } @@ -1813,7 +1813,7 @@ func (m *MockConfigStore) ListStoredRecommendationsChunk(ctx context.Context, fi args := m.Called(ctx, filter, after, limit) v, ok := args.Get(0).(config.RecommendationChunk) if !ok { - panic(fmt.Sprintf("mock: expected config.RecommendationChunk, got %T", args.Get(0))) + m.reporter.wrongReturnType("ListStoredRecommendationsChunk", "config.RecommendationChunk", args.Get(0)) } return v, args.Error(1) } @@ -2094,7 +2094,7 @@ func (m *MockConfigStore) ListHistoryExecutionsPage(ctx context.Context, q confi } v, ok := args.Get(0).([]config.PurchaseExecution) if !ok { - panic(fmt.Sprintf("mock: expected []config.PurchaseExecution, got %T", args.Get(0))) + m.reporter.wrongReturnType("ListHistoryExecutionsPage", "[]config.PurchaseExecution", args.Get(0)) } return pageExecutionsLikeStore(v, &q), args.Error(1) } @@ -2125,7 +2125,7 @@ func (m *MockConfigStore) ListPurchaseHistoryPage(ctx context.Context, filter co v[i] = config.PurchaseHistoryPageRow{PurchaseHistoryRecord: rows[i], RowID: PurchaseHistoryFixtureRowID(i)} } default: - panic(fmt.Sprintf("mock: expected []config.PurchaseHistoryPageRow or []config.PurchaseHistoryRecord, got %T", args.Get(0))) + m.reporter.wrongReturnType("ListPurchaseHistoryPage", "[]config.PurchaseHistoryPageRow or []config.PurchaseHistoryRecord", args.Get(0)) } out := make([]config.PurchaseHistoryPageRow, 0, len(v)) for i := range v {