Skip to content

Commit 3401139

Browse files
committed
Move require_dependencies suite to the one for dependencies
require_dependency has now one single definition, and its implementation does not depend on Zeitwerk or even applications. It only depends on having some autoload paths in place. We can test that in the AS test suite.
1 parent 5e8a26d commit 3401139

File tree

3 files changed

+86
-50
lines changed

3 files changed

+86
-50
lines changed

activesupport/lib/active_support/dependencies/zeitwerk_integration.rb

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,6 @@ def unhook!
4343
end
4444
end
4545

46-
module RequireDependency
47-
def require_dependency(filename)
48-
filename = filename.to_path if filename.respond_to?(:to_path)
49-
if abspath = ActiveSupport::Dependencies.search_for_file(filename)
50-
require abspath
51-
else
52-
require filename
53-
end
54-
end
55-
end
56-
5746
module Inflector
5847
# Concurrent::Map is not needed. This is a private class, and overrides
5948
# must be defined while the application boots.
@@ -116,7 +105,6 @@ def freeze_paths
116105
def decorate_dependencies
117106
Dependencies.unhook!
118107
Dependencies.singleton_class.prepend(Decorations)
119-
Object.prepend(RequireDependency)
120108
end
121109
end
122110
end

activesupport/test/dependencies_test.rb

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,91 @@ def test_load_and_require_stay_private
114114
ensure
115115
ActiveSupport::Dependencies.hook!
116116
end
117+
end
118+
119+
class RequireDependencyTest < ActiveSupport::TestCase
120+
setup do
121+
@loaded_features_copy = $LOADED_FEATURES.dup
122+
ActiveSupport::Dependencies.autoload_paths.clear
123+
124+
@root_dir = Dir.mktmpdir
125+
File.write("#{@root_dir}/x.rb", "X = :X")
126+
ActiveSupport::Dependencies.autoload_paths << @root_dir
127+
end
128+
129+
teardown do
130+
$LOADED_FEATURES.replace(@loaded_features_copy)
131+
ActiveSupport::Dependencies.autoload_paths.clear
132+
133+
FileUtils.rm_rf(@root_dir)
134+
Object.send(:remove_const, :X) if Object.const_defined?(:X)
135+
end
136+
137+
test "require_dependency looks autoload paths up" do
138+
assert require_dependency("x")
139+
assert_equal :X, X
140+
end
141+
142+
test "require_dependency looks autoload paths up (idempotent)" do
143+
assert require_dependency("x")
144+
assert !require_dependency("x")
145+
end
146+
147+
test "require_dependency handles absolute paths correctly" do
148+
assert require_dependency("#{@root_dir}/x.rb")
149+
assert_equal :X, X
150+
end
151+
152+
test "require_dependency handles absolute paths correctly (idempotent)" do
153+
assert require_dependency("#{@root_dir}/x.rb")
154+
assert !require_dependency("#{@root_dir}/x.rb")
155+
end
117156

118-
# Coverage for require_dependency can be found in the railties test suite.
157+
test "require_dependency supports arguments that respond to to_path" do
158+
x = Object.new
159+
def x.to_path; "x"; end
160+
161+
assert require_dependency(x)
162+
assert_equal :X, X
163+
end
164+
165+
test "require_dependency supports arguments that respond to to_path (idempotent)" do
166+
x = Object.new
167+
def x.to_path; "x"; end
168+
169+
assert require_dependency(x)
170+
assert !require_dependency(x)
171+
end
172+
173+
test "require_dependency fallback to Kernel#require" do
174+
dir = Dir.mktmpdir
175+
$LOAD_PATH << dir
176+
File.write("#{dir}/y.rb", "Y = :Y")
177+
178+
assert require_dependency("y")
179+
assert_equal :Y, Y
180+
ensure
181+
$LOAD_PATH.pop
182+
Object.send(:remove_const, :Y) if Object.const_defined?(:Y)
183+
end
184+
185+
test "require_dependency fallback to Kernel#require (idempotent)" do
186+
dir = Dir.mktmpdir
187+
$LOAD_PATH << dir
188+
File.write("#{dir}/y.rb", "Y = :Y")
189+
190+
assert require_dependency("y")
191+
assert !require_dependency("y")
192+
ensure
193+
$LOAD_PATH.pop
194+
Object.send(:remove_const, :Y) if Object.const_defined?(:Y)
195+
end
196+
197+
test "require_dependency raises ArgumentError if the argument is not a String and does not respond to #to_path" do
198+
assert_raises(ArgumentError) { require_dependency(Object.new) }
199+
end
200+
201+
test "require_dependency raises LoadError if the given argument is not found" do
202+
assert_raise(LoadError) { require_dependency("nonexistent_filename") }
203+
end
119204
end

railties/test/application/zeitwerk_integration_test.rb

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -139,43 +139,6 @@ def self.name
139139
assert_empty deps.autoloaded_constants
140140
end
141141

142-
[true, false].each do |add_aps_to_lp|
143-
test "require_dependency looks autoload paths up (#{add_aps_to_lp})" do
144-
add_to_config "config.add_autoload_paths_to_load_path = #{add_aps_to_lp}"
145-
app_file "app/models/user.rb", "class User; end"
146-
boot
147-
148-
assert require_dependency("user")
149-
end
150-
151-
test "require_dependency handles absolute paths correctly (#{add_aps_to_lp})" do
152-
add_to_config "config.add_autoload_paths_to_load_path = #{add_aps_to_lp}"
153-
app_file "app/models/user.rb", "class User; end"
154-
boot
155-
156-
assert require_dependency("#{app_path}/app/models/user.rb")
157-
end
158-
159-
test "require_dependency supports arguments that respond to to_path (#{add_aps_to_lp})" do
160-
add_to_config "config.add_autoload_paths_to_load_path = #{add_aps_to_lp}"
161-
app_file "app/models/user.rb", "class User; end"
162-
boot
163-
164-
user = Object.new
165-
def user.to_path; "user"; end
166-
167-
assert require_dependency(user)
168-
end
169-
end
170-
171-
test "require_dependency raises ArgumentError if the argument is not a String and does not respond to #to_path" do
172-
assert_raises(ArgumentError) { require_dependency(Object.new) }
173-
end
174-
175-
test "require_dependency raises LoadError if the given argument is not found" do
176-
assert_raise(LoadError) { require_dependency("nonexistent_filename") }
177-
end
178-
179142
test "eager loading loads the application code" do
180143
$zeitwerk_integration_test_user = false
181144
$zeitwerk_integration_test_post = false

0 commit comments

Comments
 (0)