Fix incorrectly handled tests (#4432)

* Refactor tests

* Refactor tests added lastly

* Remove unnecessary lines from a test suite

* Add a mailer config for tests

* Update a test of ExpirePushesJobTest

* Update tests changing Settings

* Update tests using `reload_routes!`

* Remove redundant comparison

Author: ozovalihasan

config/environments/test.rb

@@ -41,6 +41,8 @@
 
   config.action_mailer.perform_caching = false
 
+  config.action_mailer.default_url_options = {host: "localhost", port: 3000}
+
   # Tell Action Mailer not to deliver emails to the real world.
   # The :test delivery method accumulates sent emails in the
   # ActionMailer::Base.deliveries array.

test/controllers/api/base_controller_test.rb

@@ -13,10 +13,7 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
   end
 
   teardown do
-    # Restore feature flags so later tests (e.g. file_push, url push) don't see them false
-    Settings.require_mfa = false
-    Settings.enable_file_pushes = true
-    Settings.enable_url_pushes = true
+    Settings.reload!
     Rails.application.reload_routes!
   end
 
@@ -278,8 +275,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "/p/create with files requires authentication even when allow_anonymous is true" do
-    previous_allow_anonymous = Settings.allow_anonymous
-    previous_enable_file_pushes = Settings.enable_file_pushes
     Settings.allow_anonymous = true
     Settings.enable_file_pushes = true
     Rails.application.reload_routes!
@@ -296,10 +291,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
       }
 
     assert_response :unauthorized
-  ensure
-    Settings.allow_anonymous = previous_allow_anonymous
-    Settings.enable_file_pushes = previous_enable_file_pushes
-    Rails.application.reload_routes!
   end
 
   test "/p/create with files works with valid token when allow_anonymous is true" do
@@ -322,10 +313,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
     assert_response :created
     json = JSON.parse(response.body)
     assert json["url_token"].present?
-  ensure
-    Settings.allow_anonymous = true
-    Settings.enable_file_pushes = false
-    Rails.application.reload_routes!
   end
 
   test "/p/create with empty files key requires authentication when allow_anonymous is true" do
@@ -345,10 +332,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
       }
 
     assert_response :unauthorized
-  ensure
-    Settings.allow_anonymous = true
-    Settings.enable_file_pushes = false
-    Rails.application.reload_routes!
   end
 
   # When allow_anonymous is false, Api::V1::PushesController#create calls
@@ -372,8 +355,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
     if response.redirect?
       assert_match(%r{/users/sign_in}, response.location)
     end
-  ensure
-    Settings.allow_anonymous = true
   end
 
   test "/p/create with valid token succeeds when allow_anonymous is false" do
@@ -393,8 +374,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
     assert_response :created, "authenticated create should succeed when allow_anonymous is false"
     json = JSON.parse(response.body)
     assert json["url_token"].present?
-  ensure
-    Settings.allow_anonymous = true
   end
 
   test "v2 show requires authentication when allow_anonymous is false" do
@@ -407,8 +386,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
       }
 
     assert_response :unauthorized
-  ensure
-    Settings.allow_anonymous = true
   end
 
   # Test path-based authentication requirements for /f paths
@@ -428,9 +405,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
       }
 
     assert_response :unauthorized
-  ensure
-    Settings.enable_file_pushes = false
-    Rails.application.reload_routes!
   end
 
   test "/f/create works with valid token" do
@@ -451,9 +425,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
 
     # Should not be unauthorized (may be other validation errors)
     assert_not_equal :unauthorized, response.status
-  ensure
-    Settings.enable_file_pushes = false
-    Rails.application.reload_routes!
   end
 
   test "/f/audit requires authentication" do
@@ -465,9 +436,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
 
     get "/f/#{push.url_token}/audit.json"
     assert_response :unauthorized
-  ensure
-    Settings.enable_file_pushes = false
-    Rails.application.reload_routes!
   end
 
   test "/f/active requires authentication" do
@@ -476,9 +444,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
 
     get "/f/active.json"
     assert_response :unauthorized
-  ensure
-    Settings.enable_file_pushes = false
-    Rails.application.reload_routes!
   end
 
   test "/f/expired requires authentication" do
@@ -487,9 +452,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
 
     get "/f/expired.json"
     assert_response :unauthorized
