From 7cb6618733536fe02c3ed90994556ebacf552777 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Fri, 31 Mar 2023 14:34:35 -0400 Subject: [PATCH 1/5] Fixed phone number throttling test This test failed because it sometimes said '9 minutes' instead of '10 minutes' We removed freeze_time, and changed the test to just check for one of the two possibilities. We don't understand why freeze_time wasn't working, but it was simple enough to fix the problem without it. --- spec/features/users/sign_up_spec.rb | 33 +++++++++++++---------------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 853bd294c36..d723019349b 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -108,30 +108,27 @@ expect(page).to have_content(I18n.t('telephony.error.friendly_message.generic')) end - scenario 'rate limits sign-up phone confirmation attempts' do + scenario 'rate limits sign-up phone confirmation attempts', js: true do allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(999) sign_up_and_set_password - freeze_time do - (IdentityConfig.store.phone_confirmation_max_attempts + 1).times do - visit phone_setup_path - fill_in 'new_phone_form_phone', with: '2025551313' - click_send_one_time_code - end + (IdentityConfig.store.phone_confirmation_max_attempts + 1).times do + visit phone_setup_path + fill_in 'new_phone_form_phone', with: '2025551313' + click_send_one_time_code + end - timeout = distance_of_time_in_words( - Throttle.attempt_window_in_minutes(:phone_confirmation).minutes, - ) + # whether it says '9 minutes' or '10 minutes' depends on how + # slowly the test runs + throttled_message = I18n.t( + 'errors.messages.phone_confirmation_throttled', + timeout: '10|9', + ) - expect(current_path).to eq(authentication_methods_setup_path) - expect(page).to have_content( - I18n.t( - 'errors.messages.phone_confirmation_throttled', - timeout: timeout, - ), - ) - end + expect(current_path).to eq(authentication_methods_setup_path) + + expect(page).to have_content(/#{throttled_message}/) end context 'with js', js: true do From d765f2ab420ca193e943253dfc848a27fdbcbe48 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Fri, 31 Mar 2023 14:48:59 -0400 Subject: [PATCH 2/5] Made test a little clearer. --- spec/features/users/sign_up_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index d723019349b..fbd6101808d 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -123,7 +123,7 @@ # slowly the test runs throttled_message = I18n.t( 'errors.messages.phone_confirmation_throttled', - timeout: '10|9', + timeout: '(10|9) minutes', ) expect(current_path).to eq(authentication_methods_setup_path) From 178c47b7bb2bdc20a7008ad51e7cbb1f99f21c1f Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 3 Apr 2023 09:34:41 -0400 Subject: [PATCH 3/5] [skip changelog] --- spec/features/users/sign_up_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index fbd6101808d..7f10be0092b 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -120,7 +120,7 @@ end # whether it says '9 minutes' or '10 minutes' depends on how - # slowly the test runs + # slowly the test runs. throttled_message = I18n.t( 'errors.messages.phone_confirmation_throttled', timeout: '(10|9) minutes', From ae1c713da7836872dd38e62705634cafefb97fd4 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 3 Apr 2023 12:11:59 -0400 Subject: [PATCH 4/5] Removed :js tag from spec We added it as a debugging step. --- spec/features/users/sign_up_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 7f10be0092b..69b75b4c49c 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -108,7 +108,7 @@ expect(page).to have_content(I18n.t('telephony.error.friendly_message.generic')) end - scenario 'rate limits sign-up phone confirmation attempts', js: true do + scenario 'rate limits sign-up phone confirmation attempts' do allow(IdentityConfig.store).to receive(:otp_delivery_blocklist_maxretry).and_return(999) sign_up_and_set_password From 5fc2687557bde17f7cf6d9c09d07e0c51025a88d Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 3 Apr 2023 13:29:09 -0400 Subject: [PATCH 5/5] Fix mysteriously broken spec --- spec/views/layouts/application.html.erb_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/views/layouts/application.html.erb_spec.rb b/spec/views/layouts/application.html.erb_spec.rb index 14e1dc36ff7..94d95663f0a 100644 --- a/spec/views/layouts/application.html.erb_spec.rb +++ b/spec/views/layouts/application.html.erb_spec.rb @@ -167,6 +167,7 @@ it 'it render the new relic javascript' do allow(IdentityConfig.store).to receive(:newrelic_browser_key).and_return('foo') allow(IdentityConfig.store).to receive(:newrelic_browser_app_id).and_return('foo') + allow(BrowserSupport).to receive(:supported?).and_return(true) render