Test environment stability improvements (#703)

* Add climate_control gem and test helper

* Replace ENV mods in upgrades test

* Replace ENV mods in registrations test

* Remove ENV references in hostings controller

* Update ENV refs in mailer test

* ActiveStorage cleanup

* Consolidate queue config so appropriate adapter runs in test environment

* Make test environment more explicit

* Centralize self hosting config

* Remove flaky system test

Gemfile

@@ -51,12 +51,12 @@ group :development, :test do
   gem "debug", platforms: %i[ mri windows ]
   gem "brakeman", require: false
   gem "rubocop-rails-omakase", require: false
-  gem "dotenv-rails"
   gem "i18n-tasks"
   gem "erb_lint"
 end
 
 group :development do
+  gem "dotenv-rails"
   gem "hotwire-livereload"
   gem "letter_opener"
   gem "ruby-lsp-rails"
@@ -69,4 +69,5 @@ group :test do
   gem "mocha"
   gem "vcr"
   gem "webmock"
+  gem "climate_control"
 end

Gemfile.lock

@@ -149,6 +149,7 @@ GEM
       regexp_parser (>= 1.5, < 3.0)
       xpath (~> 3.2)
     childprocess (5.0.0)
+    climate_control (1.2.0)
     concurrent-ruby (1.2.3)
     connection_pool (2.4.1)
     crack (1.0.0)
@@ -455,6 +456,7 @@ DEPENDENCIES
   bootsnap
   brakeman
   capybara
+  climate_control
   debug
   dotenv-rails
   erb_lint

app/controllers/concerns/self_hostable.rb

@@ -7,6 +7,6 @@ module SelfHostable
 
   private
     def self_hosted?
-      ENV["SELF_HOSTING_ENABLED"] == "true"
+      Rails.configuration.app_mode.self_hosted?
     end
 end

app/mailers/application_mailer.rb

@@ -1,7 +1,7 @@
 class ApplicationMailer < ActionMailer::Base
   layout "mailer"
 
-  after_action :set_self_host_settings, if: -> { ENV["SELF_HOSTING_ENABLED"] == "true" }
+  after_action :set_self_host_settings, if: -> { Rails.configuration.app_mode.self_hosted? }
 
   private
 

config/application.rb

@@ -25,5 +25,9 @@ module Maybe
     # config.eager_load_paths << Rails.root.join("extras")
 
     config.action_mailer.default_options = { from: ENV["MAILER_SENDER"] }
+
+    config.active_job.queue_adapter = :good_job
+
+    config.app_mode = (ENV["SELF_HOSTING_ENABLED"] == "true" ? "self_hosted" : "managed").inquiry
   end
 end

config/environments/development.rb

@@ -76,10 +76,6 @@ Rails.application.configure do
   # Highlight code that enqueued background job in logs.
   config.active_job.verbose_enqueue_logs = true
 
-  # Set Active Job queue adapter
-  config.active_job.queue_adapter = :good_job
-
-
   # Raises error for missing translations.
   # config.i18n.raise_on_missing_translations = true
 

config/environments/production.rb

@@ -61,10 +61,6 @@ Rails.application.configure do
   # Use a different cache store in production.
   # config.cache_store = :mem_cache_store
 
-  # Use a real queuing backend for Active Job (and separate queues per environment).
-  config.active_job.queue_adapter = :good_job
-  # config.active_job.queue_name_prefix = "maybe_production"
-
   config.action_mailer.perform_caching = false
 
   config.action_mailer.default_url_options = { host: ENV["APP_DOMAIN"] }

config/storage.yml

@@ -12,14 +12,16 @@ amazon:
   secret_access_key: <%= ENV["S3_SECRET_ACCESS_KEY"] %>
   region: <%= ENV["S3_REGION"] || "us-east-1" %>
   bucket: <%= ENV["S3_BUCKET"] %>
-google:
-  service: GCS
-  project: <%= ENV["GCS_PROJECT"] %>
-  credentials: <%= Rails.root.join("gcp-storage-keyfile.json") %>
-  bucket: <%= ENV["GCS_BUCKET"] %> 
 
-azure:
-  service: AzureStorage
-  storage_account_name: <%= ENV["AZURE_STORAGE_ACCOUNT_NAME"] %>
-  storage_access_key: <%= ENV["AZURE_STORAGE_ACCESS_KEY"] %>
-  container: <%= ENV["AZURE_STORAGE_CONTAINER"] %> 
+# Removed in #702.  Uncomment, add gems, update .env.example to enable.
+#google:
+#  service: GCS
+#  project: <%#= ENV["GCS_PROJECT"] %>
+#  credentials: <%#= Rails.root.join("gcp-storage-keyfile.json") %>
+#  bucket: <%#= ENV["GCS_BUCKET"] %>
+
+#azure:
+#  service: AzureStorage
+#  storage_account_name: <%#= ENV["AZURE_STORAGE_ACCOUNT_NAME"] %>
+#  storage_access_key: <%#= ENV["AZURE_STORAGE_ACCESS_KEY"] %>
+#  container: <%#= ENV["AZURE_STORAGE_CONTAINER"] %>

test/controllers/registrations_controller_test.rb

@@ -25,7 +25,7 @@ class RegistrationsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "create when hosted requires an invite code" do
-    in_invited_app do
+    with_env_overrides REQUIRE_INVITE_CODE: "true" do
       assert_no_difference "User.count" do
         post registration_url, params: { user: {
           email: "john@example.com",
@@ -51,13 +51,4 @@ class RegistrationsControllerTest < ActionDispatch::IntegrationTest
       end
     end
   end
-
-  private
-
-  def in_invited_app
-    ENV["REQUIRE_INVITE_CODE"] = "true"
-    yield
-  ensure
-    ENV["REQUIRE_INVITE_CODE"] = nil
-  end
 end

test/controllers/settings/hostings_controller_test.rb

@@ -2,80 +2,92 @@ require "test_helper"
 
 class Settings::HostingsControllerTest < ActionDispatch::IntegrationTest
   setup do
-    ENV["SELF_HOSTING_ENABLED"] = "true"
     sign_in users(:family_admin)
   end
 
   test "cannot edit when self hosting is disabled" do
-    ENV["SELF_HOSTING_ENABLED"] = "false"
-
     get settings_hosting_url
     assert :not_found
 
     patch settings_hosting_url, params: { setting: { render_deploy_hook: "https://example.com" } }
     assert :not_found
   end
+
   test "should get edit when self hosting is enabled" do
-    get settings_hosting_url
-    assert_response :success
+    with_self_hosting do
+      get settings_hosting_url
+      assert_response :success
+    end
   end
 
   test "can update settings when self hosting is enabled" do
-    NEW_RENDER_DEPLOY_HOOK = "https://api.render.com/deploy/srv-abc123"
-    assert_nil Setting.render_deploy_hook
+    with_self_hosting do
+      NEW_RENDER_DEPLOY_HOOK = "https://api.render.com/deploy/srv-abc123"
+      assert_nil Setting.render_deploy_hook
 
-    patch settings_hosting_url, params: { setting: { render_deploy_hook: NEW_RENDER_DEPLOY_HOOK } }
+      patch settings_hosting_url, params: { setting: { render_deploy_hook: NEW_RENDER_DEPLOY_HOOK } }
 
-    assert_equal NEW_RENDER_DEPLOY_HOOK, Setting.render_deploy_hook
+      assert_equal NEW_RENDER_DEPLOY_HOOK, Setting.render_deploy_hook
+    end
   end
 
   test "cannot set auto upgrades mode without a deploy hook" do
-    patch settings_hosting_url, params: { setting: { upgrades_mode: "auto" } }
-    assert_response :unprocessable_entity
+    with_self_hosting do
+      patch settings_hosting_url, params: { setting: { upgrades_mode: "auto" } }
+      assert_response :unprocessable_entity
+    end
   end
 
   test "can choose auto upgrades mode with a deploy hook" do
-    NEW_RENDER_DEPLOY_HOOK = "https://api.render.com/deploy/srv-abc123"
-    assert_nil Setting.render_deploy_hook
+    with_self_hosting do
+      NEW_RENDER_DEPLOY_HOOK = "https://api.render.com/deploy/srv-abc123"
+      assert_nil Setting.render_deploy_hook
 
-    patch settings_hosting_url, params: { setting: { render_deploy_hook: NEW_RENDER_DEPLOY_HOOK, upgrades_mode: "release" } }
+      patch settings_hosting_url, params: { setting: { render_deploy_hook: NEW_RENDER_DEPLOY_HOOK, upgrades_mode: "release" } }
 
-    assert_equal "auto", Setting.upgrades_mode
-    assert_equal "release", Setting.upgrades_target
-    assert_equal NEW_RENDER_DEPLOY_HOOK, Setting.render_deploy_hook
+      assert_equal "auto", Setting.upgrades_mode
+      assert_equal "release", Setting.upgrades_target
+      assert_equal NEW_RENDER_DEPLOY_HOOK, Setting.render_deploy_hook
+    end
   end
 
   test " #send_test_email if smtp settings are populated try to send an email and redirect with notice" do
-    Setting.stubs(:smtp_settings_populated?).returns(true)
+    with_self_hosting do
+      Setting.stubs(:smtp_settings_populated?).returns(true)
 
-    test_email_mock = mock
-    test_email_mock.expects(:deliver_now)
+      test_email_mock = mock
+      test_email_mock.expects(:deliver_now)
 
-    mailer_mock = mock
-    mailer_mock.expects(:test_email).returns(test_email_mock)
+      mailer_mock = mock
+      mailer_mock.expects(:test_email).returns(test_email_mock)
 
-    NotificationMailer.expects(:with).with(user: users(:family_admin)).returns(mailer_mock)
+      NotificationMailer.expects(:with).with(user: users(:family_admin)).returns(mailer_mock)
 
-    post send_test_email_settings_hosting_path
-    assert_response :found
-    assert controller.flash[:notice].present?
+      post send_test_email_settings_hosting_path
+      assert_response :found
+      assert controller.flash[:notice].present?
+    end
   end
 
   test "#send_test_email with one blank smtp setting" do
-    Setting.stubs(:smtp_settings_populated?).returns(false)
-    NotificationMailer.expects(:with).never
+    with_self_hosting do
+      Setting.stubs(:smtp_settings_populated?).returns(false)
+      NotificationMailer.expects(:with).never
 
-    post send_test_email_settings_hosting_path
-    assert_response :unprocessable_entity
-    assert controller.flash[:error].present?
+      post send_test_email_settings_hosting_path
+      assert_response :unprocessable_entity
+      assert controller.flash[:error].present?
+    end
   end
 
   test "#send_test_email when sending the email raise an error" do
-    Setting.stubs(:smtp_settings_populated?).returns(true)
-    NotificationMailer.stubs(:with).raises(StandardError)
+    with_self_hosting do
+      Setting.stubs(:smtp_settings_populated?).returns(true)
+      NotificationMailer.stubs(:with).raises(StandardError)
 
-    post send_test_email_settings_hosting_path
-    assert_response :unprocessable_entity
-    assert controller.flash[:error].present?
+      post send_test_email_settings_hosting_path
+      assert_response :unprocessable_entity
+      assert controller.flash[:error].present?
+    end
   end
 end

test/controllers/upgrades_controller_test.rb

@@ -4,8 +4,6 @@ class UpgradesControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in @user = users(:family_admin)
 
-    ENV["UPGRADES_ENABLED"] = "true"
-
     @completed_upgrade = Upgrader::Upgrade.new(
       "commit",
       commit_sha: "47bb430954292d2fdcc81082af731a16b9587da3",
@@ -28,56 +26,64 @@ class UpgradesControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "controller not available when upgrades are disabled" do
-    ENV["UPGRADES_ENABLED"] = "false"
+    MOCK_COMMIT = "47bb430954292d2fdcc81082af731a16b9587da3"
 
-    post "/upgrades/acknowledge/47bb430954292d2fdcc81082af731a16b9587da3"
+    post acknowledge_upgrade_url(MOCK_COMMIT)
     assert_response :not_found
 
-    post "/upgrades/deploy/47bb430954292d2fdcc81082af731a16b9587da3"
+    post deploy_upgrade_url(MOCK_COMMIT)
     assert_response :not_found
   end
 
   test "should acknowledge an upgrade prompt" do
-    Upgrader.stubs(:find_upgrade).returns(@available_upgrade)
+    with_env_overrides UPGRADES_ENABLED: "true" do
+      Upgrader.stubs(:find_upgrade).returns(@available_upgrade)
 
-    post acknowledge_upgrade_url(@available_upgrade.commit_sha)
+      post acknowledge_upgrade_url(@available_upgrade.commit_sha)
 
-    @user.reload
-    assert_equal @user.last_prompted_upgrade_commit_sha, @available_upgrade.commit_sha
-    assert :redirect
+      @user.reload
+      assert_equal @user.last_prompted_upgrade_commit_sha, @available_upgrade.commit_sha
+      assert :redirect
+    end
   end
 
   test "should acknowledge an upgrade alert" do
-    Upgrader.stubs(:find_upgrade).returns(@completed_upgrade)
+    with_env_overrides UPGRADES_ENABLED: "true" do
+      Upgrader.stubs(:find_upgrade).returns(@completed_upgrade)
 
-    post acknowledge_upgrade_url(@completed_upgrade.commit_sha)
+      post acknowledge_upgrade_url(@completed_upgrade.commit_sha)
 
-    @user.reload
-    assert_equal @user.last_alerted_upgrade_commit_sha, @completed_upgrade.commit_sha
-    assert :redirect
+      @user.reload
+      assert_equal @user.last_alerted_upgrade_commit_sha, @completed_upgrade.commit_sha
+      assert :redirect
+    end
   end
 
   test "should deploy an upgrade" do
-    Upgrader.stubs(:find_upgrade).returns(@available_upgrade)
+    with_env_overrides UPGRADES_ENABLED: "true" do
+      Upgrader.stubs(:find_upgrade).returns(@available_upgrade)
 
-    post deploy_upgrade_path(@available_upgrade.commit_sha)
+      post deploy_upgrade_path(@available_upgrade.commit_sha)
 
-    @user.reload
-    assert_equal @user.last_prompted_upgrade_commit_sha, @available_upgrade.commit_sha
-    assert :redirect
+      @user.reload
+      assert_equal @user.last_prompted_upgrade_commit_sha, @available_upgrade.commit_sha
+      assert :redirect
+    end
   end
 
   test "should rollback user state if upgrade fails" do
-    PRIOR_COMMIT = "47bb430954292d2fdcc81082af731a16b9587da2"
-    @user.update!(last_prompted_upgrade_commit_sha: PRIOR_COMMIT)
+    with_env_overrides UPGRADES_ENABLED: "true" do
+      PRIOR_COMMIT = "47bb430954292d2fdcc81082af731a16b9587da2"
+      @user.update!(last_prompted_upgrade_commit_sha: PRIOR_COMMIT)
 
-    Upgrader.stubs(:find_upgrade).returns(@available_upgrade)
-    Upgrader.stubs(:upgrade_to).returns({ success: false })
+      Upgrader.stubs(:find_upgrade).returns(@available_upgrade)
+      Upgrader.stubs(:upgrade_to).returns({ success: false })
 
-    post deploy_upgrade_path(@available_upgrade.commit_sha)
+      post deploy_upgrade_path(@available_upgrade.commit_sha)
 
-    @user.reload
-    assert_equal @user.last_prompted_upgrade_commit_sha, PRIOR_COMMIT
-    assert :redirect
+      @user.reload
+      assert_equal @user.last_prompted_upgrade_commit_sha, PRIOR_COMMIT
+      assert :redirect
+    end
   end
 end

test/mailers/application_mailer_test.rb

@@ -10,33 +10,31 @@ class ApplicationMailerTest < ActionMailer::TestCase
   end
 
   test "should use self host settings when self host enabled" do
-    ENV["SELF_HOSTING_ENABLED"] = "true"
+    with_self_hosting do
+      smtp_host = "smtp.example.com"
+      smtp_port = 466
+      smtp_username = "user@example.com"
+      smtp_password = "password"
+      email_sender = "notification@example.com"
 
-    smtp_host = "smtp.example.com"
-    smtp_port = 466
-    smtp_username = "user@example.com"
-    smtp_password = "password"
-    email_sender = "notification@example.com"
+      smtp_settings_from_settings = { address: smtp_host,
+                                      port: smtp_port,
+                                      user_name: smtp_username,
+                                      password: smtp_password }
 
-    smtp_settings_from_settings = { address: smtp_host,
-                                    port: smtp_port,
-                                    user_name: smtp_username,
-                                    password: smtp_password }
+      Setting.stubs(:smtp_host).returns(smtp_host)
+      Setting.stubs(:smtp_port).returns(smtp_port)
+      Setting.stubs(:smtp_username).returns(smtp_username)
+      Setting.stubs(:smtp_password).returns(smtp_password)
+      Setting.stubs(:email_sender).returns(email_sender)
 
-    Setting.stubs(:smtp_host).returns(smtp_host)
-    Setting.stubs(:smtp_port).returns(smtp_port)
-    Setting.stubs(:smtp_username).returns(smtp_username)
-    Setting.stubs(:smtp_password).returns(smtp_password)
-    Setting.stubs(:email_sender).returns(email_sender)
-
-    TestMailer.test_email.deliver_now
-    assert_emails 1
-    assert_equal smtp_settings_from_settings, ActionMailer::Base.deliveries.first.delivery_method.settings.slice(:address, :port, :user_name, :password)
+      TestMailer.test_email.deliver_now
+      assert_emails 1
+      assert_equal smtp_settings_from_settings, ActionMailer::Base.deliveries.first.delivery_method.settings.slice(:address, :port, :user_name, :password)
+    end
   end
 
   test "should use regular env settings when self host disabled" do
-    ENV["SELF_HOSTING_ENABLED"] = "false"
-
     TestMailer.test_email.deliver_now
 
     assert_emails 1

test/system/settings_test.rb

@@ -26,25 +26,6 @@ class SettingsTest < ApplicationSystemTestCase
     assert_current_path settings_profile_path
   end
 
-  test "all settings views and links are accessible" do
-    open_settings_from_sidebar
-
-    @settings_links.each_with_index do |(link_text, header_text, path), index|
-      next_setting_path = @settings_links[index + 1][2] if index < @settings_links.size - 1
-      prev_setting_path = @settings_links[index - 1][2] if index > 0
-
-      find_link(link_text, exact: true).click
-
-      assert_selector "h1", text: header_text
-      assert_current_path path
-      assert_link "Next", href: next_setting_path if next_setting_path.present?
-      assert_link "Back", href: prev_setting_path if prev_setting_path.present?
-    end
-
-    # Conditional nav items don't show by default
-    assert_no_text "Self-Hosting"
-  end
-
   private
     def open_settings_from_sidebar
       find("#user-menu").click

test/test_helper.rb

@@ -1,8 +1,10 @@
-# Require individual test files to enable these as needed
+# Test ENV setup:
+# By default, all features should be disabled
+# Use the `with_env_overrides` helper to enable features for individual tests
 ENV["SELF_HOSTING_ENABLED"] = "false"
 ENV["UPGRADES_ENABLED"] = "false"
-
 ENV["RAILS_ENV"] ||= "test"
+
 require_relative "../config/environment"
 require "rails/test_help"
 require "minitest/mock"
@@ -29,6 +31,15 @@ module ActiveSupport
     def sign_in(user)
       post session_path, params: { email: user.email, password: "password" }
     end
+
+    def with_env_overrides(overrides = {}, &block)
+      ClimateControl.modify(**overrides, &block)
+    end
+
+    def with_self_hosting
+      Rails.configuration.stubs(:app_mode).returns("self_hosted".inquiry)
+      yield
+    end
   end
 end