reports: switch test fixtures from Amazon to Etsy to fix test isolation
domains/reports' fixtures used Amazon as their example platform - same as domains/amazon's own tests. Both packages' test binaries run concurrently under go test ./... by default, both wrote to the shared mock.raw_shop_events table with platform='amazon', and the DB trigger routed those into mock.shop_amazon_events - the exact table domains/amazon's background-processing tests poll and assert on. Confirmed directly: running the two packages together, domains/amazon's TestProcessUnprocessedEvents_RetriesUnackedNotification picked up 4 events instead of 1, three of them from domains/reports' fixture shop. Switched to Etsy instead (test-only change, no production code touched). Verified the casing first since it mattered here: accounts.Etsy's Go value is "Etsy" (capital), and separately Etsy's raw_shop_events trigger checks for lowercase 'etsy' - but the view these tests actually depend on (mock.shop_etsy_listing_event_sequence) filters on 'Etsy', matching the Go constant, confirmed via pg_get_viewdef. So the fixtures work correctly and, as a side effect, never fire the lowercase-gated trigger at all - keeping mock.shop_etsy_events untouched by these tests regardless. Combined with the previous commit's goroutine-leak fix, go test ./... and make test are both reliably green as single commands again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XEDaCB7C2NEBgyvqEtZuxY
This commit is contained in:
@@ -17,11 +17,20 @@ import (
|
|||||||
"ruben/inventory2/internal/testdb"
|
"ruben/inventory2/internal/testdb"
|
||||||
)
|
)
|
||||||
|
|
||||||
// setupAmazonMockShop creates a fresh account and an Amazon mock shop for
|
// setupEtsyMockShop creates a fresh account and an Etsy mock shop for it,
|
||||||
// it, and registers cleanup for every row it creates, in FK-safe order
|
// and registers cleanup for every row it creates, in FK-safe order
|
||||||
// (shop_amazon[_events] -> mock.accounts -> accounts; oauth_users cleanup
|
// (shop_etsy[_events] -> mock.accounts -> accounts; oauth_users cleanup is
|
||||||
// is handled by testdb.SeedOAuthUser itself).
|
// handled by testdb.SeedOAuthUser itself).
|
||||||
func setupAmazonMockShop(t *testing.T, pool *pgxpool.Pool, acctStore *accounts.Store) (acctID int64, shopID string) {
|
//
|
||||||
|
// Etsy (not Amazon) is deliberately used here: domains/amazon's tests
|
||||||
|
// process every unprocessed row in mock.shop_amazon_events system-wide
|
||||||
|
// (that's correct production behavior for a background worker, not a
|
||||||
|
// bug), so any test that writes Amazon-platform mock events risks being
|
||||||
|
// picked up by domains/amazon's tests when the two packages' test
|
||||||
|
// binaries run concurrently (go test ./... does this by default). Using
|
||||||
|
// a different platform here keeps this package's fixtures completely off
|
||||||
|
// domains/amazon's tables and NOTIFY channel.
|
||||||
|
func setupEtsyMockShop(t *testing.T, pool *pgxpool.Pool, acctStore *accounts.Store) (acctID int64, shopID string) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
@@ -34,18 +43,15 @@ func setupAmazonMockShop(t *testing.T, pool *pgxpool.Pool, acctStore *accounts.S
|
|||||||
pool.Exec(context.Background(), "DELETE FROM accounts WHERE account_id = $1", acct.AccountID)
|
pool.Exec(context.Background(), "DELETE FROM accounts WHERE account_id = $1", acct.AccountID)
|
||||||
})
|
})
|
||||||
|
|
||||||
id, err := acctStore.CreateMockShop(ctx, acct.AccountID, accounts.Amazon, "Test Shop")
|
id, err := acctStore.CreateMockShop(ctx, acct.AccountID, accounts.Etsy, "Test Shop")
|
||||||
require.NoError(t, err, "CreateMockShop()")
|
require.NoError(t, err, "CreateMockShop()")
|
||||||
shopID = id.String()
|
shopID = id.String()
|
||||||
|
|
||||||
t.Cleanup(func() {
|
t.Cleanup(func() {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
// a raw_shop_events insert for platform='amazon' fires a trigger
|
pool.Exec(ctx, "DELETE FROM mock.shop_etsy_events WHERE shop_id = $1", shopID)
|
||||||
// that also populates shop_amazon_events - clean that up too even
|
|
||||||
// though these tests don't touch the amazon event processor.
|
|
||||||
pool.Exec(ctx, "DELETE FROM mock.shop_amazon_events WHERE shop_id = $1", shopID)
|
|
||||||
pool.Exec(ctx, "DELETE FROM mock.raw_shop_events WHERE shop_id = $1", shopID)
|
pool.Exec(ctx, "DELETE FROM mock.raw_shop_events WHERE shop_id = $1", shopID)
|
||||||
pool.Exec(ctx, "DELETE FROM mock.shop_amazon WHERE account_id = $1", acct.AccountID)
|
pool.Exec(ctx, "DELETE FROM mock.shop_etsy WHERE account_id = $1", acct.AccountID)
|
||||||
pool.Exec(ctx, "DELETE FROM mock.accounts WHERE account_id = $1", acct.AccountID)
|
pool.Exec(ctx, "DELETE FROM mock.accounts WHERE account_id = $1", acct.AccountID)
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -56,8 +62,8 @@ func insertRawShopEvent(t *testing.T, pool *pgxpool.Pool, shopID, eventID string
|
|||||||
t.Helper()
|
t.Helper()
|
||||||
_, err := pool.Exec(context.Background(), `
|
_, err := pool.Exec(context.Background(), `
|
||||||
INSERT INTO mock.raw_shop_events (platform, shop_id, event_timestamp, event_id, raw_payload)
|
INSERT INTO mock.raw_shop_events (platform, shop_id, event_timestamp, event_id, raw_payload)
|
||||||
VALUES ('amazon', $1, $2, $3, $4::jsonb)
|
VALUES ($1, $2, $3, $4, $5::jsonb)
|
||||||
`, shopID, ts, eventID, payload)
|
`, string(accounts.Etsy), shopID, ts, eventID, payload)
|
||||||
require.NoError(t, err, "insert raw shop event")
|
require.NoError(t, err, "insert raw shop event")
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -67,14 +73,14 @@ func TestGetRawShopEvents(t *testing.T) {
|
|||||||
reportsStore := reports.NewStore(testdb.Logger(), pool, acctStore)
|
reportsStore := reports.NewStore(testdb.Logger(), pool, acctStore)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
acctID, shopID := setupAmazonMockShop(t, pool, acctStore)
|
acctID, shopID := setupEtsyMockShop(t, pool, acctStore)
|
||||||
|
|
||||||
older := time.Now().Add(-time.Hour).UTC()
|
older := time.Now().Add(-time.Hour).UTC()
|
||||||
newer := time.Now().UTC()
|
newer := time.Now().UTC()
|
||||||
insertRawShopEvent(t, pool, shopID, "evt-1", older, `{"n":1}`)
|
insertRawShopEvent(t, pool, shopID, "evt-1", older, `{"n":1}`)
|
||||||
insertRawShopEvent(t, pool, shopID, "evt-2", newer, `{"n":2}`)
|
insertRawShopEvent(t, pool, shopID, "evt-2", newer, `{"n":2}`)
|
||||||
|
|
||||||
got, err := reportsStore.GetRawShopEvents(ctx, acctID, accounts.Amazon, shopID)
|
got, err := reportsStore.GetRawShopEvents(ctx, acctID, accounts.Etsy, shopID)
|
||||||
require.NoError(t, err, "GetRawShopEvents()")
|
require.NoError(t, err, "GetRawShopEvents()")
|
||||||
require.Len(t, got, 2, "GetRawShopEvents()")
|
require.Len(t, got, 2, "GetRawShopEvents()")
|
||||||
|
|
||||||
@@ -82,7 +88,7 @@ func TestGetRawShopEvents(t *testing.T) {
|
|||||||
assert.Equal(t, "evt-2", got[0].EventID, "GetRawShopEvents()[0]")
|
assert.Equal(t, "evt-2", got[0].EventID, "GetRawShopEvents()[0]")
|
||||||
assert.Equal(t, "evt-1", got[1].EventID, "GetRawShopEvents()[1]")
|
assert.Equal(t, "evt-1", got[1].EventID, "GetRawShopEvents()[1]")
|
||||||
|
|
||||||
assert.Equal(t, accounts.Amazon, got[0].Platform, "got[0].Platform")
|
assert.Equal(t, accounts.Etsy, got[0].Platform, "got[0].Platform")
|
||||||
assert.Equal(t, shopID, got[0].ShopID, "got[0].ShopID")
|
assert.Equal(t, shopID, got[0].ShopID, "got[0].ShopID")
|
||||||
|
|
||||||
var payload struct{ N int }
|
var payload struct{ N int }
|
||||||
@@ -96,9 +102,9 @@ func TestGetRawShopEvents_NoEvents(t *testing.T) {
|
|||||||
reportsStore := reports.NewStore(testdb.Logger(), pool, acctStore)
|
reportsStore := reports.NewStore(testdb.Logger(), pool, acctStore)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
acctID, shopID := setupAmazonMockShop(t, pool, acctStore)
|
acctID, shopID := setupEtsyMockShop(t, pool, acctStore)
|
||||||
|
|
||||||
got, err := reportsStore.GetRawShopEvents(ctx, acctID, accounts.Amazon, shopID)
|
got, err := reportsStore.GetRawShopEvents(ctx, acctID, accounts.Etsy, shopID)
|
||||||
require.NoError(t, err, "GetRawShopEvents()")
|
require.NoError(t, err, "GetRawShopEvents()")
|
||||||
assert.Empty(t, got, "GetRawShopEvents()")
|
assert.Empty(t, got, "GetRawShopEvents()")
|
||||||
}
|
}
|
||||||
@@ -118,6 +124,6 @@ func TestGetRawShopEvents_UnknownShop(t *testing.T) {
|
|||||||
pool.Exec(context.Background(), "DELETE FROM accounts WHERE account_id = $1", acct.AccountID)
|
pool.Exec(context.Background(), "DELETE FROM accounts WHERE account_id = $1", acct.AccountID)
|
||||||
})
|
})
|
||||||
|
|
||||||
_, err = reportsStore.GetRawShopEvents(ctx, acct.AccountID, accounts.Amazon, "no-such-shop-"+uuid.NewString())
|
_, err = reportsStore.GetRawShopEvents(ctx, acct.AccountID, accounts.Etsy, "no-such-shop-"+uuid.NewString())
|
||||||
require.ErrorIs(t, err, consts.ErrNotFound, "GetRawShopEvents()")
|
require.ErrorIs(t, err, consts.ErrNotFound, "GetRawShopEvents()")
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -13,11 +13,10 @@ import (
|
|||||||
"ruben/inventory2/internal/testdb"
|
"ruben/inventory2/internal/testdb"
|
||||||
)
|
)
|
||||||
|
|
||||||
// createAmazonListing creates a listing with the given base count in
|
// createEtsyListing creates a listing with the given base count in shopID
|
||||||
// shopID and registers its cleanup. Must be called after
|
// and registers its cleanup. Must be called after setupEtsyMockShop, so
|
||||||
// setupAmazonMockShop, so cleanup order (LIFO) deletes the listing before
|
// cleanup order (LIFO) deletes the listing before the shop it belongs to.
|
||||||
// the shop it belongs to.
|
func createEtsyListing(t *testing.T, acctStore *accounts.Store, acctID int64, shopID string, baseCount int64) (listingID string) {
|
||||||
func createAmazonListing(t *testing.T, acctStore *accounts.Store, acctID int64, shopID string, baseCount int64) (listingID string) {
|
|
||||||
t.Helper()
|
t.Helper()
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
@@ -27,7 +26,7 @@ func createAmazonListing(t *testing.T, acctStore *accounts.Store, acctID int64,
|
|||||||
AccountShopListingIDs: accounts.AccountShopListingIDs{
|
AccountShopListingIDs: accounts.AccountShopListingIDs{
|
||||||
AccountShopIDs: accounts.AccountShopIDs{
|
AccountShopIDs: accounts.AccountShopIDs{
|
||||||
AccountIDs: accounts.AccountIDs{AccountID: acctID},
|
AccountIDs: accounts.AccountIDs{AccountID: acctID},
|
||||||
Platform: accounts.Amazon,
|
Platform: accounts.Etsy,
|
||||||
ShopID: shopID,
|
ShopID: shopID,
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@@ -40,7 +39,7 @@ func createAmazonListing(t *testing.T, acctStore *accounts.Store, acctID int64,
|
|||||||
|
|
||||||
t.Cleanup(func() {
|
t.Cleanup(func() {
|
||||||
pool.Exec(context.Background(), `
|
pool.Exec(context.Background(), `
|
||||||
DELETE FROM mock.shop_amazon_listings WHERE shop_id = $1 AND listing_id = $2
|
DELETE FROM mock.shop_etsy_listings WHERE shop_id = $1 AND listing_id = $2
|
||||||
`, shopID, listing.ListingID)
|
`, shopID, listing.ListingID)
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -48,8 +47,8 @@ func createAmazonListing(t *testing.T, acctStore *accounts.Store, acctID int64,
|
|||||||
}
|
}
|
||||||
|
|
||||||
// TestGetListingCountsOverTime exercises the actual running-count logic,
|
// TestGetListingCountsOverTime exercises the actual running-count logic,
|
||||||
// which lives in a recursive SQL view (mock.shop_amazon_listing_counts,
|
// which lives in a recursive SQL view (mock.shop_etsy_listing_counts,
|
||||||
// built on mock.shop_amazon_listing_event_sequence) rather than in Go: it
|
// built on mock.shop_etsy_listing_event_sequence) rather than in Go: it
|
||||||
// starts from the listing's base count and walks mock.raw_shop_events in
|
// starts from the listing's base count and walks mock.raw_shop_events in
|
||||||
// order, applying each as a delta (sale/refund) or an absolute reset
|
// order, applying each as a delta (sale/refund) or an absolute reset
|
||||||
// (inventory-reset). Uses the real SaveNewMock* methods to write those
|
// (inventory-reset). Uses the real SaveNewMock* methods to write those
|
||||||
@@ -61,17 +60,17 @@ func TestGetListingCountsOverTime(t *testing.T) {
|
|||||||
reportsStore := reports.NewStore(testdb.Logger(), pool, acctStore)
|
reportsStore := reports.NewStore(testdb.Logger(), pool, acctStore)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
acctID, shopID := setupAmazonMockShop(t, pool, acctStore)
|
acctID, shopID := setupEtsyMockShop(t, pool, acctStore)
|
||||||
listingID := createAmazonListing(t, acctStore, acctID, shopID, 100)
|
listingID := createEtsyListing(t, acctStore, acctID, shopID, 100)
|
||||||
|
|
||||||
_, err := acctStore.SaveNewMockSale(ctx, acctID, accounts.Amazon, shopID, listingID, 10)
|
_, err := acctStore.SaveNewMockSale(ctx, acctID, accounts.Etsy, shopID, listingID, 10)
|
||||||
require.NoError(t, err, "SaveNewMockSale()")
|
require.NoError(t, err, "SaveNewMockSale()")
|
||||||
_, err = acctStore.SaveNewMockRefund(ctx, acctID, accounts.Amazon, shopID, listingID, 5)
|
_, err = acctStore.SaveNewMockRefund(ctx, acctID, accounts.Etsy, shopID, listingID, 5)
|
||||||
require.NoError(t, err, "SaveNewMockRefund()")
|
require.NoError(t, err, "SaveNewMockRefund()")
|
||||||
_, err = acctStore.SaveNewMockInventoryReset(ctx, acctID, accounts.Amazon, shopID, listingID, 50)
|
_, err = acctStore.SaveNewMockInventoryReset(ctx, acctID, accounts.Etsy, shopID, listingID, 50)
|
||||||
require.NoError(t, err, "SaveNewMockInventoryReset()")
|
require.NoError(t, err, "SaveNewMockInventoryReset()")
|
||||||
|
|
||||||
got, err := reportsStore.GetListingCountsOverTime(ctx, acctID, accounts.Amazon, shopID, listingID)
|
got, err := reportsStore.GetListingCountsOverTime(ctx, acctID, accounts.Etsy, shopID, listingID)
|
||||||
require.NoError(t, err, "GetListingCountsOverTime()")
|
require.NoError(t, err, "GetListingCountsOverTime()")
|
||||||
|
|
||||||
wantCounts := []int64{100, 110, 105, 50}
|
wantCounts := []int64{100, 110, 105, 50}
|
||||||
@@ -95,9 +94,9 @@ func TestGetListingCountsOverTime_UnknownListing(t *testing.T) {
|
|||||||
reportsStore := reports.NewStore(testdb.Logger(), pool, acctStore)
|
reportsStore := reports.NewStore(testdb.Logger(), pool, acctStore)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
acctID, shopID := setupAmazonMockShop(t, pool, acctStore)
|
acctID, shopID := setupEtsyMockShop(t, pool, acctStore)
|
||||||
|
|
||||||
_, err := reportsStore.GetListingCountsOverTime(ctx, acctID, accounts.Amazon, shopID, "no-such-listing")
|
_, err := reportsStore.GetListingCountsOverTime(ctx, acctID, accounts.Etsy, shopID, "no-such-listing")
|
||||||
require.ErrorIs(t, err, consts.ErrNotFound, "GetListingCountsOverTime()")
|
require.ErrorIs(t, err, consts.ErrNotFound, "GetListingCountsOverTime()")
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -107,19 +106,19 @@ func TestGetListingCountsReport(t *testing.T) {
|
|||||||
reportsStore := reports.NewStore(testdb.Logger(), pool, acctStore)
|
reportsStore := reports.NewStore(testdb.Logger(), pool, acctStore)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
acctID, shopID := setupAmazonMockShop(t, pool, acctStore)
|
acctID, shopID := setupEtsyMockShop(t, pool, acctStore)
|
||||||
listingID := createAmazonListing(t, acctStore, acctID, shopID, 100)
|
listingID := createEtsyListing(t, acctStore, acctID, shopID, 100)
|
||||||
|
|
||||||
_, err := acctStore.SaveNewMockSale(ctx, acctID, accounts.Amazon, shopID, listingID, 10)
|
_, err := acctStore.SaveNewMockSale(ctx, acctID, accounts.Etsy, shopID, listingID, 10)
|
||||||
require.NoError(t, err, "SaveNewMockSale()")
|
require.NoError(t, err, "SaveNewMockSale()")
|
||||||
_, err = acctStore.SaveNewMockInventoryReset(ctx, acctID, accounts.Amazon, shopID, listingID, 20)
|
_, err = acctStore.SaveNewMockInventoryReset(ctx, acctID, accounts.Etsy, shopID, listingID, 20)
|
||||||
require.NoError(t, err, "SaveNewMockInventoryReset()")
|
require.NoError(t, err, "SaveNewMockInventoryReset()")
|
||||||
|
|
||||||
report, err := reportsStore.GetListingCountsReport(ctx, acctID, accounts.Amazon, shopID, listingID)
|
report, err := reportsStore.GetListingCountsReport(ctx, acctID, accounts.Etsy, shopID, listingID)
|
||||||
require.NoError(t, err, "GetListingCountsReport()")
|
require.NoError(t, err, "GetListingCountsReport()")
|
||||||
|
|
||||||
assert.Equal(t, acctID, report.AccountID, "report.AccountID")
|
assert.Equal(t, acctID, report.AccountID, "report.AccountID")
|
||||||
assert.Equal(t, accounts.Amazon, report.Platform, "report.Platform")
|
assert.Equal(t, accounts.Etsy, report.Platform, "report.Platform")
|
||||||
assert.Equal(t, shopID, report.ShopID, "report.ShopID")
|
assert.Equal(t, shopID, report.ShopID, "report.ShopID")
|
||||||
assert.Equal(t, listingID, report.ListingID, "report.ListingID")
|
assert.Equal(t, listingID, report.ListingID, "report.ListingID")
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,22 @@
|
|||||||
|
# Work Summary — 2026-08-06 20:44
|
||||||
|
|
||||||
|
## Task
|
||||||
|
Follow-up from the testify migration: fix the remaining test-isolation failure between `domains/amazon` and `domains/reports` (the goroutine-leak half of the original `go test ./...` hang was already fixed and committed separately as `7c65f92`).
|
||||||
|
|
||||||
|
## Root cause
|
||||||
|
`domains/reports`' fixtures used `accounts.Amazon` as their example platform - same as `domains/amazon`'s own tests. Both packages' test binaries run concurrently under `go test ./...` (Go's default), both write to the shared `mock.raw_shop_events` table with `platform='amazon'`, and the DB trigger routes those into `mock.shop_amazon_events`, the exact table `domains/amazon`'s background-processing tests poll and assert on. Confirmed directly: running the two packages together, `domains/amazon`'s `TestProcessUnprocessedEvents_RetriesUnackedNotification` picked up 4 events instead of 1, three of them from `domains/reports`' fixture shop.
|
||||||
|
|
||||||
|
## Fix
|
||||||
|
Switched `domains/reports`' fixtures from Amazon to Etsy (renamed `setupAmazonMockShop`→`setupEtsyMockShop`, `createAmazonListing`→`createEtsyListing`, and all table names/platform constants throughout `domains/reports/events_test.go` and `domains/reports/reports_test.go`). Test-only change, no production code touched.
|
||||||
|
|
||||||
|
Verified the exact casing before switching, since it mattered: `accounts.Etsy`'s Go value is `"Etsy"` (capital E) unlike most other platform constants (`"amazon"`, `"big_cartel"`, etc., all lowercase) - and separately, Etsy's `mock.raw_shop_events` *trigger* (which routes into `mock.shop_etsy_events`, the table `domains/amazon`-style processors would use) checks for lowercase `'etsy'`. Confirmed via `pg_get_viewdef` that the *listing-counts view* (`mock.shop_etsy_listing_event_sequence`, what these tests actually depend on) filters on `'Etsy'` (capital), matching the Go constant correctly - so the fixtures work correctly, and as a side effect never fire the lowercase-gated trigger at all, keeping `mock.shop_etsy_events` completely untouched by these tests regardless.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
- `go build ./...` / `go vet ./...` clean.
|
||||||
|
- `domains/reports` alone: all 6 tests still pass.
|
||||||
|
- `domains/amazon` + `domains/reports` together, 8x repeated (`-race -count=1`): all clean, no failures.
|
||||||
|
- `go test ./... -race` (the exact command that used to hang before the goroutine-leak fix, and would still have failed on the isolation issue afterward): now green as one command, no special-casing needed.
|
||||||
|
- `make test`: green, zero leftover rows in `accounts`, `mock.accounts`, `mock.raw_shop_events` afterward.
|
||||||
|
|
||||||
|
## Follow-ups / not done here
|
||||||
|
None specific to this fix. Combined with the goroutine-leak fix (`7c65f92`), `go test ./...` / `make test` are both reliably green as single commands again - the workaround of running `domains/amazon` separately from everything else is no longer needed.
|
||||||
Reference in New Issue
Block a user