Fix flaky tests#479
Open
willnet wants to merge 1 commit into
Open
Conversation
## Summary
Fix an order-dependent PostgreSQL test failure caused by the `upsert_all` test leaving the `users` primary key sequence out of sync.
## Details
The `upsert_all with different value formats works correctly` test inserts rows with explicit primary keys:
```ruby
{ id: 1, ... }
{ id: 2, ... }
{ id: 3, ... }
```
On PostgreSQL, inserting or upserting rows with explicit IDs does not advance the table’s primary key sequence. As a result, later tests that create `User` records without specifying an ID can receive an already-used ID from the sequence, causing errors like:
```text
PG::UniqueViolation: duplicate key value violates unique constraint "users_pkey"
```
This failure depends on randomized test order. In the failing CI run, the `upsert_all` test ran before tests that performed normal `User.create!` / `save!` calls.
## Fix
Wrap the `upsert_all` test in an `ensure` block and reset the primary key sequence afterward when the adapter supports it:
```ruby
User.connection.reset_pk_sequence!(User.table_name) if User.connection.respond_to?(:reset_pk_sequence!)
```
Using `ensure` makes sure the sequence is reset even if the test fails partway through, preventing this test from leaking database state into later tests.
## Verification
Reproduced with PostgreSQL using the CI seed:
```
POSTGRES_USER=postgres \
POSTGRES_PASSWORD=postgres \
DB=postgresql \
bundle exec ruby -w -Ilib:test test/activerecord_test.rb --seed 40802
```
result example
```
1) Error:
ActiveRecordTest#test_0028_stores multiple value passed passed to new:
ActiveRecord::RecordNotUnique: PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "users_pkey"
DETAIL: Key (id)=(2) already exists.
# … snip …
2) Error:
ActiveRecordTest#test_0044_does not change by the practical same value:
ActiveRecord::RecordNotUnique: PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "users_pkey"
DETAIL: Key (id)=(3) already exists.
# … snip …
```
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix an order-dependent PostgreSQL test failure caused by the
upsert_alltest leaving theusersprimary key sequence out of sync.Details
The
upsert_all with different value formats works correctlytest inserts rows with explicit primary keys:On PostgreSQL, inserting or upserting rows with explicit IDs does not advance the table’s primary key sequence. As a result, later tests that create
Userrecords without specifying an ID can receive an already-used ID from the sequence, causing errors like:This failure depends on randomized test order. In the failing CI run, the
upsert_alltest ran before tests that performed normalUser.create!/save!calls.Fix
Wrap the
upsert_alltest in anensureblock and reset the primary key sequence afterward when the adapter supports it:Using
ensuremakes sure the sequence is reset even if the test fails partway through, preventing this test from leaking database state into later tests.Verification
Reproduced with PostgreSQL using the CI seed:
result example