Skip to content

Track and unregister loaders in tests manually #293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
katafrakt wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Track and unregister loaders in tests manually #293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c0e41f5
Use Zeitwerk::Registry to unload loaders in tests
katafrakt Dec 4, 2025
99834e7
Track and unregister loaders in tests manually
katafrakt Dec 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions ...tegration/container/auto_registration/component_dir_namespaces/autoloading_loader_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
require "zeitwerk"

RSpec.describe "Component dir namespaces / Autoloading loader" do
include ZeitwerkHelpers

let(:container) {
root = @dir
dir_config = defined?(component_dir_config) ? component_dir_config : -> * {}
Expand All @@ -21,9 +19,9 @@
}
}

let(:loader) { Zeitwerk::Loader.new }
let(:loader) { ZeitwerkLoaderRegistry.new_loader }

after { teardown_zeitwerk }
after { ZeitwerkLoaderRegistry.clear }

context "top-level constant namespace" do
let(:component_dir_config) {
Expand Down
6 changes: 2 additions & 4 deletions spec/integration/container/autoloading_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
require "zeitwerk"

RSpec.describe "Autoloading loader" do
include ZeitwerkHelpers

specify "Resolving components using Zeitwerk" do
module Test
class Container < Dry::System::Container
Expand All @@ -19,7 +17,7 @@ class Container < Dry::System::Container
end
end

loader = Zeitwerk::Loader.new
loader = ZeitwerkLoaderRegistry.new_loader
loader.push_dir Test::Container.config.root.join("lib").realpath
loader.setup

Expand All @@ -29,6 +27,6 @@ class Container < Dry::System::Container
expect(foo).to be_a Test::Foo
expect(entity).to be_a Test::Entities::FooEntity

teardown_zeitwerk
ZeitwerkLoaderRegistry.clear
end
end
4 changes: 1 addition & 3 deletions spec/integration/container/plugins/zeitwerk/eager_loading_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
# rubocop:disable Style/GlobalVars

RSpec.describe "Zeitwerk plugin / Eager loading" do
include ZeitwerkHelpers

before do
$eager_loaded = false
end

after { teardown_zeitwerk }
after { ZeitwerkLoaderRegistry.clear }

it "Eager loads after finalization" do
with_tmp_directory do |tmp_dir|
Expand Down
4 changes: 1 addition & 3 deletions spec/integration/container/plugins/zeitwerk/namespaces_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# frozen_string_literal: true

RSpec.describe "Zeitwerk plugin / Namespaces" do
include ZeitwerkHelpers

after { teardown_zeitwerk }
after { ZeitwerkLoaderRegistry.clear }
Copy link
Member

@timriley timriley Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the loaders created in this example group get torn down? Here we're letting the plugin create the loader itself (which is just a plain old ::Zeitwerk::Loader.new), rather than assigning a loader that we've created ourselves via ZeitwerkLoaderRegistry.new_loader.

(This question applies to any of the spec files that follow a similar approach to this one)

Copy link
Contributor Author

@katafrakt katafrakt Dec 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good point. One thing to remedy this could be to just stub Zeitwerk::Loader.new in tests with ZeitwerkLoaderRegistry.new_loader, but I'm not sure how we feel about this.

Copy link
Member

@timriley timriley Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to remedy this could be to just stub Zeitwerk::Loader.new in tests with ZeitwerkLoaderRegistry.new_loader, but I'm not sure how we feel about this.

I think this is no worse than what we were doing to iterate over ObjectSpace and find the classes we want. So I think it's a reasonable move at this point just to keep things us moving forward and unlock JRuby support.

We should at least file an issue about finding a cleaner way of doing this.


it "loads components from a root namespace with a const namespace" do
with_tmp_directory do |tmp_dir|
Expand Down
4 changes: 1 addition & 3 deletions spec/integration/container/plugins/zeitwerk/resolving_components_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# frozen_string_literal: true

RSpec.describe "Zeitwerk plugin / Resolving components" do
include ZeitwerkHelpers

after { teardown_zeitwerk }
after { ZeitwerkLoaderRegistry.clear }

specify "Resolving components using Zeitwerk" do
with_tmp_directory do |tmp_dir|
Expand Down
6 changes: 2 additions & 4 deletions spec/integration/container/plugins/zeitwerk/user_configured_loader_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# frozen_string_literal: true

RSpec.describe "Zeitwerk plugin / User-configured loader" do
include ZeitwerkHelpers

after { teardown_zeitwerk }
after { ZeitwerkLoaderRegistry.clear }

it "uses the user-configured loader and pushes component dirs to it" do
with_tmp_directory do |tmp_dir|
Expand All @@ -26,7 +24,7 @@ class Foo;end
dir.namespaces.add_root const: "test"
end

config.autoloader = Zeitwerk::Loader.new.tap do |loader|
config.autoloader = ZeitwerkLoaderRegistry.new_loader.tap do |loader|
loader.tag = "custom_loader"
loader.logger = -> str { logs << str }
end
Expand Down
16 changes: 0 additions & 16 deletions spec/support/zeitwerk_helpers.rb
View file
Open in desktop

This file was deleted.

23 changes: 23 additions & 0 deletions spec/support/zeitwerk_loader_registry.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

module ZeitwerkLoaderRegistry
class << self
def new_loader
Zeitwerk::Loader.new.tap do |loader|
loaders << loader
end
end

def clear
loaders.each do |loader|
loader.unregister
end
end

private

def loaders
@loaders ||= []
end
end
end