Skip to content

Flaky spec: CollectionRetriever most_popular "makes occurrences counts available after retrieve" #903

Description

@suttondemlong

spec/services/collection_retriever_spec.rb:50 fails intermittently on master. It is not caused by any in-flight upgrade work — I measured it on master at 2 failures in 100 runs while verifying #901, and it is worth its own fix rather than a change smuggled into an upgrade PR.

Symptom

1) CollectionRetriever Tag most_popular makes occurrences counts available after retrieve
   Failure/Error: expect(min_occurrence).to be < max_occurrence

     expected: < 2
          got:   2
   # ./spec/services/collection_retriever_spec.rb:54

Cause

The setup builds 20 objects and 5 check-ins, each referencing a random 5 of them:

let(:objects) { create_list(model_slug, 20) }

before do
  5.times do
    create(:checkin, ids_key => object_ids.sample(5))
  end
end

That is 25 draws spread over 20 objects. The example then asserts the returned occurrence counts have a strict spread:

max_occurrence = subject.occurrences.to_a.first["count"]
min_occurrence = subject.occurrences.to_a.last["count"]
expect(min_occurrence).to be < max_occurrence

occurrences is capped at 10 entries (per the sibling limits results to 10 example). Whenever the ten highest-counted objects happen to tie — ten objects each drawn twice — min == max and the assertion fails. With only 25 draws over 20 objects that is an ordinary outcome, not a rare one.

Nothing here depends on gem versions: the randomness is plain Array#sample. It is also not spec-ordering related — .rspec sets no random order.

Note the sibling example on line 37 asserts the same underlying property non-strictly (be <= min_occurrence) and is not flaky. Line 54 is the only strict comparison.

Measured rate

Branch Failures
master (globalize 6.3.0, puma 5.6.8, Rails 7.1) 2 / 100
chore/globalize-7-puma-6 1 / 30

Same assertion and same values (expected: < 2, got: 2) in every case. The spec is parameterised over MODELS = [Food, Tag], so either context can trip it; the runs I captured were Tag.

Reproduce with:

cd backend
for i in $(seq 1 100); do bundle exec rspec spec/services/collection_retriever_spec.rb || echo "FAILED on run $i"; done

Suggested fix

Make the fixture data deterministic rather than loosening the assertion — the example is genuinely trying to check that counts are ordered and accurate, and that is worth keeping. Assigning object ids explicitly so the counts are known up front (a clear winner, a clear middle, a clear tail) would let the example assert exact counts instead of a statistical property, and would fix the sibling examples' reliance on the same random setup at the same time.

Loosening line 54 to be <= would stop the failures but would also stop the example from testing anything about ordering, so it is not the preferred route.

🤖 Generated with Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    language:rubyPull requests that update Ruby codetype:bugIssues that impair or prevent product functionality

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions