From 769453023811e5f8f967e9b05d312219e0c864a6 Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Thu, 19 Sep 2024 10:37:23 -0400 Subject: [PATCH 1/2] Fix flaky `RECAPTCHA_SIGN_IN` A/B test The `RECAPTCHA_SIGN_IN` has a discriminator block that does the following: 1. Return `nil` if captcha was not performed (this puts the user in the default bucket) 2. Return the user's UUID if the user is present 3. Return a random value if no user is present The implementation of the bucket method checks if the discriminator is blank and returns `nil` if it is ([ref](https://github.com/18F/identity-idp/blob/983e21648dc7695f11c37e3eff7997ef4bd90f1e/lib/ab_test.rb#L46)). The `blank?` method has a special implementation implementation for `String` ([ref](https://github.com/rails/rails/blob/dfd1e951aa1aeef06c39fffb2994db8a8fa1914f/activesupport/lib/active_support/core_ext/object/blank.rb#L141-L163)) that returns `true` if the string consists of only whitespace. The random value here was generated using `SecureRandom.gen_random`. This value was then used in the `AbTest#bucket` to randomnly sort unidentified users into AB test buckets. `SecureRandom.gen_random` returns a string of random bytes which can include `" "`, `"\n"`, `"\r"`, etc. The combination of the above meant that the discriminator could generate value that caused the `AbTest#bucket` to return `nil` for `RECAPTCHA_SIGN_IN` when it was intended to return a bucket. This lead to a flaky test. This commit changes the call to generate a random string such that it only ever generates alphanumeric strings which will never be blank. [skip changelog] --- config/initializers/ab_tests.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/ab_tests.rb b/config/initializers/ab_tests.rb index c3b33a45119..729de1443ba 100644 --- a/config/initializers/ab_tests.rb +++ b/config/initializers/ab_tests.rb @@ -78,7 +78,7 @@ def self.all elsif user user.uuid else - SecureRandom.gen_random(1) + SecureRandom.alphanumeric(1) end end.freeze end From 6c8dbbeb632283d42fa2a8400b08de769dc7e82f Mon Sep 17 00:00:00 2001 From: Jonathan Hooper Date: Thu, 19 Sep 2024 11:26:30 -0400 Subject: [PATCH 2/2] even more random ab test bucket discriminators --- config/initializers/ab_tests.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/ab_tests.rb b/config/initializers/ab_tests.rb index 729de1443ba..5a970f79907 100644 --- a/config/initializers/ab_tests.rb +++ b/config/initializers/ab_tests.rb @@ -78,7 +78,7 @@ def self.all elsif user user.uuid else - SecureRandom.alphanumeric(1) + SecureRandom.alphanumeric(8) end end.freeze end