Skip to content

Commit

Permalink
Ensure application gets reloaded only once
Browse files Browse the repository at this point in the history
  • Loading branch information
jscheid committed Apr 25, 2019
1 parent 77b4d5f commit 21b6138
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 14 deletions.
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ version: 2.1
command: |
COVERAGE=true PARALLEL_TEST_PROCESSORS=4 bin/parallel_rspec spec/
COVERAGE=true RSPEC_FILESYSTEM_CHANGES=true bin/rspec
COVERAGE=true CLASS_RELOADING=true bin/rspec
.run_features: &run_features
run:
Expand Down
1 change: 1 addition & 0 deletions .rspec
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
--format <%= ENV['CI'] ? 'documentation' : 'progress' %>
--require spec_helper
<%= "--require #{__dir__}/spec/support/simplecov_changes_env.rb --tag changes_filesystem" if ENV['RSPEC_FILESYSTEM_CHANGES'] %>
<%= "--require #{__dir__}/spec/support/simplecov_reload_env.rb --tag requires_reloading" if ENV['CLASS_RELOADING'] %>
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Bug Fixes

* Ensure application gets reloaded only once. [#5740] by [@jscheid]

### Removals

* Support for ruby 2.3 has been removed. [#5751] by [@deivid-rodriguez]
Expand Down Expand Up @@ -439,6 +443,7 @@ Please check [0-6-stable] for previous changes.
[#5662]: https://github.com/activeadmin/activeadmin/pull/5662
[#5726]: https://github.com/activeadmin/activeadmin/pull/5726
[#5738]: https://github.com/activeadmin/activeadmin/pull/5738
[#5740]: https://github.com/activeadmin/activeadmin/pull/5740
[#5751]: https://github.com/activeadmin/activeadmin/pull/5751

[@5t111111]: https://github.com/5t111111
Expand Down Expand Up @@ -504,3 +509,4 @@ Please check [0-6-stable] for previous changes.
[@chrp]: https://github.com/chrp
[@bartoszkopinski]: https://github.com/bartoszkopinski
[@panasyuk]: https://github.com/panasyuk
[@jscheid]: https://github.com/jscheid
13 changes: 2 additions & 11 deletions lib/active_admin/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,17 +187,8 @@ def remove_active_admin_load_paths_from_rails_autoload_and_eager_load
# regenerate the routes as well.
def attach_reloader
Rails.application.config.after_initialize do |app|
unload_active_admin = -> { ActiveAdmin.application.unload! }

if app.config.reload_classes_only_on_change
# Rails is about to unload all the app files (e.g. models), so we
# should first unload the classes generated by Active Admin, otherwise
# they will contain references to the stale (unloaded) classes.
ActiveSupport::Reloader.to_prepare(prepend: true, &unload_active_admin)
else
# If the user has configured the app to always reload app files after
# each request, so we should unload the generated classes too.
ActiveSupport::Reloader.to_complete(&unload_active_admin)
ActiveSupport::Reloader.after_class_unload do
ActiveAdmin.application.unload!
end

admin_dirs = {}
Expand Down
13 changes: 13 additions & 0 deletions spec/integration/rails_integration_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
require 'rails_helper'

RSpec.describe 'Rails/ActiveAdmin integration' do
it 'unloads and reloads ActiveAdmin only once', :requires_reloading do
allow(ActiveAdmin.application).to receive(:unload!)
allow(ActiveAdmin.application).to receive(:load!)

Rails.application.reloader.reload!

expect(ActiveAdmin.application).to have_received(:unload!).once
expect(ActiveAdmin.application).to have_received(:load!).once
end
end
3 changes: 2 additions & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
RSpec.configure do |config|
config.disable_monkey_patching!
config.filter_run focus: true
config.filter_run_excluding changes_filesystem: true
config.filter_run_excluding changes_filesystem: true,
requires_reloading: true
config.run_all_when_everything_filtered = true
config.color = true
config.order = :random
Expand Down
5 changes: 5 additions & 0 deletions spec/support/simplecov_reload_env.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
if ENV["COVERAGE"] == "true"
require "simplecov"

SimpleCov.command_name "reload specs"
end
7 changes: 6 additions & 1 deletion spec/unit/application_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@
end

describe "#prepare" do
before { application.prepare! }
before do
%i[run complete prepare class_unload].each do |event|
ActiveSupport::Reloader.reset_callbacks(event)
end
application.prepare!
end

it "should remove app/admin from the autoload paths" do
expect(ActiveSupport::Dependencies.autoload_paths).to_not include(Rails.root.join("app/admin"))
Expand Down
7 changes: 6 additions & 1 deletion tasks/test.rake
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ task spec: :"spec:all"

namespace :spec do
desc "Run all specs"
task all: [:regular, :filesystem_changes]
task all: [:regular, :filesystem_changes, :reloading]

desc "Run the standard specs in parallel"
task :regular do
Expand All @@ -23,6 +23,11 @@ namespace :spec do
task :filesystem_changes do
sh({ "RSPEC_FILESYSTEM_CHANGES" => "true" }, "bin/rspec")
end

desc "Run the specs that require reloading"
task :reloading do
sh({ "CLASS_RELOADING" => "true" }, "bin/rspec")
end
end

desc "Run the cucumber scenarios in parallel"
Expand Down

0 comments on commit 21b6138

Please sign in to comment.