From d880aecadaeb68dd6119687673e75139976c6aab Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Thu, 13 Aug 2026 09:34:50 -0400 Subject: [PATCH 1/3] Reset ActiveSupport::CurrentAttributes between specs to stop test-order pollution Current.user/source is only auto-reset by the executor around real requests/jobs. Controller/view/service specs set it in the test thread with no executor to clear it, so a stale Current leaks into later examples and silently flips model behavior that branches on it (Organization affiliation-lock validation, AhoyTrackable lifecycle tracking). Mirror the existing travel_back / Warden.test_reset! hooks with an after-each reset, plus an order:defined regression guard. Co-Authored-By: Claude Opus 4.8 (1M context) --- spec/current_attributes_reset_spec.rb | 23 +++++++++++++++++++++++ spec/rails_helper.rb | 8 ++++++++ 2 files changed, 31 insertions(+) create mode 100644 spec/current_attributes_reset_spec.rb diff --git a/spec/current_attributes_reset_spec.rb b/spec/current_attributes_reset_spec.rb new file mode 100644 index 0000000000..9dde47e97a --- /dev/null +++ b/spec/current_attributes_reset_spec.rb @@ -0,0 +1,23 @@ +require "rails_helper" + +# Regression guard for test-order pollution via ActiveSupport::CurrentAttributes. +# The executor only auto-resets Current around real requests/jobs; a spec that +# sets Current in the test thread (controller/view/service specs) has nothing to +# clear it, so a stale value leaks into later examples and silently changes +# model behavior that branches on Current (Organization affiliation-lock +# validation, AhoyTrackable lifecycle tracking). rails_helper resets Current +# after every example — this proves it. order: :defined pins the two examples so +# the second reliably follows the first regardless of the suite seed. +RSpec.describe "Current attributes reset between examples", order: :defined do + it "leaves Current set within an example" do + Current.user = User.new + Current.source = "leak_probe" + + expect(Current.source).to eq "leak_probe" + end + + it "starts the next example with a clean Current" do + expect(Current.user).to be_nil + expect(Current.source).to be_nil + end +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 962224f3db..4a4482a9fd 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -53,6 +53,14 @@ # each example (no-op when time wasn't traveled). config.after { travel_back } + # ActiveSupport::CurrentAttributes (Current.user/source) is only auto-reset by + # the executor around real requests/jobs. Controller/view/service specs set it + # in the test thread with no executor to clear it, so a stale Current leaks + # into later examples and silently changes model behavior that branches on it + # (Organization affiliation-lock validation, AhoyTrackable lifecycle tracking). + # Reset after every example, like travel_back above (no-op when nothing set it). + config.after { Current.reset } + # Include pagination helper globally config.include PaginationHelpers From ffa873021dca5608c0efda8756a043337c71089b Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Thu, 13 Aug 2026 09:41:42 -0400 Subject: [PATCH 2/3] Pin address fields in upsert_address spec to fix Faker-order flake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Faker::Config uses one shared, order-seeded stream (Random.new(42)), so a factory's random country/street/zip is determined by the example's position in that stream. Under some seeds (e.g. 62433) the existing address's country came out as 'Canada' — the exact value the upsert sets — so 'Country' dropped out of the reported changes and the example failed. Pin the changed fields to fixed, distinct values so the assertion is order-independent. Co-Authored-By: Claude Opus 4.8 (1M context) --- spec/services/organization_services/upsert_address_spec.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/spec/services/organization_services/upsert_address_spec.rb b/spec/services/organization_services/upsert_address_spec.rb index 03d338769e..d2f4205250 100644 --- a/spec/services/organization_services/upsert_address_spec.rb +++ b/spec/services/organization_services/upsert_address_spec.rb @@ -87,7 +87,12 @@ end it "updates the matching city/state address in place instead of duplicating" do - existing = create(:address, addressable: organization, city: "Austin", state: "TX", primary: true) + # Pin the fields this example expects to change to values distinct from the + # upsert below. The factory otherwise fills them from Faker's shared, + # order-dependent stream, which can coincidentally match a new value (e.g. + # country "Canada") and drop it from the reported changes under some seeds. + existing = create(:address, addressable: organization, city: "Austin", state: "TX", primary: true, + street_address: "1 Old St", zip_code: "10001", country: "United States") result = described_class.call( organization: organization, From 79db1d70d3199667c96347f7af4edc759377d980 Mon Sep 17 00:00:00 2001 From: Mae Beale Date: Thu, 13 Aug 2026 10:42:01 -0400 Subject: [PATCH 3/3] Reseed Faker per example to make faked data order-independent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Faker draws from one shared global RNG stream, so a factory's value depends on how many Faker calls ran earlier in the suite — making data order-dependent and specs flaky under some seeds (the upsert_address country=Canada collision was one instance). Reseed Random.new(42) before each example so every example draws the same fixed sequence regardless of order. Faker's .unique generator remembers used values for the whole run, so a bare reseed replays them and hits RetryLimitExceeded (1782 failures); clearing it per example fixes that. Verified green on the full suite (system specs included) at seeds 11334 and 18114: 6330 examples, 0 failures. Co-Authored-By: Claude Opus 4.8 (1M context) --- spec/support/faker.rb | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/spec/support/faker.rb b/spec/support/faker.rb index 8e5b8bd2e7..699f85514d 100644 --- a/spec/support/faker.rb +++ b/spec/support/faker.rb @@ -1,5 +1,16 @@ -# give it a specific seed to ensure the same data is generated. This combined -# with running rspec with a specific seed value will ensure that specs are run -# in the same order and the same "faked" data is generated each time: giving -# us deterministic results. +# Reseed Faker's RNG before every example so each example draws from the same +# fixed sequence regardless of test order. Faker uses one shared, global stream, +# so without this a factory's "random" value depends on how many Faker calls ran +# earlier in the suite — making data order-dependent and specs flaky under some +# seeds (e.g. an address factory landing on country "Canada"). Reseeding per +# example makes the faked data deterministic AND independent of ordering. Faker::Config.random = Random.new(42) + +RSpec.configure do |config| + config.before(:each) do + Faker::Config.random = Random.new(42) + # Faker's .unique generator remembers used values for the whole run; reseeding + # to the same stream would replay them and hit RetryLimitExceeded, so clear it. + Faker::UniqueGenerator.clear + end +end