Skip to content

Commit

Permalink
Move require_dependencies suite to the one for dependencies
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fxn committed Aug 10, 2021
1 parent 5e8a26d commit 3401139
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 50 deletions.
Expand Up @@ -43,17 +43,6 @@ def unhook!
end
end

module RequireDependency
def require_dependency(filename)
filename = filename.to_path if filename.respond_to?(:to_path)
if abspath = ActiveSupport::Dependencies.search_for_file(filename)
require abspath
else
require filename
end
end
end

module Inflector
# Concurrent::Map is not needed. This is a private class, and overrides
# must be defined while the application boots.
Expand Down Expand Up @@ -116,7 +105,6 @@ def freeze_paths
def decorate_dependencies
Dependencies.unhook!
Dependencies.singleton_class.prepend(Decorations)
Object.prepend(RequireDependency)
end
end
end
Expand Down
87 changes: 86 additions & 1 deletion activesupport/test/dependencies_test.rb
Expand Up @@ -114,6 +114,91 @@ def test_load_and_require_stay_private
ensure
ActiveSupport::Dependencies.hook!
end
end

class RequireDependencyTest < ActiveSupport::TestCase
setup do
@loaded_features_copy = $LOADED_FEATURES.dup
ActiveSupport::Dependencies.autoload_paths.clear

@root_dir = Dir.mktmpdir
File.write("#{@root_dir}/x.rb", "X = :X")
ActiveSupport::Dependencies.autoload_paths << @root_dir
end

teardown do
$LOADED_FEATURES.replace(@loaded_features_copy)
ActiveSupport::Dependencies.autoload_paths.clear

FileUtils.rm_rf(@root_dir)
Object.send(:remove_const, :X) if Object.const_defined?(:X)
end

test "require_dependency looks autoload paths up" do
assert require_dependency("x")
assert_equal :X, X
end

test "require_dependency looks autoload paths up (idempotent)" do
assert require_dependency("x")
assert !require_dependency("x")
end

test "require_dependency handles absolute paths correctly" do
assert require_dependency("#{@root_dir}/x.rb")
assert_equal :X, X
end

test "require_dependency handles absolute paths correctly (idempotent)" do
assert require_dependency("#{@root_dir}/x.rb")
assert !require_dependency("#{@root_dir}/x.rb")
end

# Coverage for require_dependency can be found in the railties test suite.
test "require_dependency supports arguments that respond to to_path" do
x = Object.new
def x.to_path; "x"; end

assert require_dependency(x)
assert_equal :X, X
end

test "require_dependency supports arguments that respond to to_path (idempotent)" do
x = Object.new
def x.to_path; "x"; end

assert require_dependency(x)
assert !require_dependency(x)
end

test "require_dependency fallback to Kernel#require" do
dir = Dir.mktmpdir
$LOAD_PATH << dir
File.write("#{dir}/y.rb", "Y = :Y")

assert require_dependency("y")
assert_equal :Y, Y
ensure
$LOAD_PATH.pop
Object.send(:remove_const, :Y) if Object.const_defined?(:Y)
end

test "require_dependency fallback to Kernel#require (idempotent)" do
dir = Dir.mktmpdir
$LOAD_PATH << dir
File.write("#{dir}/y.rb", "Y = :Y")

assert require_dependency("y")
assert !require_dependency("y")
ensure
$LOAD_PATH.pop
Object.send(:remove_const, :Y) if Object.const_defined?(:Y)
end

test "require_dependency raises ArgumentError if the argument is not a String and does not respond to #to_path" do
assert_raises(ArgumentError) { require_dependency(Object.new) }
end

test "require_dependency raises LoadError if the given argument is not found" do
assert_raise(LoadError) { require_dependency("nonexistent_filename") }
end
end
37 changes: 0 additions & 37 deletions railties/test/application/zeitwerk_integration_test.rb
Expand Up @@ -139,43 +139,6 @@ def self.name
assert_empty deps.autoloaded_constants
end

[true, false].each do |add_aps_to_lp|
test "require_dependency looks autoload paths up (#{add_aps_to_lp})" do
add_to_config "config.add_autoload_paths_to_load_path = #{add_aps_to_lp}"
app_file "app/models/user.rb", "class User; end"
boot

assert require_dependency("user")
end

test "require_dependency handles absolute paths correctly (#{add_aps_to_lp})" do
add_to_config "config.add_autoload_paths_to_load_path = #{add_aps_to_lp}"
app_file "app/models/user.rb", "class User; end"
boot

assert require_dependency("#{app_path}/app/models/user.rb")
end

test "require_dependency supports arguments that respond to to_path (#{add_aps_to_lp})" do
add_to_config "config.add_autoload_paths_to_load_path = #{add_aps_to_lp}"
app_file "app/models/user.rb", "class User; end"
boot

user = Object.new
def user.to_path; "user"; end

assert require_dependency(user)
end
end

test "require_dependency raises ArgumentError if the argument is not a String and does not respond to #to_path" do
assert_raises(ArgumentError) { require_dependency(Object.new) }
end

test "require_dependency raises LoadError if the given argument is not found" do
assert_raise(LoadError) { require_dependency("nonexistent_filename") }
end

test "eager loading loads the application code" do
$zeitwerk_integration_test_user = false
$zeitwerk_integration_test_post = false
Expand Down

0 comments on commit 3401139

Please sign in to comment.