-  ensure
-    Settings.enable_file_pushes = false
-    Rails.application.reload_routes!
   end
 
   # Test path-based authentication requirements for /r paths
@@ -508,9 +470,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
       }
 
     assert_response :unauthorized
-  ensure
-    Settings.enable_url_pushes = false
-    Rails.application.reload_routes!
   end
 
   test "/r/create works with valid token" do
@@ -530,9 +489,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
 
     # Should not be unauthorized (may be other validation errors)
     assert_not_equal :unauthorized, response.status
-  ensure
-    Settings.enable_url_pushes = false
-    Rails.application.reload_routes!
   end
 
   test "/r/audit requires authentication" do
@@ -544,9 +500,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
 
     get "/r/#{push.url_token}/audit.json"
     assert_response :unauthorized
-  ensure
-    Settings.enable_url_pushes = false
-    Rails.application.reload_routes!
   end
 
   test "/r/active requires authentication" do
@@ -555,9 +508,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
 
     get "/r/active.json"
     assert_response :unauthorized
-  ensure
-    Settings.enable_url_pushes = false
-    Rails.application.reload_routes!
   end
 
   test "/r/expired requires authentication" do
@@ -566,9 +516,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
 
     get "/r/expired.json"
     assert_response :unauthorized
-  ensure
-    Settings.enable_url_pushes = false
-    Rails.application.reload_routes!
   end
 
   # Test user already signed in (session-based auth)
@@ -664,9 +611,6 @@ class Api::BaseControllerTest < ActionDispatch::IntegrationTest
     assert_response :bad_request
     json_response = JSON.parse(@response.body)
     assert json_response.key?("error")
-  ensure
-    Settings.enable_file_pushes = false
-    Rails.application.reload_routes!
   end
 
   # Test token with blank/nil values

test/controllers/file_push_controller_test.rb

@@ -11,8 +11,8 @@ class FilePushControllerTest < ActionDispatch::IntegrationTest
   end
 
   teardown do
-    @luca = users(:luca)
-    sign_out @luca
+    Settings.reload!
+    Rails.application.reload_routes!
   end
 
   test "New push form is NOT available anonymous" do

test/controllers/first_run_controller_test.rb

@@ -22,8 +22,6 @@ def stub_boot_code_file
   setup do
     Settings.disable_signups = false
     Rails.application.reload_routes!
-    # Set default URL options for test environment to avoid missing host errors
-    Rails.application.routes.default_url_options[:host] = "localhost:3000"
 
     User.destroy_all
     FirstRunBootCode.clear!
@@ -32,8 +30,8 @@ def stub_boot_code_file
   teardown do
     User.destroy_all
     FirstRunBootCode.clear!
-    Settings.disable_logins = false
-    Settings.disable_signups = false
+    Settings.reload!
+    Rails.application.reload_routes!
   end
 
   test "any page redirects to first run when no users exist" do

test/controllers/qr_push_controller_test.rb

@@ -10,8 +10,7 @@ class QrPushControllerTest < ActionDispatch::IntegrationTest
   end
 
   teardown do
-    @luca = users(:luca)
-    sign_out @luca
+    Settings.reload!
   end
 
   test "New push form is available when anonymous" do
@@ -113,8 +112,6 @@ class QrPushControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to root_path
     follow_redirect!
     assert_match(/QR code pushes are disabled\./i, flash[:notice])
-  ensure
-    Settings.enable_qr_pushes = true
   end
 
   test "when QR pushes disabled, creating a QR push redirects to root with notice" do
@@ -129,8 +126,6 @@ class QrPushControllerTest < ActionDispatch::IntegrationTest
 
     assert_redirected_to root_path
     assert_equal I18n._("QR code pushes are disabled."), flash[:notice]
-  ensure
-    Settings.enable_qr_pushes = true
   end
 
   test "when QR pushes disabled, logged-in user creating QR push redirects to root with notice" do
@@ -147,7 +142,5 @@ class QrPushControllerTest < ActionDispatch::IntegrationTest
 
     assert_redirected_to root_path
     assert_equal I18n._("QR code pushes are disabled."), flash[:notice]
-  ensure
-    Settings.enable_qr_pushes = true
   end
 end

test/controllers/text_push_controller_test.rb

