DEV: Bump Capybara.default_max_wait_time to 20 on CI (#32143)

There are flaky system tests that have been exhausting the 10 seconds
default max wait time which we have set previously on CI. However, I
have yet to be able to figure out why and lack the tools to be able to
figure out why. The hope here is that the upcoming playwright driver
will provide us with better tooling to debug such problems.

I've attempted to use `Capybara::Session#using_wait_time` by there is
some race condition going on where the session's default max wait time
is sometimes not set properly. I can't figure out why and have spent too
much time trying to figure out why. Instead, I will just bump up the
`default_max_wait_time` to `20`. This may the build take longer when
there are test failures but it is a trade-off we will make right now.

.github/workflows/tests.yml

@@ -38,7 +38,7 @@ jobs:
       PGUSER: discourse
       PGPASSWORD: discourse
       USES_PARALLEL_DATABASES: ${{ matrix.build_type == 'backend' || matrix.build_type == 'system' }}
-      CAPYBARA_DEFAULT_MAX_WAIT_TIME: 10
+      CAPYBARA_DEFAULT_MAX_WAIT_TIME: 20
       MINIO_RUNNER_LOG_LEVEL: DEBUG
       DISCOURSE_TURBO_RSPEC_RETRY_AND_LOG_FLAKY_TESTS: ${{ (matrix.build_type == 'system' || matrix.build_type == 'backend') && '1' }}
       CHEAP_SOURCE_MAPS: "1"

spec/system/email_change_spec.rb

@@ -71,46 +71,43 @@ describe "Changing email", type: :system do
   end
 
   it "works when user has webauthn 2fa" do
-    # Test is flaky so trying with a longer wait time as a workaround
-    using_wait_time(Capybara.default_max_wait_time * 2) do
-      begin
-        # enforced 2FA flow needs a user created > 5 minutes ago
-        user.created_at = 6.minutes.ago
-        user.save!
+    begin
+      # enforced 2FA flow needs a user created > 5 minutes ago
+      user.created_at = 6.minutes.ago
+      user.save!
 
-        sign_in user
+      sign_in user
 
-        DiscourseWebauthn.stubs(:origin).returns(current_host + ":" + Capybara.server_port.to_s)
-        options =
-          ::Selenium::WebDriver::VirtualAuthenticatorOptions.new(
-            user_verification: true,
-            user_verified: true,
-            resident_key: true,
-          )
-        authenticator = page.driver.browser.add_virtual_authenticator(options)
+      DiscourseWebauthn.stubs(:origin).returns(current_host + ":" + Capybara.server_port.to_s)
+      options =
+        ::Selenium::WebDriver::VirtualAuthenticatorOptions.new(
+          user_verification: true,
+          user_verified: true,
+          resident_key: true,
+        )
+      authenticator = page.driver.browser.add_virtual_authenticator(options)
 
-        user_preferences_security_page.visit(user)
-        user_preferences_security_page.visit_second_factor(user, password)
+      user_preferences_security_page.visit(user)
+      user_preferences_security_page.visit_second_factor(user, password)
 
-        find(".security-key .new-security-key").click
-        expect(user_preferences_security_page).to have_css("input#security-key-name")
+      find(".security-key .new-security-key").click
+      expect(user_preferences_security_page).to have_css("input#security-key-name")
 
-        find(".d-modal__body input#security-key-name").fill_in(with: "First Key")
-        find(".add-security-key").click
+      find(".d-modal__body input#security-key-name").fill_in(with: "First Key")
+      find(".add-security-key").click
 
-        expect(user_preferences_security_page).to have_css(".security-key .second-factor-item")
+      expect(user_preferences_security_page).to have_css(".security-key .second-factor-item")
 
-        visit generate_confirm_link
+      visit generate_confirm_link
 
-        find(".confirm-new-email .btn-primary").click
-        find("#security-key-authenticate-button").click
+      find(".confirm-new-email .btn-primary").click
+      find("#security-key-authenticate-button").click
 
-        try_until_success(timeout: Capybara.default_max_wait_time * 2) do
-          expect(user.reload.primary_email.email).to eq(new_email)
-        end
-      ensure
-        authenticator&.remove!
+      try_until_success(timeout: Capybara.default_max_wait_time * 2) do
+        expect(user.reload.primary_email.email).to eq(new_email)
       end
+    ensure
+      authenticator&.remove!
     end
   end
 

spec/system/forgot_password_spec.rb

@@ -42,14 +42,11 @@ shared_examples "forgot password scenarios" do
 
   context "when user does not have any multi-factor authentication configured" do
     it "should allow a user to reset their password" do
-      # Test is flaky so trying with a longer wait time as a workaround
-      using_wait_time(Capybara.default_max_wait_time * 2) do
-        visit_reset_password_link
+      visit_reset_password_link
 
-        user_reset_password_page.fill_in_new_password("newsuperpassword").submit_new_password
+      user_reset_password_page.fill_in_new_password("newsuperpassword").submit_new_password
 
-        expect(user_reset_password_page).to have_logged_in_user
-      end
+      expect(user_reset_password_page).to have_logged_in_user
     end
   end
 
@@ -58,46 +55,40 @@ shared_examples "forgot password scenarios" do
       fab!(:user_second_factor_totp) { Fabricate(:user_second_factor_totp, user:) }
 
       it "should allow a user to reset password with TOTP" do
-        # Test is flaky so trying with a longer wait time as a workaround
-        using_wait_time(Capybara.default_max_wait_time * 2) do
-          visit_reset_password_link
+        visit_reset_password_link
 
-          expect(user_reset_password_page).to have_no_toggle_button_to_second_factor_form
+        expect(user_reset_password_page).to have_no_toggle_button_to_second_factor_form
 
-          user_reset_password_page
-            .fill_in_totp(ROTP::TOTP.new(user_second_factor_totp.data).now)
-            .submit_totp
-            .fill_in_new_password("newsuperpassword")
-            .submit_new_password
+        user_reset_password_page
+          .fill_in_totp(ROTP::TOTP.new(user_second_factor_totp.data).now)
+          .submit_totp
+          .fill_in_new_password("newsuperpassword")
+          .submit_new_password
 
-          expect(user_reset_password_page).to have_logged_in_user
-        end
+        expect(user_reset_password_page).to have_logged_in_user
       end
     end
 
     context "when user only has security key configured" do
       it "should allow a user to reset password with a security key" do
-        # Test is flaky so trying with a longer wait time as a workaround
-        using_wait_time(Capybara.default_max_wait_time * 2) do
-          authenticator =
-            page.driver.browser.add_virtual_authenticator(
-              Selenium::WebDriver::VirtualAuthenticatorOptions.new,
-            )
+        authenticator =
+          page.driver.browser.add_virtual_authenticator(
+            Selenium::WebDriver::VirtualAuthenticatorOptions.new,
+          )
 
-          create_user_security_key(user)
+        create_user_security_key(user)
 
-          visit_reset_password_link
+        visit_reset_password_link
 
-          expect(user_reset_password_page).to have_no_toggle_button_to_second_factor_form
+        expect(user_reset_password_page).to have_no_toggle_button_to_second_factor_form
 
-          user_reset_password_page.submit_security_key
+        user_reset_password_page.submit_security_key
 
-          user_reset_password_page.fill_in_new_password("newsuperpassword").submit_new_password
+        user_reset_password_page.fill_in_new_password("newsuperpassword").submit_new_password
 
-          expect(user_reset_password_page).to have_logged_in_user
-        ensure
-          authenticator.remove!
-        end
+        expect(user_reset_password_page).to have_logged_in_user
+      ensure
+        authenticator.remove!
       end
     end
 
@@ -106,19 +97,16 @@ shared_examples "forgot password scenarios" do
       fab!(:user_second_factor_totp) { Fabricate(:user_second_factor_totp, user:) }
 
       it "should allow a user to reset password with backup code" do
-        # Test is flaky so trying with a longer wait time as a workaround
-        using_wait_time(Capybara.default_max_wait_time * 2) do
-          visit_reset_password_link
+        visit_reset_password_link
 
-          user_reset_password_page
-            .use_backup_codes
-            .fill_in_backup_code("iAmValidBackupCode")
-            .submit_backup_code
-            .fill_in_new_password("newsuperpassword")
-            .submit_new_password
+        user_reset_password_page
+          .use_backup_codes
+          .fill_in_backup_code("iAmValidBackupCode")
+          .submit_backup_code
+          .fill_in_new_password("newsuperpassword")
+          .submit_new_password
 
-          expect(user_reset_password_page).to have_logged_in_user
-        end
+        expect(user_reset_password_page).to have_logged_in_user
       end
     end
 
@@ -126,31 +114,28 @@ shared_examples "forgot password scenarios" do
       fab!(:user_second_factor_backup) { Fabricate(:user_second_factor_backup, user:) }
 
       it "should allow a user to reset password with backup code instead of security key" do
-        # Test is flaky so trying with a longer wait time as a workaround
-        using_wait_time(Capybara.default_max_wait_time * 2) do
-          authenticator =
-            page.driver.browser.add_virtual_authenticator(
-              Selenium::WebDriver::VirtualAuthenticatorOptions.new,
-            )
+        authenticator =
+          page.driver.browser.add_virtual_authenticator(
+            Selenium::WebDriver::VirtualAuthenticatorOptions.new,
+          )
 
-          create_user_security_key(user)
+        create_user_security_key(user)
 
-          visit_reset_password_link
+        visit_reset_password_link
 
-          user_reset_password_page.try_another_way
+        user_reset_password_page.try_another_way
 
-          expect(user_reset_password_page).to have_no_toggle_button_in_second_factor_form
+        expect(user_reset_password_page).to have_no_toggle_button_in_second_factor_form
 
-          user_reset_password_page
-            .fill_in_backup_code("iAmValidBackupCode")
-            .submit_backup_code
-            .fill_in_new_password("newsuperpassword")
-            .submit_new_password
+        user_reset_password_page
+          .fill_in_backup_code("iAmValidBackupCode")
+          .submit_backup_code
+          .fill_in_new_password("newsuperpassword")
+          .submit_new_password
 
-          expect(user_reset_password_page).to have_logged_in_user
-        ensure
-          authenticator.remove!
-        end
+        expect(user_reset_password_page).to have_logged_in_user
+      ensure
+        authenticator.remove!
       end
     end
 
@@ -159,31 +144,28 @@ shared_examples "forgot password scenarios" do
       fab!(:user_second_factor_backup) { Fabricate(:user_second_factor_backup, user:) }
 
       it "should allow a user to toggle from security key to TOTP and between TOTP and backup codes" do
-        # Test is flaky so trying with a longer wait time as a workaround
-        using_wait_time(Capybara.default_max_wait_time * 2) do
-          authenticator =
-            page.driver.browser.add_virtual_authenticator(
-              Selenium::WebDriver::VirtualAuthenticatorOptions.new,
-            )
+        authenticator =
+          page.driver.browser.add_virtual_authenticator(
+            Selenium::WebDriver::VirtualAuthenticatorOptions.new,
+          )
 
-          create_user_security_key(user)
+        create_user_security_key(user)
 
-          visit_reset_password_link
+        visit_reset_password_link
 
-          user_reset_password_page.try_another_way
+        user_reset_password_page.try_another_way
 
-          expect(user_reset_password_page).to have_totp_description
+        expect(user_reset_password_page).to have_totp_description
 
-          user_reset_password_page.use_backup_codes
+        user_reset_password_page.use_backup_codes
 
-          expect(user_reset_password_page).to have_backup_codes_description
+        expect(user_reset_password_page).to have_backup_codes_description
 
-          user_reset_password_page.use_totp
+        user_reset_password_page.use_totp
 
-          expect(user_reset_password_page).to have_totp_description
-        ensure
-          authenticator.remove!
-        end
+        expect(user_reset_password_page).to have_totp_description
+      ensure
+        authenticator.remove!
       end
     end
 
@@ -191,31 +173,28 @@ shared_examples "forgot password scenarios" do
       fab!(:user_second_factor_totp) { Fabricate(:user_second_factor_totp, user:) }
 
       it "should allow a user to reset password with TOTP instead of security key" do
-        # Test is flaky so trying with a longer wait time as a workaround
-        using_wait_time(Capybara.default_max_wait_time * 2) do
-          authenticator =
-            page.driver.browser.add_virtual_authenticator(
-              Selenium::WebDriver::VirtualAuthenticatorOptions.new,
-            )
+        authenticator =
+          page.driver.browser.add_virtual_authenticator(
+            Selenium::WebDriver::VirtualAuthenticatorOptions.new,
+          )
 
-          create_user_security_key(user)
+        create_user_security_key(user)
 
-          visit_reset_password_link
+        visit_reset_password_link
 
-          user_reset_password_page.try_another_way
+        user_reset_password_page.try_another_way
 
-          expect(user_reset_password_page).to have_no_toggle_button_in_second_factor_form
+        expect(user_reset_password_page).to have_no_toggle_button_in_second_factor_form
 
-          user_reset_password_page
-            .fill_in_totp(ROTP::TOTP.new(user_second_factor_totp.data).now)
-            .submit_totp
-            .fill_in_new_password("newsuperpassword")
-            .submit_new_password
+        user_reset_password_page
+          .fill_in_totp(ROTP::TOTP.new(user_second_factor_totp.data).now)
+          .submit_totp
+          .fill_in_new_password("newsuperpassword")
+          .submit_new_password
 
-          expect(user_reset_password_page).to have_logged_in_user
-        ensure
-          authenticator.remove!
-        end
+        expect(user_reset_password_page).to have_logged_in_user
+      ensure
+        authenticator.remove!
       end
     end
   end