diff --git a/USAGE.md b/USAGE.md index 4d77b052d..6c26de3a6 100644 --- a/USAGE.md +++ b/USAGE.md @@ -72,15 +72,16 @@ Secondly, to enable Spring, first run `bin/spring binstub packwerk` which will " Packwerk reads from the `packwerk.yml` configuration file in the root directory. Packwerk will run with the default configuration if any of these settings are not specified. -| Key | Default value | Description | -|----------------------|-------------------------------------------|--------------| -| include | **/*.{rb,rake,erb} | list of patterns for folder paths to include | -| exclude | {bin,node_modules,script,tmp,vendor}/**/* | list of patterns for folder paths to exclude | -| package_paths | **/ | a single pattern or a list of patterns to find package configuration files, see: [Defining packages](#Defining-packages) | -| custom_associations | N/A | list of custom associations, if any | -| parallel | true | when true, fork code parsing out to subprocesses | -| cache | false | when true, caches the results of parsing files | -| cache_directory | tmp/cache/packwerk | the directory that will hold the packwerk cache | +| Key | Default value | Description | +|-------------------------------------|-------------------------------------------|--------------------------------------------------------------------------------------------------------------------------| +| include | **/*.{rb,rake,erb} | list of patterns for folder paths to include | +| exclude | {bin,node_modules,script,tmp,vendor}/**/* | list of patterns for folder paths to exclude | +| package_paths | **/ | a single pattern or a list of patterns to find package configuration files, see: [Defining packages](#Defining-packages) | +| custom_associations | N/A | list of custom associations, if any | +| parallel | true | when true, fork code parsing out to subprocesses | +| cache | false | when true, caches the results of parsing files | +| cache_directory | tmp/cache/packwerk | the directory that will hold the packwerk cache | +| packages_outside_of_app_dir_enabled | false | when true, packwerk will scan for packages in folders for locally published rails engines | ### Using a custom ERB parser diff --git a/lib/packwerk.rb b/lib/packwerk.rb index 39023d1ca..a7b11b9e1 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -27,6 +27,8 @@ module Packwerk autoload :Package autoload :PackageSet autoload :PackageTodo + autoload :PackagePaths + autoload :ParsedConstantDefinitions autoload :Parsers autoload :RailsLoadPaths autoload :Reference diff --git a/lib/packwerk/configuration.rb b/lib/packwerk/configuration.rb index 22a7dee20..11302f3f7 100644 --- a/lib/packwerk/configuration.rb +++ b/lib/packwerk/configuration.rb @@ -60,6 +60,9 @@ def from_packwerk_config(path) sig { returns(Pathname) } attr_reader(:cache_directory) + sig { returns(T::Boolean) } + attr_reader(:packages_outside_of_app_dir_enabled) + sig { params(parallel: T::Boolean).returns(T::Boolean) } attr_writer(:parallel) @@ -80,6 +83,7 @@ def initialize(configs = {}, config_path: nil) @cache_enabled = T.let(configs.key?("cache") ? configs["cache"] : false, T::Boolean) @cache_directory = T.let(Pathname.new(configs["cache_directory"] || "tmp/cache/packwerk"), Pathname) @config_path = config_path + @packages_outside_of_app_dir_enabled = configs["packages_outside_of_app_dir_enabled"] || false @offenses_formatter_identifier = T.let( configs["offenses_formatter"] || Formatters::DefaultOffensesFormatter::IDENTIFIER, String diff --git a/lib/packwerk/package_paths.rb b/lib/packwerk/package_paths.rb new file mode 100644 index 000000000..59cb80d8f --- /dev/null +++ b/lib/packwerk/package_paths.rb @@ -0,0 +1,71 @@ +# typed: strict +# frozen_string_literal: true + +require "pathname" +require "bundler" + +module Packwerk + class PackagePaths + PACKAGE_CONFIG_FILENAME = "package.yml" + PathSpec = T.type_alias { T.any(String, T::Array[String]) } + + extend T::Sig + extend T::Generic + + sig do + params( + root_path: String, + package_pathspec: T.nilable(PathSpec), + exclude_pathspec: T.nilable(PathSpec), + scan_for_packages_outside_of_app_dir: T.nilable(T::Boolean) + ).void + end + def initialize(root_path, package_pathspec, exclude_pathspec = nil, scan_for_packages_outside_of_app_dir = false) + @root_path = root_path + @package_pathspec = package_pathspec + @exclude_pathspec = exclude_pathspec + @scan_for_packages_outside_of_app_dir = scan_for_packages_outside_of_app_dir + end + + sig {returns(T::Array[Pathname])} + def all_paths + exclude_pathspec = Array(@exclude_pathspec).dup + .push(Bundler.bundle_path.join("**").to_s) + .map { |glob| File.expand_path(glob) } + + paths_to_scan = if @scan_for_packages_outside_of_app_dir + engine_paths_to_scan.push(@root_path) + else + [@root_path] + end + + glob_patterns = paths_to_scan.product(Array(@package_pathspec)).map do |path, pathspec| + File.join(path, pathspec, PACKAGE_CONFIG_FILENAME) + end + + Dir.glob(glob_patterns) + .map { |path| Pathname.new(path).cleanpath } + .reject { |path| exclude_path?(exclude_pathspec, path) } + end + + private + + sig { params(globs: T::Array[String], path: Pathname).returns(T::Boolean) } + def exclude_path?(globs, path) + globs.any? do |glob| + path.realpath.fnmatch(glob, File::FNM_EXTGLOB) + end + end + + sig { returns(T::Array[String]) } + def engine_paths_to_scan + bundle_path_match = Bundler.bundle_path.join("**") + + Rails.application.railties + .select { |r| r.is_a?(Rails::Engine) } + .map { |r| Pathname.new(r.root).expand_path } + .reject { |path| path.fnmatch(bundle_path_match.to_s) } # reject paths from vendored gems + .map(&:to_s) + end + end +end diff --git a/lib/packwerk/package_set.rb b/lib/packwerk/package_set.rb index 251c36f59..930e196c1 100644 --- a/lib/packwerk/package_set.rb +++ b/lib/packwerk/package_set.rb @@ -5,7 +5,6 @@ require "bundler" module Packwerk - PathSpec = T.type_alias { T.any(String, T::Array[String]) } # A set of {Packwerk::Package}s as well as methods to parse packages from the filesystem. class PackageSet @@ -20,11 +19,14 @@ class PackageSet class << self extend T::Sig - sig { params(root_path: String, package_pathspec: T.nilable(PathSpec)).returns(PackageSet) } - def load_all_from(root_path, package_pathspec: nil) - package_paths = package_paths(root_path, package_pathspec || "**") + sig do + params(root_path: String, package_pathspec: T.nilable(PackagePaths::PathSpec), + scan_for_packages_outside_of_app_dir: T.nilable(T::Boolean)).returns(PackageSet) + end + def load_all_from(root_path, package_pathspec: nil, scan_for_packages_outside_of_app_dir: false) + package_paths = PackagePaths.new(root_path, package_pathspec || "**", nil, scan_for_packages_outside_of_app_dir) - packages = package_paths.map do |path| + packages = package_paths.all_paths.map do |path| root_relative = path.dirname.relative_path_from(root_path) Package.new(name: root_relative.to_s, config: YAML.load_file(path, fallback: nil)) end @@ -34,27 +36,6 @@ def load_all_from(root_path, package_pathspec: nil) new(packages) end - sig do - params( - root_path: String, - package_pathspec: PathSpec, - exclude_pathspec: T.nilable(PathSpec) - ).returns(T::Array[Pathname]) - end - def package_paths(root_path, package_pathspec, exclude_pathspec = []) - exclude_pathspec = Array(exclude_pathspec).dup - .push(Bundler.bundle_path.join("**").to_s) - .map { |glob| File.expand_path(glob) } - - glob_patterns = Array(package_pathspec).map do |pathspec| - File.join(root_path, pathspec, PACKAGE_CONFIG_FILENAME) - end - - Dir.glob(glob_patterns) - .map { |path| Pathname.new(path).cleanpath } - .reject { |path| exclude_path?(exclude_pathspec, path) } - end - private sig { params(packages: T::Array[Package]).void } @@ -64,12 +45,7 @@ def create_root_package_if_none_in(packages) packages << Package.new(name: Package::ROOT_PACKAGE_NAME, config: nil) end - sig { params(globs: T::Array[String], path: Pathname).returns(T::Boolean) } - def exclude_path?(globs, path) - globs.any? do |glob| - path.realpath.fnmatch(glob, File::FNM_EXTGLOB) - end - end + end sig { returns(T::Hash[String, Package]) } diff --git a/lib/packwerk/rails_load_paths.rb b/lib/packwerk/rails_load_paths.rb index c2b2336e3..b7189c248 100644 --- a/lib/packwerk/rails_load_paths.rb +++ b/lib/packwerk/rails_load_paths.rb @@ -35,11 +35,9 @@ def extract_application_autoload_paths end def filter_relevant_paths(all_paths, bundle_path: Bundler.bundle_path, rails_root: Rails.root) bundle_path_match = bundle_path.join("**") - rails_root_match = rails_root.join("**") all_paths .transform_keys { |path| Pathname.new(path).expand_path } - .select { |path| path.fnmatch(rails_root_match.to_s) } # path needs to be in application directory .reject { |path| path.fnmatch(bundle_path_match.to_s) } # reject paths from vendored gems end diff --git a/lib/packwerk/run_context.rb b/lib/packwerk/run_context.rb index 8dae554a1..1d4a7b7dd 100644 --- a/lib/packwerk/run_context.rb +++ b/lib/packwerk/run_context.rb @@ -21,6 +21,7 @@ def from_configuration(configuration) root_path: configuration.root_path, load_paths: configuration.load_paths, package_paths: configuration.package_paths, + packages_outside_of_app_dir_enabled: configuration.packages_outside_of_app_dir_enabled, inflector: inflector, custom_associations: configuration.custom_associations, cache_enabled: configuration.cache_enabled?, @@ -41,6 +42,7 @@ def from_configuration(configuration) custom_associations: AssociationInspector::CustomAssociations, checkers: T::Array[Checker], cache_enabled: T::Boolean, + packages_outside_of_app_dir_enabled: T.nilable(T::Boolean), ).void end def initialize( @@ -52,7 +54,8 @@ def initialize( package_paths: nil, custom_associations: [], checkers: Checker.all, - cache_enabled: false + cache_enabled: false, + packages_outside_of_app_dir_enabled: false ) @root_path = root_path @load_paths = load_paths @@ -63,7 +66,7 @@ def initialize( @cache_enabled = cache_enabled @cache_directory = cache_directory @config_path = config_path - + @packages_outside_of_app_dir_enabled = packages_outside_of_app_dir_enabled @file_processor = T.let(nil, T.nilable(FileProcessor)) @context_provider = T.let(nil, T.nilable(ConstantDiscovery)) @package_set = T.let(nil, T.nilable(PackageSet)) @@ -77,10 +80,13 @@ def initialize( def process_file(relative_file:) processed_file = file_processor.call(relative_file) + puts relative_file + references = ReferenceExtractor.get_fully_qualified_references_from( processed_file.unresolved_references, context_provider ) + puts references reference_checker = ReferenceChecking::ReferenceChecker.new(@checkers) processed_file.offenses + references.flat_map { |reference| reference_checker.call(reference) } @@ -88,7 +94,13 @@ def process_file(relative_file:) sig { returns(PackageSet) } def package_set - @package_set ||= ::Packwerk::PackageSet.load_all_from(@root_path, package_pathspec: @package_paths) + @package_set ||= ::Packwerk::PackageSet.load_all_from( + @root_path, + package_pathspec: @package_paths, + scan_for_packages_outside_of_app_dir: @packages_outside_of_app_dir_enabled, + ) + puts @package_set.packages + @package_set end private @@ -123,7 +135,7 @@ def resolver inflector: @inflector, ) end - + sig { returns(T::Array[ConstantNameInspector]) } def constant_name_inspectors [ diff --git a/lib/packwerk/validator.rb b/lib/packwerk/validator.rb index a79de1eea..8f08ac05f 100644 --- a/lib/packwerk/validator.rb +++ b/lib/packwerk/validator.rb @@ -61,8 +61,13 @@ def package_manifests_settings_for(configuration, setting) end def package_manifests(configuration, glob_pattern = nil) glob_pattern ||= package_glob(configuration) - PackageSet.package_paths(configuration.root_path, glob_pattern, configuration.exclude) - .map { |f| File.realpath(f) } + package_paths = PackagePaths.new( + configuration.root_path, + glob_pattern, + configuration.exclude, + configuration.packages_outside_of_app_dir_enabled + ) + package_paths.all_paths.map { |f| File.realpath(f) } end sig { params(configuration: Configuration).returns(T.any(T::Array[String], String)) } diff --git a/test/fixtures/external_packages/app/components/timeline/app/models/concerns/has_timeline.rb b/test/fixtures/external_packages/app/components/timeline/app/models/concerns/has_timeline.rb new file mode 100644 index 000000000..92461a137 --- /dev/null +++ b/test/fixtures/external_packages/app/components/timeline/app/models/concerns/has_timeline.rb @@ -0,0 +1,5 @@ +# typed: ignore +# frozen_string_literal: true + +module HasTimeline +end diff --git a/test/fixtures/external_packages/app/components/timeline/app/models/imaginary/.gitkeep b/test/fixtures/external_packages/app/components/timeline/app/models/imaginary/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/test/fixtures/external_packages/app/components/timeline/app/models/private_thing.rb b/test/fixtures/external_packages/app/components/timeline/app/models/private_thing.rb new file mode 100644 index 000000000..6ae09c969 --- /dev/null +++ b/test/fixtures/external_packages/app/components/timeline/app/models/private_thing.rb @@ -0,0 +1,2 @@ +class PrivateThing +end diff --git a/test/fixtures/external_packages/app/components/timeline/app/models/sales/.gitkeep b/test/fixtures/external_packages/app/components/timeline/app/models/sales/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/test/fixtures/external_packages/app/components/timeline/nested/package.yml b/test/fixtures/external_packages/app/components/timeline/nested/package.yml new file mode 100644 index 000000000..c0ce986be --- /dev/null +++ b/test/fixtures/external_packages/app/components/timeline/nested/package.yml @@ -0,0 +1 @@ +enforce_dependencies: true \ No newline at end of file diff --git a/test/fixtures/external_packages/app/components/timeline/package.yml b/test/fixtures/external_packages/app/components/timeline/package.yml new file mode 100644 index 000000000..46f5ff725 --- /dev/null +++ b/test/fixtures/external_packages/app/components/timeline/package.yml @@ -0,0 +1,2 @@ +enforce_privacy: + - "::PrivateThing" diff --git a/test/fixtures/external_packages/app/config/environment.rb b/test/fixtures/external_packages/app/config/environment.rb new file mode 100644 index 000000000..e69de29bb diff --git a/test/fixtures/external_packages/app/package.yml b/test/fixtures/external_packages/app/package.yml new file mode 100644 index 000000000..e69de29bb diff --git a/test/fixtures/external_packages/app/packwerk.yml b/test/fixtures/external_packages/app/packwerk.yml new file mode 100644 index 000000000..dca3889ac --- /dev/null +++ b/test/fixtures/external_packages/app/packwerk.yml @@ -0,0 +1 @@ +packages_outside_of_app_dir_enabled: true \ No newline at end of file diff --git a/test/fixtures/external_packages/sales/components/sales/app/models/order.rb b/test/fixtures/external_packages/sales/components/sales/app/models/order.rb new file mode 100644 index 000000000..627a03838 --- /dev/null +++ b/test/fixtures/external_packages/sales/components/sales/app/models/order.rb @@ -0,0 +1,5 @@ +# typed: ignore +# frozen_string_literal: true + +class Order +end diff --git a/test/fixtures/external_packages/sales/components/sales/app/models/sales/order.rb b/test/fixtures/external_packages/sales/components/sales/app/models/sales/order.rb new file mode 100644 index 000000000..125b83364 --- /dev/null +++ b/test/fixtures/external_packages/sales/components/sales/app/models/sales/order.rb @@ -0,0 +1,7 @@ +# typed: ignore +# frozen_string_literal: true + +module Sales + class Order + end +end diff --git a/test/fixtures/external_packages/sales/components/sales/app/public/sales/record_new_order.rb b/test/fixtures/external_packages/sales/components/sales/app/public/sales/record_new_order.rb new file mode 100644 index 000000000..ed72bc35b --- /dev/null +++ b/test/fixtures/external_packages/sales/components/sales/app/public/sales/record_new_order.rb @@ -0,0 +1,7 @@ +# typed: ignore +# frozen_string_literal: true + +module Sales + module RecordNewOrder + end +end diff --git a/test/fixtures/external_packages/sales/components/sales/package.yml b/test/fixtures/external_packages/sales/components/sales/package.yml new file mode 100644 index 000000000..6c9ff1ee9 --- /dev/null +++ b/test/fixtures/external_packages/sales/components/sales/package.yml @@ -0,0 +1,8 @@ +--- +enforce_privacy: true + +metadata: + stewards: + - "@Shopify/sales" + slack_channels: + - "#sales" diff --git a/test/integration/packwerk/custom_executable_integration_test.rb b/test/integration/packwerk/custom_executable_integration_test.rb index 2289a46fd..bcb1c4a02 100644 --- a/test/integration/packwerk/custom_executable_integration_test.rb +++ b/test/integration/packwerk/custom_executable_integration_test.rb @@ -6,14 +6,13 @@ module Packwerk class CustomExecutableIntegrationTest < Minitest::Test - include ApplicationFixtureHelper + include RailsApplicationFixtureHelper TIMELINE_PATH = Pathname.new("components/timeline") setup do reset_output setup_application_fixture - use_template(:skeleton) end teardown do @@ -21,6 +20,7 @@ class CustomExecutableIntegrationTest < Minitest::Test end test "'packwerk check' with no violations succeeds in all variants" do + use_template(:skeleton) assert_successful_run("check") assert_match(/No offenses detected/, captured_output) @@ -34,6 +34,7 @@ class CustomExecutableIntegrationTest < Minitest::Test end test "'packwerk check' with violations only in nested packages has different outcomes per variant" do + use_template(:skeleton) open_app_file(TIMELINE_PATH.join("nested", "timeline_comment.rb")) do |file| file.write("class TimelineComment; belongs_to :order, class_name: '::Order'; end") file.flush @@ -54,6 +55,7 @@ class CustomExecutableIntegrationTest < Minitest::Test end test "'packwerk check' with failures in different parts of the app has different outcomes per variant" do + use_template(:skeleton) open_app_file(TIMELINE_PATH.join("nested", "timeline_comment.rb")) do |file| file.write("class TimelineComment; belongs_to :order, class_name: '::Order'; end") file.flush @@ -73,6 +75,7 @@ class CustomExecutableIntegrationTest < Minitest::Test end test "'packwerk update-todo' with no violations succeeds and updates no files" do + use_template(:skeleton) package_todo_content = read_package_todo assert_successful_run("update-todo") @@ -93,6 +96,7 @@ class CustomExecutableIntegrationTest < Minitest::Test end test "'packwerk update-todo' with violations succeeds and updates relevant package_todo" do + use_template(:skeleton) package_todo_content = read_package_todo timeline_package_todo_path = to_app_path(File.join(TIMELINE_PATH, "package_todo.yml")) @@ -129,6 +133,7 @@ class CustomExecutableIntegrationTest < Minitest::Test end test "'packwerk check' does not blow up when parsing files with syntax issues from a false positive association" do + use_template(:skeleton) open_app_file(TIMELINE_PATH.join("app", "models", "bad_file.rb")) do |file| # This is an example of a file that has an object called `belongs_to` that accepts methods content = <<~CONTENT @@ -151,6 +156,19 @@ class CustomExecutableIntegrationTest < Minitest::Test end end + test "'packwerk check' for an app with external packages it detects a violation referencing a constant in the external package" do + use_template(:external_packages) + + open_app_file(TIMELINE_PATH.join("nested", "timeline_comment.rb")) do |file| + file.write("class TimelineComment; belongs_to :order, class_name: '::Order'; end") + file.flush + end + + refute_successful_run("check") + assert_match(/Dependency violation: ::Order/, captured_output) + assert_match(/1 offense detected/, captured_output) + end + private def assert_successful_run(command) diff --git a/test/support/packwerk/application_fixture_helper.rb b/test/support/packwerk/application_fixture_helper.rb index bc0f6b96c..3db777a95 100644 --- a/test/support/packwerk/application_fixture_helper.rb +++ b/test/support/packwerk/application_fixture_helper.rb @@ -35,6 +35,10 @@ def use_template(template) raise "use_template may only be called once per test" if using_template? copy_dir("test/fixtures/#{template}") + if template == :external_packages + @app_dir = File.join(app_dir, "./app") + end + Dir.chdir(app_dir) end diff --git a/test/support/rails_application_fixture_helper.rb b/test/support/rails_application_fixture_helper.rb index 8e5e13857..671f7f1e5 100644 --- a/test/support/rails_application_fixture_helper.rb +++ b/test/support/rails_application_fixture_helper.rb @@ -32,16 +32,21 @@ def use_template(template) Rails.stubs(:autoloaders).returns(Autoloaders.new) + root = Pathname.new(app_dir) + case template when :minimal set_load_paths_for_minimal_template when :skeleton set_load_paths_for_skeleton_template + when :external_packages + set_load_paths_for_external_packages_template + create_new_engine_at_path(*to_app_paths("../sales/components/sales/")) + Rails.application.stubs(:railties).returns(Rails::Engine::Railties.new) else raise "Unknown fixture template #{template}" end - root = Pathname.new(app_dir) Rails.application.config.stubs(:root).returns(root) end @@ -59,4 +64,19 @@ def set_load_paths_for_skeleton_template Rails.autoloaders.once.push_dir(*to_app_paths("components/timeline/app/models/concerns")) Rails.autoloaders.once.push_dir(*to_app_paths("vendor/cache/gems/example/models")) end + + def set_load_paths_for_external_packages_template + Rails.autoloaders.main.push_dir(*to_app_paths("/components/timeline/app/models")) + + Rails.autoloaders.once.push_dir(*to_app_paths("../sales/components/sales/app/models")) + end + + def create_new_engine_at_path(path) + Class.new(Rails::Engine) do + T.bind(self, Class) + define_method(:root) do + path + end + end + end end diff --git a/test/unit/package_paths_test.rb b/test/unit/package_paths_test.rb new file mode 100644 index 000000000..4a2f427a8 --- /dev/null +++ b/test/unit/package_paths_test.rb @@ -0,0 +1,93 @@ +# typed: true +# frozen_string_literal: true + +require "test_helper" + +module Packwerk + class PackagePathsTest < Minitest::Test + include RailsApplicationFixtureHelper + + setup do + setup_application_fixture + end + + teardown do + teardown_application_fixture + end + + test "#all_paths supports a path wildcard" do + use_template(:skeleton) + package_paths = PackagePaths.new(".", "**") + + assert_includes(package_paths.all_paths, Pathname.new("components/sales/package.yml")) + assert_includes(package_paths.all_paths, Pathname.new("package.yml")) + end + + test "#all_paths supports a single path as a string" do + use_template(:skeleton) + package_paths = PackagePaths.new(".", "components/sales") + + assert_equal(package_paths.all_paths, [Pathname.new("components/sales/package.yml")]) + end + + test "#all_paths supports many paths as an array" do + use_template(:skeleton) + package_paths = PackagePaths.new(".", ["components/sales", "."]) + + assert_equal( + package_paths.all_paths, + [ + Pathname.new("components/sales/package.yml"), + Pathname.new("package.yml"), + ] + ) + end + + test "#all_paths excludes paths inside the gem directory" do + use_template(:skeleton) + vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml") + + package_paths = PackagePaths.new(".", "**") + assert_includes(package_paths.all_paths, vendor_package_path) + + Bundler.expects(:bundle_path).once.returns(Rails.root.join("vendor/cache/gems")) + package_paths = PackagePaths.new(".", "**") + refute_includes(package_paths.all_paths, vendor_package_path) + end + + test "#all_paths excludes paths in exclude pathspec" do + use_template(:skeleton) + vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml") + + package_paths = PackagePaths.new(".", "**") + assert_includes(package_paths.all_paths, vendor_package_path) + + package_paths = PackagePaths.new(".", "**", "vendor/*") + refute_includes(package_paths.all_paths, vendor_package_path) + end + + test "#all_paths excludes paths in multiple exclude pathspecs" do + use_template(:skeleton) + + vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml") + sales_package_path = Pathname.new("components/sales/package.yml") + + package_paths = PackagePaths.new(".", "**") + assert_includes(package_paths.all_paths, vendor_package_path) + assert_includes(package_paths.all_paths, sales_package_path) + + package_paths = PackagePaths.new(".", "**", ["vendor/*", "components/sales/*"]) + refute_includes(package_paths.all_paths, vendor_package_path) + refute_includes(package_paths.all_paths, sales_package_path) + end + + test "#all_paths ignores empty excludes" do + use_template(:skeleton) + + assert_equal( + PackagePaths.new(".", "**").all_paths, + PackagePaths.new(".", "**", []).all_paths + ) + end + end +end diff --git a/test/unit/packwerk/application_validator_test.rb b/test/unit/packwerk/application_validator_test.rb index 998d4bf9e..27f824c60 100644 --- a/test/unit/packwerk/application_validator_test.rb +++ b/test/unit/packwerk/application_validator_test.rb @@ -42,7 +42,7 @@ class ApplicationValidatorTest < Minitest::Test merge_into_app_yaml_file("packwerk.yml", { "package_paths" => ["components/**/*", "."] }) merge_into_app_yaml_file("packwerk.yml", { "exclude" => ["vendor/**/*"] }) - package_paths = PackageSet.package_paths(".", "**") + package_paths = PackagePaths.new(".", "**").all_paths vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml") assert_includes(package_paths, vendor_package_path) diff --git a/test/unit/packwerk/package_set_test.rb b/test/unit/packwerk/package_set_test.rb index 18ff2bc93..da5d8ef50 100644 --- a/test/unit/packwerk/package_set_test.rb +++ b/test/unit/packwerk/package_set_test.rb @@ -9,8 +9,6 @@ class PackageSetTest < Minitest::Test setup do setup_application_fixture - use_template(:skeleton) - @package_set = PackageSet.load_all_from(app_dir) end teardown do @@ -18,92 +16,51 @@ class PackageSetTest < Minitest::Test end test "#package_from_path returns package instance for a known path" do - assert_equal("components/timeline", @package_set.package_from_path("components/timeline/something.rb").name) + use_template(:skeleton) + package_set = PackageSet.load_all_from(app_dir) + + assert_equal("components/timeline", package_set.package_from_path("components/timeline/something.rb").name) end test "#package_from_path returns root package for an unpackaged path" do - assert_equal(".", @package_set.package_from_path("components/unknown/something.rb").name) + use_template(:skeleton) + package_set = PackageSet.load_all_from(app_dir) + + assert_equal(".", package_set.package_from_path("components/unknown/something.rb").name) end test "#package_from_path returns nested packages" do + use_template(:skeleton) + package_set = PackageSet.load_all_from(app_dir) + assert_equal( "components/timeline/nested", - @package_set.package_from_path("components/timeline/nested/something.rb").name + package_set.package_from_path("components/timeline/nested/something.rb").name ) end - test "#fetch returns a package instance for known package name" do - assert_equal("components/timeline", @package_set.fetch("components/timeline").name) - end - - test "#fetch returns nil for unknown package name" do - assert_nil(@package_set.fetch("components/unknown")) - end - - test ".package_paths supports a path wildcard" do - package_paths = PackageSet.package_paths(".", "**") - - assert_includes(package_paths, Pathname.new("components/sales/package.yml")) - assert_includes(package_paths, Pathname.new("package.yml")) - end - - test ".package_paths supports a single path as a string" do - package_paths = PackageSet.package_paths(".", "components/sales") - - assert_equal(package_paths, [Pathname.new("components/sales/package.yml")]) - end - - test ".package_paths supports many paths as an array" do - package_paths = PackageSet.package_paths(".", ["components/sales", "."]) + test "#package_from_path returns a package from an external package in autoload paths" do + use_template(:external_packages) + package_set = PackageSet.load_all_from(app_dir, scan_for_packages_outside_of_app_dir: true) assert_equal( - package_paths, - [ - Pathname.new("components/sales/package.yml"), - Pathname.new("package.yml"), - ] + package_set.package_from_path("../sales/components/sales/app/models/order.rb").name, + "../sales/components/sales" ) end - test ".package_paths excludes paths inside the gem directory" do - vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml") - - package_paths = PackageSet.package_paths(".", "**") - assert_includes(package_paths, vendor_package_path) - - Bundler.expects(:bundle_path).returns(Rails.root.join("vendor/cache/gems")) - package_paths = PackageSet.package_paths(".", "**") - refute_includes(package_paths, vendor_package_path) - end - - test ".package_paths excludes paths in exclude pathspec" do - vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml") - - package_paths = PackageSet.package_paths(".", "**") - assert_includes(package_paths, vendor_package_path) + test "#fetch returns a package instance for known package name" do + use_template(:skeleton) + package_set = PackageSet.load_all_from(app_dir) - package_paths = PackageSet.package_paths(".", "**", "vendor/*") - refute_includes(package_paths, vendor_package_path) + assert_equal("components/timeline", T.must(package_set.fetch("components/timeline")).name) end - test ".package_paths excludes paths in multiple exclude pathspecs" do - vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml") - sales_package_path = Pathname.new("components/sales/package.yml") - - package_paths = PackageSet.package_paths(".", "**") - assert_includes(package_paths, vendor_package_path) - assert_includes(package_paths, sales_package_path) - - package_paths = PackageSet.package_paths(".", "**", ["vendor/*", "components/sales/*"]) - refute_includes(package_paths, vendor_package_path) - refute_includes(package_paths, sales_package_path) - end + test "#fetch returns nil for unknown package name" do + use_template(:skeleton) + package_set = PackageSet.load_all_from(app_dir) - test ".package_paths ignores empty excludes" do - assert_equal( - PackageSet.package_paths(".", "**"), - PackageSet.package_paths(".", "**", []) - ) + assert_nil(package_set.fetch("components/unknown")) end end end diff --git a/test/unit/packwerk/rails_load_paths_test.rb b/test/unit/packwerk/rails_load_paths_test.rb index c3e5841b3..e781e22ba 100644 --- a/test/unit/packwerk/rails_load_paths_test.rb +++ b/test/unit/packwerk/rails_load_paths_test.rb @@ -17,21 +17,6 @@ class RailsLoadPathsTest < Minitest::Test assert_equal({ relative_path => Object }, relative_path_strings) end - test ".for excludes paths outside of the application root" do - RailsLoadPaths.expects(:require_application).with("/application/", "test").once.returns(true) - rails_root = Pathname.new("/application/") - Rails.expects(:root).twice.returns(rails_root) - Bundler.expects(:bundle_path).once.returns(Pathname.new("/application/vendor/")) - - valid_paths = ["/application/app/models"] - paths = valid_paths + ["/users/tobi/.gems/something/app/models", "/application/../something/"] - paths = paths.each_with_object({}) { |p, h| h[p.to_s] = Object } - RailsLoadPaths.expects(:extract_application_autoload_paths).once.returns(paths) - filtered_path_strings = RailsLoadPaths.for(rails_root.to_s, environment: "test") - - assert_equal({ "app/models" => Object }, filtered_path_strings.transform_keys(&:to_s)) - end - test ".for excludes paths from vendored gems" do RailsLoadPaths.expects(:require_application).with("/application/", "test").once.returns(true) rails_root = Pathname.new("/application/")