DEV: Set `config.eager_load = true` on CI (#25032)

Why this change?

When running system tests on our CI, we have been occasionally seeing
server errors like:

```
Error encountered while proccessing /stylesheets/desktop_e58cf7f686aab173f9b778797f241913c2833c39.css
  NoMethodError: undefined method `+' for nil:NilClass
    /__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/path/pattern.rb:139:in `[]'
    /__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:127:in `block (2 levels) in find_routes'
    /__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:126:in `each'
    /__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:126:in `each_with_index'
    /__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:126:in `block in find_routes'
    /__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:123:in `map!'
    /__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:123:in `find_routes'
    /__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/journey/router.rb:32:in `serve'
    /__w/discourse/discourse/vendor/bundle/ruby/3.2.0/gems/actionpack-7.0.7/lib/action_dispatch/routing/route_set.rb:852:in `call'
```

While looking through various Rails issues related to the error above, I
came across https://github.com/rails/rails/pull/27647 which is a fix to
fully initialize routes before the first request is handled. However,
the routes are only fully initialize only if `config.eager_load` is set
to `true`. There is no reason why `config.eager_load` shouldn't be `true` in the
CI environment and this is what a new Rails 7.1 app is generated with.

What does this change do?

Enable `config.eager_load` when `env["CI"]` is present
This commit is contained in:
Alan Guo Xiang Tan 2023-12-26 13:05:55 +08:00 committed by GitHub
parent a69b386556
commit bf3e121323
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 25 additions and 40 deletions

View File

@ -190,22 +190,6 @@ jobs:
key: rspec-runtime-${{ matrix.build_type }}-${{ matrix.target }}-${{ github.run_id }}
restore-keys: rspec-runtime-${{ matrix.build_type }}-${{ matrix.target }}-
- name: Check Zeitwerk eager_load
if: matrix.build_type == 'backend'
env:
LOAD_PLUGINS: ${{ (matrix.target == 'plugins') && '1' || '0' }}
run: |
if ! bin/rails zeitwerk:check --trace; then
echo
echo "---------------------------------------------"
echo
echo "::error::'bin/rails zeitwerk:check' failed - the app will fail to boot with 'eager_load=true' (e.g. in production)."
echo "To reproduce locally, run 'bin/rails zeitwerk:check'."
echo "Alternatively, you can run your local server/tests with the 'DISCOURSE_ZEITWERK_EAGER_LOAD=1' environment variable."
echo
exit 1
fi
- name: Check Zeitwerk reloading
if: matrix.build_type == 'backend'
env:

View File

@ -132,6 +132,22 @@ class SessionController < ApplicationController
end
end
if Rails.env.test?
skip_before_action :check_xhr, only: :test_second_factor_restricted_route
def test_second_factor_restricted_route
result =
run_second_factor!(TestSecondFactorAction) do |manager|
manager.allow_backup_codes! if params[:allow_backup_codes]
end
if result.no_second_factors_enabled?
render json: { result: "no_second_factors_enabled" }
else
render json: { result: "second_factor_auth_completed" }
end
end
end
def sso_login
raise Discourse::NotFound unless SiteSetting.enable_discourse_connect
raise Discourse::ReadOnly if @readonly_mode && !staff_writes_only_mode?

View File

@ -9,6 +9,12 @@ Discourse::Application.configure do
# and recreated between test runs. Don't rely on the data there!
config.cache_classes = true
# Eager loading loads your entire application. When running a single test locally,
# this is usually not necessary, and can slow down your test suite. However, it's
# recommended that you enable it in continuous integration systems to ensure eager
# loading is working properly before deploying your code.
config.eager_load = ENV["CI"].present?
# Configure static asset server for tests with Cache-Control for performance
config.public_file_server.enabled = true
@ -44,8 +50,6 @@ Discourse::Application.configure do
config.assets.compile = true
config.assets.digest = false
config.eager_load = ENV["DISCOURSE_ZEITWERK_EAGER_LOAD"] == "1"
if ENV["RAILS_ENABLE_TEST_LOG"]
config.logger = Logger.new(STDOUT)
config.log_level =

View File

@ -6,7 +6,9 @@ RSpec.describe Chat::FlagMessage do
it { is_expected.to validate_presence_of(:channel_id) }
it { is_expected.to validate_presence_of(:message_id) }
it do
skip("TODO: This does not work in CI when eager load is set to true") if ENV["CI"]
is_expected.to validate_inclusion_of(:flag_type_id).in_array(ReviewableScore.types.values)
end
end

View File

@ -590,7 +590,7 @@ RSpec.configure do |config|
lines << "\n" if index != 0
lines << "Error encountered while proccessing #{path}"
lines << " #{ex.class}: #{ex.message}"
ex.backtrace.each { |line| lines << " #{line}" }
ex.backtrace.each { |line| lines << " #{line}\n" }
end
lines << "~~~~~~~ END SERVER EXCEPTIONS ~~~~~~~"

View File

@ -1,21 +0,0 @@
# frozen_string_literal: true
module SessionControllerExtension
def self.included(base)
base.skip_before_action :check_xhr, only: %i[test_second_factor_restricted_route]
end
def test_second_factor_restricted_route
result =
run_second_factor!(TestSecondFactorAction) do |manager|
manager.allow_backup_codes! if params[:allow_backup_codes]
end
if result.no_second_factors_enabled?
render json: { result: "no_second_factors_enabled" }
else
render json: { result: "second_factor_auth_completed" }
end
end
end
SessionController.class_eval { include SessionControllerExtension }