@@ -12,8 +12,7 @@ class TextPushControllerTest < ActionDispatch::IntegrationTest
   end
 
   teardown do
-    Settings.enable_file_pushes = false
-    Settings.enable_url_pushes = false
+    Settings.reload!
   end
 
   test "New push form is available anonymous" do
@@ -217,8 +216,6 @@ class TextPushControllerTest < ActionDispatch::IntegrationTest
     assert_response :redirect
     follow_redirect!
     assert response.body.include?("You need to sign in or sign up before continuing.")
-  ensure
-    Settings.allow_anonymous = true
   end
 
   test "when allow_anonymous disabled, anonymous user cannot create password push" do
@@ -234,8 +231,6 @@ class TextPushControllerTest < ActionDispatch::IntegrationTest
     assert_response :redirect
     follow_redirect!
     assert response.body.include?("You need to sign in or sign up before continuing.")
-  ensure
-    Settings.allow_anonymous = true
   end
 
   test "when allow_anonymous disabled, logged-in user can access new and create password push" do
@@ -253,7 +248,5 @@ class TextPushControllerTest < ActionDispatch::IntegrationTest
       }
     }
     assert_response :redirect
-  ensure
-    Settings.allow_anonymous = true
   end
 end

test/controllers/urls_controller_test.rb

@@ -11,8 +11,8 @@ class UrlsControllerTest < ActionDispatch::IntegrationTest
   end
 
   teardown do
-    @luca = users(:luca)
-    sign_out @luca
+    Settings.reload!
+    Rails.application.reload_routes!
   end
 
   test "New push form is available when anonymous" do
@@ -93,9 +93,6 @@ class UrlsControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to root_path
     follow_redirect!
     assert_match(/URL pushes are disabled\./i, flash[:notice])
-  ensure
-    Settings.enable_url_pushes = true
-    Rails.application.reload_routes!
   end
 
   test "when URL pushes disabled, creating a URL push redirects to root with notice" do
@@ -111,9 +108,6 @@ class UrlsControllerTest < ActionDispatch::IntegrationTest
 
     assert_redirected_to root_path
     assert_equal I18n._("URL pushes are disabled."), flash[:notice]
-  ensure
-    Settings.enable_url_pushes = true
-    Rails.application.reload_routes!
   end
 
   test "when URL pushes disabled, logged-in user creating URL push redirects to root with notice" do
@@ -131,8 +125,5 @@ class UrlsControllerTest < ActionDispatch::IntegrationTest
 
     assert_redirected_to root_path
     assert_equal I18n._("URL pushes are disabled."), flash[:notice]
-  ensure
-    Settings.enable_url_pushes = true
-    Rails.application.reload_routes!
   end
 end

test/controllers/users/registrations_controller_test.rb

@@ -10,10 +10,6 @@ class Users::RegistrationsControllerTest < ActionDispatch::IntegrationTest
     @other_user = users(:one)
   end
 
-  teardown do
-    Settings.disable_logins = false
-  end
-
   # DELETE /users (destroy action)
 
   test "authenticated user can delete their own account" do

test/helpers/application_helper_test.rb

@@ -7,18 +7,10 @@ class ApplicationHelperTest < ActionView::TestCase
 
   setup do
     @push = pushes(:test_push)
-    # Ensure Settings are available
-    @original_title = Settings.brand.title
-    @original_enabled_language_codes = Settings.enabled_language_codes.dup
-    @original_override_base_url = Settings.override_base_url
   end
 
   teardown do
-    # Restore original settings
-    Settings.brand.title = @original_title
-    Settings.enabled_language_codes = @original_enabled_language_codes
-    Settings.override_base_url = @original_override_base_url
-    ENV.delete("FORCE_SSL")
+    Settings.reload!
   end
 
   # Test title method

test/helpers/language_helper_test.rb

@@ -7,12 +7,11 @@ class LanguageHelperTest < ActionView::TestCase
 
   setup do
     @original_available_locales = I18n.available_locales.dup
-    @original_language_codes = Settings.language_codes.dup
   end
 
   teardown do
     I18n.available_locales = @original_available_locales
-    Settings.language_codes = @original_language_codes
+    Settings.reload!
   end
 
   test "language_options_for_select returns array of language code and locale pairs" do