fix(tests): resolve flaky integration tests caused by slug collisions#128
fix(tests): resolve flaky integration tests caused by slug collisions#128iulspop wants to merge 1 commit intojanhesters:mainfrom
Conversation
3439d5c to
158288f
Compare
Factory-generated organization slugs now include a unique cuid suffix to prevent collisions from faker's limited company name pool. Tests that assert on route-produced slugs use unique names with createId(). Global setup properly awaits Stripe seeding instead of fire-and-forget. Closes janhesters#127 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
158288f to
bdef9f0
Compare
janhesters
left a comment
There was a problem hiding this comment.
@iulspop thank you so much for this PR 🙌
I really appreciate you digging into the flaky tests.
That said, I think we should investigate this a bit more deeply and see if we can arrive at a more elegant fix.
My gut feeling is that the underlying issue might be in our test setup rather than the individual test cases. Instead of working around it in the tests, I’d prefer to understand what’s actually causing the collisions.
A couple of thoughts:
-
Regarding slug collisions:
If we’re seeing slug collisions, that suggests our middleware isn’t doing what it’s supposed to. We have logic that should prevent collisions (see here). If that’s failing, that’s something we should address directly. -
Regarding name collisions:
Did you notice these collisions happening within the same test run, or only across different runs?- If it’s across runs, that could point to issues in our cleanup logic. Ideally, all integration tests with Vitest should use
onTestFinishedto clean up any data they create. Our Playwright E2E tests also usually have explicit teardown logic at the end. - If it’s within the same run, then maybe the right fix is to tweak
createPopulatedOrganization()so it appends theidto the organization name by default, rather than adjusting individual tests.
- If it’s across runs, that could point to issues in our cleanup logic. Ideally, all integration tests with Vitest should use
Would you be willing to take a closer look and help us figure out what’s really going on here?
Yes, I will and get back to you. ETA a week. |
|
@iulspop Sounds good, thank you! |
Summary
createId()so route-produced slugs don't collide with stale dataawaitglobal setup'sensureStripeProductsAndPricesExist()instead of fire-and-forgetCloses #127
Test plan
🤖 Generated with Claude Code