From 70e3a174b29f4a0b94ed3c7f857ef49f2f626bde Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Sat, 13 Aug 2022 20:01:53 -0400 Subject: [PATCH 01/19] bump parse packwerk --- Gemfile.lock | 2 +- .../{parse_packwerk@0.11.0.rbi => parse_packwerk@0.12.0.rbi} | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) rename sorbet/rbi/gems/{parse_packwerk@0.11.0.rbi => parse_packwerk@0.12.0.rbi} (98%) diff --git a/Gemfile.lock b/Gemfile.lock index 284275a..52be8ee 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -41,7 +41,7 @@ GEM rubocop-sorbet sorbet-runtime parallel (1.22.1) - parse_packwerk (0.11.0) + parse_packwerk (0.12.0) sorbet-runtime parser (3.1.2.0) ast (~> 2.4.1) diff --git a/sorbet/rbi/gems/parse_packwerk@0.11.0.rbi b/sorbet/rbi/gems/parse_packwerk@0.12.0.rbi similarity index 98% rename from sorbet/rbi/gems/parse_packwerk@0.11.0.rbi rename to sorbet/rbi/gems/parse_packwerk@0.12.0.rbi index 923a31a..af8d82f 100644 --- a/sorbet/rbi/gems/parse_packwerk@0.11.0.rbi +++ b/sorbet/rbi/gems/parse_packwerk@0.12.0.rbi @@ -9,6 +9,9 @@ module ParsePackwerk sig { returns(T::Array[::ParsePackwerk::Package]) } def all; end + sig { void } + def bust_cache!; end + sig { params(name: ::String).returns(T.nilable(::ParsePackwerk::Package)) } def find(name); end From cb4dc5a4af6ce193c3f44ac91c4aa146abd18f95 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Sat, 13 Aug 2022 20:02:05 -0400 Subject: [PATCH 02/19] bump package protections --- Gemfile.lock | 2 +- ....0.0.rbi => package_protections@1.4.0.rbi} | 225 +++++++++--------- 2 files changed, 119 insertions(+), 108 deletions(-) rename sorbet/rbi/gems/{package_protections@1.0.0.rbi => package_protections@1.4.0.rbi} (89%) diff --git a/Gemfile.lock b/Gemfile.lock index 52be8ee..1340780 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -34,7 +34,7 @@ GEM json (2.6.2) method_source (1.0.0) minitest (5.16.2) - package_protections (1.3.0) + package_protections (1.4.0) activesupport parse_packwerk rubocop diff --git a/sorbet/rbi/gems/package_protections@1.0.0.rbi b/sorbet/rbi/gems/package_protections@1.4.0.rbi similarity index 89% rename from sorbet/rbi/gems/package_protections@1.0.0.rbi rename to sorbet/rbi/gems/package_protections@1.4.0.rbi index a38a5c5..3d5665a 100644 --- a/sorbet/rbi/gems/package_protections@1.0.0.rbi +++ b/sorbet/rbi/gems/package_protections@1.4.0.rbi @@ -23,14 +23,6 @@ module PackageProtections end def get_offenses(packages:, new_violations:); end - sig do - params( - package_names: T::Array[::String], - all_packages: T::Array[::ParsePackwerk::Package] - ).returns(T::Array[::ParsePackwerk::Package]) - end - def packages_for_names(package_names, all_packages); end - sig { params(identifier: ::String).returns(T::Hash[T.untyped, T.untyped]) } def private_cop_config(identifier); end @@ -119,14 +111,6 @@ module PackageProtections::Private sig { params(name: ::String).returns(::PackageProtections::ProtectedPackage) } def get_package_with_name(name); end - sig do - params( - package_names: T::Array[::String], - all_packages: T::Array[::ParsePackwerk::Package] - ).returns(T::Array[::ParsePackwerk::Package]) - end - def packages_for_names(package_names, all_packages); end - sig { params(identifier: ::String).returns(T::Hash[T.untyped, T.untyped]) } def private_cop_config(identifier); end @@ -279,51 +263,6 @@ class PackageProtections::Private::MetadataModifiers end end -class PackageProtections::Private::MultipleNamespacesProtection - include ::PackageProtections::ProtectionInterface - include ::PackageProtections::RubocopProtectionInterface - - sig do - override - .params( - packages: T::Array[::PackageProtections::ProtectedPackage] - ).returns(T::Array[::PackageProtections::RubocopProtectionInterface::CopConfig]) - end - def cop_configs(packages); end - - sig { params(package: ::PackageProtections::ProtectedPackage).returns(T::Hash[T.untyped, T.untyped]) } - def custom_cop_config(package); end - - sig do - override - .params( - protected_packages: T::Array[::PackageProtections::ProtectedPackage] - ).returns(T::Array[::PackageProtections::Offense]) - end - def get_offenses_for_existing_violations(protected_packages); end - - sig { override.returns(::String) } - def humanized_protection_description; end - - sig { override.returns(::String) } - def humanized_protection_name; end - - sig { override.returns(::String) } - def identifier; end - - sig do - override - .params( - behavior: ::PackageProtections::ViolationBehavior, - package: ::ParsePackwerk::Package - ).returns(T.nilable(::String)) - end - def unmet_preconditions_for_behavior(behavior, package); end -end - -PackageProtections::Private::MultipleNamespacesProtection::COP_NAME = T.let(T.unsafe(nil), String) -PackageProtections::Private::MultipleNamespacesProtection::IDENTIFIER = T.let(T.unsafe(nil), String) - class PackageProtections::Private::OutgoingDependencyProtection include ::PackageProtections::ProtectionInterface @@ -382,48 +321,6 @@ class PackageProtections::Private::Output end end -class PackageProtections::Private::TypedApiProtection - include ::PackageProtections::ProtectionInterface - include ::PackageProtections::RubocopProtectionInterface - - sig do - override - .params( - packages: T::Array[::PackageProtections::ProtectedPackage] - ).returns(T::Array[::PackageProtections::RubocopProtectionInterface::CopConfig]) - end - def cop_configs(packages); end - - sig do - override - .params( - protected_packages: T::Array[::PackageProtections::ProtectedPackage] - ).returns(T::Array[::PackageProtections::Offense]) - end - def get_offenses_for_existing_violations(protected_packages); end - - sig { override.returns(::String) } - def humanized_protection_description; end - - sig { override.returns(::String) } - def humanized_protection_name; end - - sig { override.returns(::String) } - def identifier; end - - sig do - override - .params( - behavior: ::PackageProtections::ViolationBehavior, - package: ::ParsePackwerk::Package - ).returns(T.nilable(::String)) - end - def unmet_preconditions_for_behavior(behavior, package); end -end - -PackageProtections::Private::TypedApiProtection::COP_NAME = T.let(T.unsafe(nil), String) -PackageProtections::Private::TypedApiProtection::IDENTIFIER = T.let(T.unsafe(nil), String) - class PackageProtections::Private::VisibilityProtection include ::PackageProtections::ProtectionInterface @@ -583,16 +480,26 @@ module PackageProtections::RubocopProtectionInterface abstract! sig do - abstract - .params( - packages: T::Array[::PackageProtections::ProtectedPackage] - ).returns(T::Array[::PackageProtections::RubocopProtectionInterface::CopConfig]) + params( + packages: T::Array[::PackageProtections::ProtectedPackage] + ).returns(T::Array[::PackageProtections::RubocopProtectionInterface::CopConfig]) end def cop_configs(packages); end + sig { abstract.returns(::String) } + def cop_name; end + sig { params(package: ::PackageProtections::ProtectedPackage).returns(T::Hash[T.untyped, T.untyped]) } def custom_cop_config(package); end + sig do + override + .params( + protected_packages: T::Array[::PackageProtections::ProtectedPackage] + ).returns(T::Array[::PackageProtections::Offense]) + end + def get_offenses_for_existing_violations(protected_packages); end + sig do override .params( @@ -601,6 +508,21 @@ module PackageProtections::RubocopProtectionInterface end def get_offenses_for_new_violations(new_violations); end + sig { abstract.returns(T::Array[::String]) } + def included_globs_for_pack; end + + sig { abstract.params(file: ::String).returns(::String) } + def message_for_fail_on_any(file); end + + sig do + override + .params( + behavior: ::PackageProtections::ViolationBehavior, + package: ::ParsePackwerk::Package + ).returns(T.nilable(::String)) + end + def unmet_preconditions_for_behavior(behavior, package); end + private sig { params(rule: ::String).returns(T::Set[::String]) } @@ -653,3 +575,92 @@ class PackageProtections::ViolationBehavior < ::T::Enum def from_raw_value(value); end end end + +module RuboCop; end +module RuboCop::Cop; end +RuboCop::Cop::IgnoredMethods = RuboCop::Cop::AllowedMethods +RuboCop::Cop::IgnoredPattern = RuboCop::Cop::AllowedPattern +module RuboCop::Cop::PackageProtections; end + +class RuboCop::Cop::PackageProtections::NamespacedUnderPackageName < ::RuboCop::Cop::Base + include ::RuboCop::Cop::RangeHelp + include ::PackageProtections::ProtectionInterface + include ::PackageProtections::RubocopProtectionInterface + + sig { override.returns(::String) } + def cop_name; end + + sig { params(package: ::PackageProtections::ProtectedPackage).returns(T::Hash[T.untyped, T.untyped]) } + def custom_cop_config(package); end + + sig { override.returns(::String) } + def humanized_protection_description; end + + sig { override.returns(::String) } + def humanized_protection_name; end + + sig { override.returns(::String) } + def identifier; end + + sig { override.returns(T::Array[::String]) } + def included_globs_for_pack; end + + sig { override.params(file: ::String).returns(::String) } + def message_for_fail_on_any(file); end + + def on_new_investigation; end + + sig do + override + .params( + behavior: ::PackageProtections::ViolationBehavior, + package: ::ParsePackwerk::Package + ).returns(T.nilable(::String)) + end + def unmet_preconditions_for_behavior(behavior, package); end + + private + + def get_actual_namespace(remaining_file_path, relative_filepath, package_name); end + def get_allowed_namespaces(package_name); end + def root_pathname; end +end + +RuboCop::Cop::PackageProtections::NamespacedUnderPackageName::IDENTIFIER = T.let(T.unsafe(nil), String) + +class RuboCop::Cop::PackageProtections::TypedPublicApi < ::RuboCop::Cop::Sorbet::StrictSigil + include ::PackageProtections::ProtectionInterface + include ::PackageProtections::RubocopProtectionInterface + + sig { override.returns(::String) } + def cop_name; end + + sig { override.returns(::String) } + def humanized_protection_description; end + + sig { override.returns(::String) } + def humanized_protection_name; end + + sig { override.returns(::String) } + def identifier; end + + sig { override.returns(T::Array[::String]) } + def included_globs_for_pack; end + + sig { override.params(file: ::String).returns(::String) } + def message_for_fail_on_any(file); end + + sig do + override + .params( + behavior: ::PackageProtections::ViolationBehavior, + package: ::ParsePackwerk::Package + ).returns(T.nilable(::String)) + end + def unmet_preconditions_for_behavior(behavior, package); end +end + +RuboCop::Cop::PackageProtections::TypedPublicApi::IDENTIFIER = T.let(T.unsafe(nil), String) +RuboCop::NodePattern = RuboCop::AST::NodePattern +RuboCop::ProcessedSource = RuboCop::AST::ProcessedSource +RuboCop::Token = RuboCop::AST::Token From f89f5a53ea40c5bf075cb3286dd996c7ff3aed83 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 07:36:55 -0400 Subject: [PATCH 03/19] change code/tests for new version of parse packwerk --- lib/use_packwerk/private.rb | 1 + spec/spec_helper.rb | 5 +++++ spec/use_packwerk_spec.rb | 10 +++++++++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/use_packwerk/private.rb b/lib/use_packwerk/private.rb index d7bfa8f..117d516 100644 --- a/lib/use_packwerk/private.rb +++ b/lib/use_packwerk/private.rb @@ -427,6 +427,7 @@ def self.create_pack_if_not_exists!(pack_name:, enforce_privacy:, enforce_depend sig { params(original_package: ParsePackwerk::Package).returns(ParsePackwerk::Package) } def self.rewrite_package_with_original_packwerk_values(original_package) + ParsePackwerk.bust_cache! package_with_protection_defaults = T.must(ParsePackwerk.all.find { |package| package.name == original_package.name }) # PackageProtections also sets `enforce_privacy` and `enforce_dependency` to be true, so we set these back down to their original values package = ParsePackwerk::Package.new( diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6a549f6..7118ac6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,6 +18,11 @@ config.include_context 'app fixtures' + config.around do |example| + ParsePackwerk.bust_cache! + example.run + end + config.around do |example| prefix = [File.basename($0), Process.pid].join('-') # rubocop:disable Style/SpecialGlobalVars tmpdir = Dir.mktmpdir(prefix) diff --git a/spec/use_packwerk_spec.rb b/spec/use_packwerk_spec.rb index bd76450..18ec487 100644 --- a/spec/use_packwerk_spec.rb +++ b/spec/use_packwerk_spec.rb @@ -1,6 +1,10 @@ # typed: false RSpec.describe UsePackwerk do + + # Note: Once we migrate `ParsePackwerk` to use an initialized `PackageSet`, the cache behavior will become more clear, hopefully. + # The client can get a new package set each time they are sensitive to a stale cache. def get_packages + ParsePackwerk.bust_cache! ParsePackwerk.all end @@ -27,6 +31,7 @@ def expect_files_to_not_exist(files) def bust_cache_and_configure_code_ownership! CodeOwnership.bust_caches! + ParsePackwerk.bust_cache! end before do @@ -904,7 +909,7 @@ def bust_cache_and_configure_code_ownership! # @team Artists CONTENTS - write_file('packs/package.yml', <<~CONTENTS) + write_file('package.yml', <<~CONTENTS) enforce_dependencies: true enforce_privacy: true metadata: @@ -970,6 +975,7 @@ def bust_cache_and_configure_code_ownership! end create_pack + ParsePackwerk.bust_cache! expect(Pathname.new('app/services/owned_by_chefs/sandwich.rb').read).to eq <<~RUBY # @team Chefs @@ -1713,6 +1719,8 @@ def bust_cache_and_configure_code_ownership! enforce_dependencies: false enforce_privacy: false YML + + ParsePackwerk.bust_cache! end it 'adds the dependency' do From c00ba4107dabea4cac80a69494eabf84b40a3195 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 07:39:42 -0400 Subject: [PATCH 04/19] add nested pack tests for create_pack --- spec/use_packwerk_spec.rb | 69 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/spec/use_packwerk_spec.rb b/spec/use_packwerk_spec.rb index 18ec487..f572d07 100644 --- a/spec/use_packwerk_spec.rb +++ b/spec/use_packwerk_spec.rb @@ -162,6 +162,75 @@ def bust_cache_and_configure_code_ownership! expect(only_nonroot_package.name).to eq('gems/my_sick_new_pack') end end + + context 'pack is nested' do + let(:pack_name) { 'packs/fruits/apples' } + + it 'creates a package.yml correctly' do + create_pack + + expect(only_nonroot_package.name).to eq('packs/fruits/apples') + expect(only_nonroot_package.enforce_privacy).to eq(true) + expect(only_nonroot_package.enforce_dependencies).to eq(true) + expect(only_nonroot_package.dependencies).to eq([]) + expect(only_nonroot_package.metadata).to eq({ 'owner' => 'MyTeam', 'protections' => {"prevent_other_packages_from_using_this_packages_internals"=>"fail_on_new", "prevent_this_package_from_creating_other_namespaces"=>"fail_on_new", "prevent_this_package_from_exposing_an_untyped_api"=>"fail_on_new", "prevent_this_package_from_violating_its_stated_dependencies"=>"fail_on_new"} }) + + expected = <<~EXPECTED + enforce_dependencies: true + enforce_privacy: true + metadata: + owner: MyTeam # specify your team here, or delete this key if this package is not owned by one team + protections: + prevent_this_package_from_violating_its_stated_dependencies: fail_on_new + prevent_other_packages_from_using_this_packages_internals: fail_on_new + prevent_this_package_from_exposing_an_untyped_api: fail_on_new + prevent_this_package_from_creating_other_namespaces: fail_on_new + EXPECTED + + expect(only_nonroot_package.yml.read).to eq expected + end + + context 'pack already exists and has content' do + before do + write_file('packs/fruit/apples/package.yml', <<~CONTENTS) + enforce_privacy: true + enforce_dependencies: true + dependencies: + - packs/some_other_pack + metadata: + protections: + prevent_this_package_from_exposing_an_untyped_api: fail_never + prevent_this_package_from_violating_its_stated_dependencies: fail_on_new + prevent_other_packages_from_using_this_packages_internals: fail_on_new + prevent_this_package_from_creating_other_namespaces: fail_on_new + CONTENTS + end + + it 'is idempotent' do + expect(packages.count).to eq 1 + existing_package = packages.first + expect(existing_package.dependencies).to eq(['packs/some_other_pack']) + expect(existing_package.metadata).to eq({ + 'protections' => { + 'prevent_this_package_from_exposing_an_untyped_api' => 'fail_never', + 'prevent_this_package_from_violating_its_stated_dependencies' => 'fail_on_new', + 'prevent_other_packages_from_using_this_packages_internals' => 'fail_on_new', + 'prevent_this_package_from_creating_other_namespaces' => 'fail_on_new', + } + }) + UsePackwerk.create_pack!(pack_name: 'packs/fruit/apples/') + new_packages = get_packages + expect(new_packages.count).to eq 1 + new_package = new_packages.first + + expect(new_package.name).to eq(existing_package.name) + expect(new_package.enforce_privacy).to eq(existing_package.enforce_privacy) + expect(new_package.enforce_dependencies).to eq(existing_package.enforce_dependencies) + expect(new_package.dependencies).to eq(existing_package.dependencies) + expect(new_package.metadata).to eq(existing_package.metadata) + end + end + end end describe '.move_to_pack!' do From a13e4b156c76c3346e3b3f19c2ae06c37da4410d Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 07:40:50 -0400 Subject: [PATCH 05/19] nest existing tests --- spec/use_packwerk_spec.rb | 1407 +++++++++++++++++++------------------ 1 file changed, 705 insertions(+), 702 deletions(-) diff --git a/spec/use_packwerk_spec.rb b/spec/use_packwerk_spec.rb index f572d07..3bb023c 100644 --- a/spec/use_packwerk_spec.rb +++ b/spec/use_packwerk_spec.rb @@ -234,7 +234,6 @@ def bust_cache_and_configure_code_ownership! end describe '.move_to_pack!' do - let(:pack_name) { 'packs/animals' } let(:create_pack) do UsePackwerk.create_pack!( @@ -264,101 +263,54 @@ def bust_cache_and_configure_code_ownership! ) end - context 'pack not yet created' do - it 'errors' do - expect { move_to_pack }.to raise_error("Can not find package with name packs/animals. Make sure the argument is of the form `packs/my_pack/`") - end - end - - it 'can move files from a monolith into a package' do - complex_app - - expected_files_before = [ - # Files in monolith - 'app/services/horse_like/zebra.rb', - 'app/services/horse_like/donkey.rb', - 'app/services/horse_like/horse.rb', - 'app/services/horse_like/zebra.rb', - 'app/services/fish_like/small_ones/goldfish.rb', - 'app/services/fish_like/small_ones/seahorse.rb', - 'app/services/fish_like/big_ones/whale.rb', - # Specs in monolith - 'spec/services/dog_like/golden_retriever_spec.rb', - 'spec/services/fish_like/big_ones/whale_spec.rb', - 'spec/services/horse_like/donkey_spec.rb', - ] - - expect_files_to_exist expected_files_before - - create_pack - move_to_pack - - expect_files_to_not_exist expected_files_before - - expected_files_after = [ - 'packs/animals/app/services/horse_like/zebra.rb', - 'packs/animals/app/services/horse_like/donkey.rb', - 'packs/animals/app/services/horse_like/horse.rb', - 'packs/animals/app/services/horse_like/zebra.rb', - 'packs/animals/app/services/fish_like/small_ones/goldfish.rb', - 'packs/animals/app/services/fish_like/small_ones/seahorse.rb', - 'packs/animals/app/services/fish_like/big_ones/whale.rb', - 'packs/animals/spec/services/dog_like/golden_retriever_spec.rb', - 'packs/animals/spec/services/fish_like/big_ones/whale_spec.rb', - 'packs/animals/spec/services/horse_like/donkey_spec.rb', - ] - - expect_files_to_exist expected_files_after - end + context 'pack is not nested' do + let(:pack_name) { 'packs/animals' } - it 'can move files from one pack to another pack' do - complex_app + context 'pack not yet created' do + it 'errors' do + expect { move_to_pack }.to raise_error("Can not find package with name packs/animals. Make sure the argument is of the form `packs/my_pack/`") + end + end - expected_files_before = [ - # Files in packs - 'packs/organisms/app/services/bird_like/eagle.rb', - 'packs/organisms/app/services/bird_like/swan.rb', - 'packs/organisms/app/services/bug_like/fly.rb', - # Specs in packs - 'packs/organisms/spec/services/bird_like/eagle_spec.rb', - 'packs/organisms/spec/services/bug_like/fly_spec.rb', - ] + it 'can move files from a monolith into a package' do + complex_app - expect_files_to_exist expected_files_before + expected_files_before = [ + # Files in monolith + 'app/services/horse_like/zebra.rb', + 'app/services/horse_like/donkey.rb', + 'app/services/horse_like/horse.rb', + 'app/services/horse_like/zebra.rb', + 'app/services/fish_like/small_ones/goldfish.rb', + 'app/services/fish_like/small_ones/seahorse.rb', + 'app/services/fish_like/big_ones/whale.rb', + # Specs in monolith + 'spec/services/dog_like/golden_retriever_spec.rb', + 'spec/services/fish_like/big_ones/whale_spec.rb', + 'spec/services/horse_like/donkey_spec.rb', + ] - create_pack - move_to_pack + expect_files_to_exist expected_files_before - expect_files_to_not_exist expected_files_before + create_pack + move_to_pack - expected_files_after = [ - 'packs/animals/app/services/bird_like/eagle.rb', - 'packs/animals/app/services/bird_like/swan.rb', - 'packs/animals/app/services/bug_like/fly.rb', - 'packs/animals/spec/services/bird_like/eagle_spec.rb', - 'packs/animals/spec/services/bug_like/fly_spec.rb', - ] + expect_files_to_not_exist expected_files_before - expect_files_to_exist expected_files_after - end + expected_files_after = [ + 'packs/animals/app/services/horse_like/zebra.rb', + 'packs/animals/app/services/horse_like/donkey.rb', + 'packs/animals/app/services/horse_like/horse.rb', + 'packs/animals/app/services/horse_like/zebra.rb', + 'packs/animals/app/services/fish_like/small_ones/goldfish.rb', + 'packs/animals/app/services/fish_like/small_ones/seahorse.rb', + 'packs/animals/app/services/fish_like/big_ones/whale.rb', + 'packs/animals/spec/services/dog_like/golden_retriever_spec.rb', + 'packs/animals/spec/services/fish_like/big_ones/whale_spec.rb', + 'packs/animals/spec/services/horse_like/donkey_spec.rb', + ] - context 'directory moves have trailing slashes' do - let(:move_to_pack) do - UsePackwerk.move_to_pack!( - pack_name: pack_name, - paths_relative_to_root: [ - # Files in monolith - 'app/services/horse_like/', - 'app/services/fish_like/small_ones/', - 'app/services/fish_like/big_ones/', - 'app/services/dog_like/golden_retriever.rb', - # Files in packs - 'packs/organisms/app/services/bird_like/eagle.rb', - 'packs/organisms/app/services/bird_like/swan.rb', - 'packs/organisms/app/services/bug_like/fly.rb', - ], - per_file_processors: [UsePackwerk::RubocopPostProcessor.new, UsePackwerk::CodeOwnershipPostProcessor.new], - ) + expect_files_to_exist expected_files_after end it 'can move files from one pack to another pack' do @@ -391,755 +343,806 @@ def bust_cache_and_configure_code_ownership! expect_files_to_exist expected_files_after end - end - describe 'RubocopPostProcessor' do - context 'moving file listed in top-level .rubocop_todo.yml' do - it 'modifies an application-specific file, .rubocop_todo.yml, correctly' do - write_file('.rubocop_todo.yml', <<~CONTENTS) - --- - Layout/BeginEndAlignment: - Exclude: - - packs/foo/app/services/foo.rb - CONTENTS - - before_rubocop_todo = YAML.load_file(Pathname.new('.rubocop_todo.yml')) - - expect(before_rubocop_todo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/foo/app/services/foo.rb"]}}) - - write_file('packs/foo/app/services/foo.rb') - UsePackwerk.create_pack!(pack_name: 'packs/bar') - UsePackwerk.create_pack!(pack_name: 'packs/foo') + context 'directory moves have trailing slashes' do + let(:move_to_pack) do UsePackwerk.move_to_pack!( - pack_name: 'packs/bar', - paths_relative_to_root: ['packs/foo/app/services/foo.rb'], - per_file_processors: [UsePackwerk::RubocopPostProcessor.new], + pack_name: pack_name, + paths_relative_to_root: [ + # Files in monolith + 'app/services/horse_like/', + 'app/services/fish_like/small_ones/', + 'app/services/fish_like/big_ones/', + 'app/services/dog_like/golden_retriever.rb', + # Files in packs + 'packs/organisms/app/services/bird_like/eagle.rb', + 'packs/organisms/app/services/bird_like/swan.rb', + 'packs/organisms/app/services/bug_like/fly.rb', + ], + per_file_processors: [UsePackwerk::RubocopPostProcessor.new, UsePackwerk::CodeOwnershipPostProcessor.new], ) - - after_rubocop_todo = YAML.load_file(Pathname.new('.rubocop_todo.yml')) - expect(after_rubocop_todo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/bar/app/services/foo.rb"]}}) end - end - - context 'origin pack has a pack-level .rubocop_todo.yml, destination pack does not' do - it 'modifies packs/*/.rubocop_todo.yml, correctly' do - write_file('packs/foo/.rubocop_todo.yml', <<~CONTENTS) - --- - Layout/BeginEndAlignment: - Exclude: - - packs/foo/app/services/foo.rb - CONTENTS - before_rubocop_todo_foo = YAML.load_file(Pathname.new('packs/foo/.rubocop_todo.yml')) - expect(before_rubocop_todo_foo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/foo/app/services/foo.rb"]}}) - expect(Pathname.new('packs/bar/.rubocop_todo.yml')).to_not exist + it 'can move files from one pack to another pack' do + complex_app - write_file('packs/foo/app/services/foo.rb') - UsePackwerk.create_pack!(pack_name: 'packs/bar') - UsePackwerk.create_pack!(pack_name: 'packs/foo') - UsePackwerk.move_to_pack!( - pack_name: 'packs/bar', - paths_relative_to_root: ['packs/foo/app/services/foo.rb'], - per_file_processors: [UsePackwerk::RubocopPostProcessor.new], - ) + expected_files_before = [ + # Files in packs + 'packs/organisms/app/services/bird_like/eagle.rb', + 'packs/organisms/app/services/bird_like/swan.rb', + 'packs/organisms/app/services/bug_like/fly.rb', + # Specs in packs + 'packs/organisms/spec/services/bird_like/eagle_spec.rb', + 'packs/organisms/spec/services/bug_like/fly_spec.rb', + ] - after_rubocop_todo_foo = YAML.load_file(Pathname.new('packs/foo/.rubocop_todo.yml')) - expect(after_rubocop_todo_foo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>[]}}) - after_rubocop_todo_bar = YAML.load_file(Pathname.new('packs/bar/.rubocop_todo.yml')) - expect(after_rubocop_todo_bar).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/bar/app/services/foo.rb"]}}) - end - end + expect_files_to_exist expected_files_before - context 'origin and destination pack both have .rubocop_todo.yml' do - it 'modifies packs/*/.rubocop_todo.yml, correctly' do - write_file('packs/foo/.rubocop_todo.yml', <<~CONTENTS) - --- - Layout/BeginEndAlignment: - Exclude: - - packs/foo/app/services/foo.rb - CONTENTS - - write_file('packs/bar/.rubocop_todo.yml', <<~CONTENTS) - --- - Layout/BeginEndAlignment: - Exclude: - - packs/bar/app/services/bar.rb - CONTENTS + create_pack + move_to_pack - before_rubocop_todo_foo = YAML.load_file(Pathname.new('packs/foo/.rubocop_todo.yml')) - expect(before_rubocop_todo_foo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/foo/app/services/foo.rb"]}}) - before_rubocop_todo_bar = YAML.load_file(Pathname.new('packs/bar/.rubocop_todo.yml')) - expect(before_rubocop_todo_bar).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/bar/app/services/bar.rb"]}}) + expect_files_to_not_exist expected_files_before - write_file('packs/foo/app/services/foo.rb') - UsePackwerk.create_pack!(pack_name: 'packs/bar') - UsePackwerk.create_pack!(pack_name: 'packs/foo') - UsePackwerk.move_to_pack!( - pack_name: 'packs/bar', - paths_relative_to_root: ['packs/foo/app/services/foo.rb'], - per_file_processors: [UsePackwerk::RubocopPostProcessor.new], - ) + expected_files_after = [ + 'packs/animals/app/services/bird_like/eagle.rb', + 'packs/animals/app/services/bird_like/swan.rb', + 'packs/animals/app/services/bug_like/fly.rb', + 'packs/animals/spec/services/bird_like/eagle_spec.rb', + 'packs/animals/spec/services/bug_like/fly_spec.rb', + ] - after_rubocop_todo_foo = YAML.load_file(Pathname.new('packs/foo/.rubocop_todo.yml')) - expect(after_rubocop_todo_foo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>[]}}) - after_rubocop_todo_bar = YAML.load_file(Pathname.new('packs/bar/.rubocop_todo.yml')) - expect(after_rubocop_todo_bar).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/bar/app/services/bar.rb", "packs/bar/app/services/foo.rb"]}}) + expect_files_to_exist expected_files_after end end - context 'destination pack does not have same key in .rubocop_todo.yml' do - it 'modifies packs/*/.rubocop_todo.yml, correctly' do - write_file('packs/foo/.rubocop_todo.yml', <<~CONTENTS) - --- - Layout/BeginEndAlignment: - Exclude: - - packs/foo/app/services/foo.rb - CONTENTS - - write_file('packs/bar/.rubocop_todo.yml', <<~CONTENTS) - --- - Layout/OtherCop: - Exclude: - - packs/bar/app/services/bar.rb - CONTENTS + describe 'RubocopPostProcessor' do + context 'moving file listed in top-level .rubocop_todo.yml' do + it 'modifies an application-specific file, .rubocop_todo.yml, correctly' do + write_file('.rubocop_todo.yml', <<~CONTENTS) + --- + Layout/BeginEndAlignment: + Exclude: + - packs/foo/app/services/foo.rb + CONTENTS + + before_rubocop_todo = YAML.load_file(Pathname.new('.rubocop_todo.yml')) + + expect(before_rubocop_todo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/foo/app/services/foo.rb"]}}) + + write_file('packs/foo/app/services/foo.rb') + UsePackwerk.create_pack!(pack_name: 'packs/bar') + UsePackwerk.create_pack!(pack_name: 'packs/foo') + UsePackwerk.move_to_pack!( + pack_name: 'packs/bar', + paths_relative_to_root: ['packs/foo/app/services/foo.rb'], + per_file_processors: [UsePackwerk::RubocopPostProcessor.new], + ) + + after_rubocop_todo = YAML.load_file(Pathname.new('.rubocop_todo.yml')) + expect(after_rubocop_todo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/bar/app/services/foo.rb"]}}) + end + end - before_rubocop_todo_foo = YAML.load_file(Pathname.new('packs/foo/.rubocop_todo.yml')) - expect(before_rubocop_todo_foo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/foo/app/services/foo.rb"]}}) - before_rubocop_todo_bar = YAML.load_file(Pathname.new('packs/bar/.rubocop_todo.yml')) - expect(before_rubocop_todo_bar).to eq({"Layout/OtherCop" => {"Exclude"=>["packs/bar/app/services/bar.rb"]}}) + context 'origin pack has a pack-level .rubocop_todo.yml, destination pack does not' do + it 'modifies packs/*/.rubocop_todo.yml, correctly' do + write_file('packs/foo/.rubocop_todo.yml', <<~CONTENTS) + --- + Layout/BeginEndAlignment: + Exclude: + - packs/foo/app/services/foo.rb + CONTENTS + + before_rubocop_todo_foo = YAML.load_file(Pathname.new('packs/foo/.rubocop_todo.yml')) + expect(before_rubocop_todo_foo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/foo/app/services/foo.rb"]}}) + expect(Pathname.new('packs/bar/.rubocop_todo.yml')).to_not exist + + write_file('packs/foo/app/services/foo.rb') + UsePackwerk.create_pack!(pack_name: 'packs/bar') + UsePackwerk.create_pack!(pack_name: 'packs/foo') + UsePackwerk.move_to_pack!( + pack_name: 'packs/bar', + paths_relative_to_root: ['packs/foo/app/services/foo.rb'], + per_file_processors: [UsePackwerk::RubocopPostProcessor.new], + ) + + after_rubocop_todo_foo = YAML.load_file(Pathname.new('packs/foo/.rubocop_todo.yml')) + expect(after_rubocop_todo_foo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>[]}}) + after_rubocop_todo_bar = YAML.load_file(Pathname.new('packs/bar/.rubocop_todo.yml')) + expect(after_rubocop_todo_bar).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/bar/app/services/foo.rb"]}}) + end + end - write_file('packs/foo/app/services/foo.rb') - UsePackwerk.create_pack!(pack_name: 'packs/bar') - UsePackwerk.create_pack!(pack_name: 'packs/foo') - UsePackwerk.move_to_pack!( - pack_name: 'packs/bar', - paths_relative_to_root: ['packs/foo/app/services/foo.rb'], - per_file_processors: [UsePackwerk::RubocopPostProcessor.new], - ) + context 'origin and destination pack both have .rubocop_todo.yml' do + it 'modifies packs/*/.rubocop_todo.yml, correctly' do + write_file('packs/foo/.rubocop_todo.yml', <<~CONTENTS) + --- + Layout/BeginEndAlignment: + Exclude: + - packs/foo/app/services/foo.rb + CONTENTS + + write_file('packs/bar/.rubocop_todo.yml', <<~CONTENTS) + --- + Layout/BeginEndAlignment: + Exclude: + - packs/bar/app/services/bar.rb + CONTENTS + + before_rubocop_todo_foo = YAML.load_file(Pathname.new('packs/foo/.rubocop_todo.yml')) + expect(before_rubocop_todo_foo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/foo/app/services/foo.rb"]}}) + before_rubocop_todo_bar = YAML.load_file(Pathname.new('packs/bar/.rubocop_todo.yml')) + expect(before_rubocop_todo_bar).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/bar/app/services/bar.rb"]}}) + + write_file('packs/foo/app/services/foo.rb') + UsePackwerk.create_pack!(pack_name: 'packs/bar') + UsePackwerk.create_pack!(pack_name: 'packs/foo') + UsePackwerk.move_to_pack!( + pack_name: 'packs/bar', + paths_relative_to_root: ['packs/foo/app/services/foo.rb'], + per_file_processors: [UsePackwerk::RubocopPostProcessor.new], + ) + + after_rubocop_todo_foo = YAML.load_file(Pathname.new('packs/foo/.rubocop_todo.yml')) + expect(after_rubocop_todo_foo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>[]}}) + after_rubocop_todo_bar = YAML.load_file(Pathname.new('packs/bar/.rubocop_todo.yml')) + expect(after_rubocop_todo_bar).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/bar/app/services/bar.rb", "packs/bar/app/services/foo.rb"]}}) + end + end - after_rubocop_todo_foo = YAML.load_file(Pathname.new('packs/foo/.rubocop_todo.yml')) - expect(after_rubocop_todo_foo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>[]}}) - after_rubocop_todo_bar = YAML.load_file(Pathname.new('packs/bar/.rubocop_todo.yml')) - expect(after_rubocop_todo_bar).to eq({ - "Layout/BeginEndAlignment" => {"Exclude"=>["packs/bar/app/services/foo.rb"]}, - "Layout/OtherCop" => {"Exclude"=>["packs/bar/app/services/bar.rb"]}, - }) + context 'destination pack does not have same key in .rubocop_todo.yml' do + it 'modifies packs/*/.rubocop_todo.yml, correctly' do + write_file('packs/foo/.rubocop_todo.yml', <<~CONTENTS) + --- + Layout/BeginEndAlignment: + Exclude: + - packs/foo/app/services/foo.rb + CONTENTS + + write_file('packs/bar/.rubocop_todo.yml', <<~CONTENTS) + --- + Layout/OtherCop: + Exclude: + - packs/bar/app/services/bar.rb + CONTENTS + + before_rubocop_todo_foo = YAML.load_file(Pathname.new('packs/foo/.rubocop_todo.yml')) + expect(before_rubocop_todo_foo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>["packs/foo/app/services/foo.rb"]}}) + before_rubocop_todo_bar = YAML.load_file(Pathname.new('packs/bar/.rubocop_todo.yml')) + expect(before_rubocop_todo_bar).to eq({"Layout/OtherCop" => {"Exclude"=>["packs/bar/app/services/bar.rb"]}}) + + write_file('packs/foo/app/services/foo.rb') + UsePackwerk.create_pack!(pack_name: 'packs/bar') + UsePackwerk.create_pack!(pack_name: 'packs/foo') + UsePackwerk.move_to_pack!( + pack_name: 'packs/bar', + paths_relative_to_root: ['packs/foo/app/services/foo.rb'], + per_file_processors: [UsePackwerk::RubocopPostProcessor.new], + ) + + after_rubocop_todo_foo = YAML.load_file(Pathname.new('packs/foo/.rubocop_todo.yml')) + expect(after_rubocop_todo_foo).to eq({"Layout/BeginEndAlignment" => {"Exclude"=>[]}}) + after_rubocop_todo_bar = YAML.load_file(Pathname.new('packs/bar/.rubocop_todo.yml')) + expect(after_rubocop_todo_bar).to eq({ + "Layout/BeginEndAlignment" => {"Exclude"=>["packs/bar/app/services/foo.rb"]}, + "Layout/OtherCop" => {"Exclude"=>["packs/bar/app/services/bar.rb"]}, + }) + end end end - end - - it 'modifies an application-specific file, config/code_ownership.yml, correctly' do - complex_app - before_codeownership_yml = File.read(Pathname.new('config/code_ownership.yml')) - - expect(before_codeownership_yml).to include "- app/services/horse_like/donkey.rb" - expect(before_codeownership_yml).to include "- app/services/fish_like/small_ones/goldfish.rb" - expect(before_codeownership_yml).to include "- app/services/fish_like/big_ones/whale.rb" - expect(before_codeownership_yml).to include "- app/services/dog_like/golden_retriever.rb" - expect(before_codeownership_yml).to include "- packs/organisms/app/services/bird_like/eagle.rb" - expect(before_codeownership_yml).to include "- packs/organisms/app/services/bird_like/swan.rb" - expect(before_codeownership_yml).to include "- packs/organisms/app/services/bug_like/fly.rb" - - create_pack - move_to_pack - - after_codeownership_yml = File.read(Pathname.new('config/code_ownership.yml')) - - expect(after_codeownership_yml).to_not include "- app/services/horse_like/donkey.rb" - expect(after_codeownership_yml).to_not include "- app/services/fish_like/small_ones/goldfish.rb" - expect(after_codeownership_yml).to_not include "- app/services/fish_like/big_ones/whale.rb" - expect(after_codeownership_yml).to_not include "- app/services/dog_like/golden_retriever.rb" - expect(after_codeownership_yml).to_not include "- packs/organisms/app/services/bird_like/eagle.rb" - expect(after_codeownership_yml).to_not include "- packs/organisms/app/services/bird_like/swan.rb" - expect(after_codeownership_yml).to_not include "- packs/organisms/app/services/bug_like/fly.rb" - - expect(after_codeownership_yml).to include "- packs/animals/app/services/horse_like/donkey.rb" - expect(after_codeownership_yml).to include "- packs/animals/app/services/fish_like/small_ones/goldfish.rb" - expect(after_codeownership_yml).to include "- packs/animals/app/services/fish_like/big_ones/whale.rb" - expect(after_codeownership_yml).to include "- packs/animals/app/services/dog_like/golden_retriever.rb" - expect(after_codeownership_yml).to include "- packs/animals/app/services/bird_like/eagle.rb" - expect(after_codeownership_yml).to include "- packs/animals/app/services/bird_like/swan.rb" - expect(after_codeownership_yml).to include "- packs/animals/app/services/bug_like/fly.rb" - end + it 'modifies an application-specific file, config/code_ownership.yml, correctly' do + complex_app - context 'packs have folders of the same name' do - before { app_with_files_and_directories_with_same_names } + before_codeownership_yml = File.read(Pathname.new('config/code_ownership.yml')) - it 'merges the set of files in common folders' do - expected_files_before = [ - # Files in food pack - 'packs/food/app/public/tomato.rb', - 'packs/food/app/services/salad.rb', - 'packs/food/app/services/salads/dressing.rb', - 'packs/food/spec/public/tomato_spec.rb', - 'packs/food/spec/services/salad_spec.rb', - 'packs/food/spec/services/salads/dressing_spec.rb', - # Files in organisms pack - 'packs/organisms/app/public/tomato.rb', - 'packs/organisms/app/services/eagle.rb', - 'packs/organisms/app/services/other_bird.rb', - 'packs/organisms/app/services/vulture.rb', - # Files in monolith - 'app/services/salads/types/cobb.rb', - 'spec/services/salads/types/cobb_spec.rb', - ] + expect(before_codeownership_yml).to include "- app/services/horse_like/donkey.rb" + expect(before_codeownership_yml).to include "- app/services/fish_like/small_ones/goldfish.rb" + expect(before_codeownership_yml).to include "- app/services/fish_like/big_ones/whale.rb" + expect(before_codeownership_yml).to include "- app/services/dog_like/golden_retriever.rb" + expect(before_codeownership_yml).to include "- packs/organisms/app/services/bird_like/eagle.rb" + expect(before_codeownership_yml).to include "- packs/organisms/app/services/bird_like/swan.rb" + expect(before_codeownership_yml).to include "- packs/organisms/app/services/bug_like/fly.rb" - expect_files_to_exist expected_files_before + create_pack + move_to_pack - UsePackwerk.move_to_pack!( - pack_name: 'packs/food', - paths_relative_to_root: [ - 'packs/organisms/app/services', - 'app/services' - ], - ) + after_codeownership_yml = File.read(Pathname.new('config/code_ownership.yml')) + + expect(after_codeownership_yml).to_not include "- app/services/horse_like/donkey.rb" + expect(after_codeownership_yml).to_not include "- app/services/fish_like/small_ones/goldfish.rb" + expect(after_codeownership_yml).to_not include "- app/services/fish_like/big_ones/whale.rb" + expect(after_codeownership_yml).to_not include "- app/services/dog_like/golden_retriever.rb" + expect(after_codeownership_yml).to_not include "- packs/organisms/app/services/bird_like/eagle.rb" + expect(after_codeownership_yml).to_not include "- packs/organisms/app/services/bird_like/swan.rb" + expect(after_codeownership_yml).to_not include "- packs/organisms/app/services/bug_like/fly.rb" + + expect(after_codeownership_yml).to include "- packs/animals/app/services/horse_like/donkey.rb" + expect(after_codeownership_yml).to include "- packs/animals/app/services/fish_like/small_ones/goldfish.rb" + expect(after_codeownership_yml).to include "- packs/animals/app/services/fish_like/big_ones/whale.rb" + expect(after_codeownership_yml).to include "- packs/animals/app/services/dog_like/golden_retriever.rb" + expect(after_codeownership_yml).to include "- packs/animals/app/services/bird_like/eagle.rb" + expect(after_codeownership_yml).to include "- packs/animals/app/services/bird_like/swan.rb" + expect(after_codeownership_yml).to include "- packs/animals/app/services/bug_like/fly.rb" + end + + context 'packs have folders of the same name' do + before { app_with_files_and_directories_with_same_names } + + it 'merges the set of files in common folders' do + expected_files_before = [ + # Files in food pack + 'packs/food/app/public/tomato.rb', + 'packs/food/app/services/salad.rb', + 'packs/food/app/services/salads/dressing.rb', + 'packs/food/spec/public/tomato_spec.rb', + 'packs/food/spec/services/salad_spec.rb', + 'packs/food/spec/services/salads/dressing_spec.rb', + # Files in organisms pack + 'packs/organisms/app/public/tomato.rb', + 'packs/organisms/app/services/eagle.rb', + 'packs/organisms/app/services/other_bird.rb', + 'packs/organisms/app/services/vulture.rb', + # Files in monolith + 'app/services/salads/types/cobb.rb', + 'spec/services/salads/types/cobb_spec.rb', + ] - expect_files_to_not_exist([ - 'packs/organisms/app/services/eagle.rb', - 'packs/organisms/app/services/other_bird.rb', - 'packs/organisms/app/services/vulture.rb', - 'app/services/salads/types/cobb.rb', - 'spec/services/salads/types/cobb_spec.rb', - ]) + expect_files_to_exist expected_files_before - expected_files_after = [ - 'packs/food/app/public/tomato.rb', - 'packs/food/app/services/salad.rb', - 'packs/food/app/services/salads/dressing.rb', - 'packs/food/spec/public/tomato_spec.rb', - 'packs/food/spec/services/salad_spec.rb', - 'packs/food/spec/services/salads/dressing_spec.rb', - 'packs/food/app/services/eagle.rb', - 'packs/food/app/services/other_bird.rb', - 'packs/food/app/services/vulture.rb', - 'packs/food/app/services/salads/types/cobb.rb', - 'packs/food/spec/services/salads/types/cobb_spec.rb', - ] + UsePackwerk.move_to_pack!( + pack_name: 'packs/food', + paths_relative_to_root: [ + 'packs/organisms/app/services', + 'app/services' + ], + ) - expect_files_to_exist expected_files_after + expect_files_to_not_exist([ + 'packs/organisms/app/services/eagle.rb', + 'packs/organisms/app/services/other_bird.rb', + 'packs/organisms/app/services/vulture.rb', + 'app/services/salads/types/cobb.rb', + 'spec/services/salads/types/cobb_spec.rb', + ]) + + expected_files_after = [ + 'packs/food/app/public/tomato.rb', + 'packs/food/app/services/salad.rb', + 'packs/food/app/services/salads/dressing.rb', + 'packs/food/spec/public/tomato_spec.rb', + 'packs/food/spec/services/salad_spec.rb', + 'packs/food/spec/services/salads/dressing_spec.rb', + 'packs/food/app/services/eagle.rb', + 'packs/food/app/services/other_bird.rb', + 'packs/food/app/services/vulture.rb', + 'packs/food/app/services/salads/types/cobb.rb', + 'packs/food/spec/services/salads/types/cobb_spec.rb', + ] + + expect_files_to_exist expected_files_after + end end - end - context 'packs have files of the same name' do - before { app_with_files_and_directories_with_same_names } - - it 'leaves the origin and destination in the same place' do - expected_files_before = [ - # Files in food pack - 'packs/food/app/public/tomato.rb', - 'packs/food/app/services/salad.rb', - 'packs/food/app/services/salads/dressing.rb', - 'packs/food/spec/public/tomato_spec.rb', - 'packs/food/spec/services/salad_spec.rb', - 'packs/food/spec/services/salads/dressing_spec.rb', - # Files in organisms pack - 'packs/organisms/app/public/tomato.rb', - 'packs/organisms/app/services/eagle.rb', - 'packs/organisms/app/services/other_bird.rb', - 'packs/organisms/app/services/vulture.rb', - # Files in monolith - 'app/services/salads/types/cobb.rb', - 'spec/services/salads/types/cobb_spec.rb', - ] + context 'packs have files of the same name' do + before { app_with_files_and_directories_with_same_names } + + it 'leaves the origin and destination in the same place' do + expected_files_before = [ + # Files in food pack + 'packs/food/app/public/tomato.rb', + 'packs/food/app/services/salad.rb', + 'packs/food/app/services/salads/dressing.rb', + 'packs/food/spec/public/tomato_spec.rb', + 'packs/food/spec/services/salad_spec.rb', + 'packs/food/spec/services/salads/dressing_spec.rb', + # Files in organisms pack + 'packs/organisms/app/public/tomato.rb', + 'packs/organisms/app/services/eagle.rb', + 'packs/organisms/app/services/other_bird.rb', + 'packs/organisms/app/services/vulture.rb', + # Files in monolith + 'app/services/salads/types/cobb.rb', + 'spec/services/salads/types/cobb_spec.rb', + ] - expect_files_to_exist expected_files_before + expect_files_to_exist expected_files_before - UsePackwerk.move_to_pack!( - pack_name: 'packs/food', - paths_relative_to_root: [ - 'packs/organisms/app/public', - ], - ) + UsePackwerk.move_to_pack!( + pack_name: 'packs/food', + paths_relative_to_root: [ + 'packs/organisms/app/public', + ], + ) - expected_files_after = [ - 'packs/food/app/public/tomato.rb', - 'packs/organisms/app/public/tomato.rb', - ] + expected_files_after = [ + 'packs/food/app/public/tomato.rb', + 'packs/organisms/app/public/tomato.rb', + ] - expect_files_to_exist expected_files_after + expect_files_to_exist expected_files_after + end end - end - - describe 'creating a TODO.md inside app/public' do - let(:expected_todo) do - <<~TODO - This directory holds your public API! - Any classes, constants, or modules that you want other packs to use and you intend to support should go in here. - Anything that is considered private should go in other folders. + describe 'creating a TODO.md inside app/public' do + let(:expected_todo) do + <<~TODO + This directory holds your public API! - If another pack uses classes, constants, or modules that are not in your public folder, it will be considered a "privacy violation" by packwerk. - You can prevent other packs from using private API by using package_protections. + Any classes, constants, or modules that you want other packs to use and you intend to support should go in here. + Anything that is considered private should go in other folders. - Want to find how your private API is being used today? - Try running: `bin/use_packwerk list_top_privacy_violations packs/organisms` + If another pack uses classes, constants, or modules that are not in your public folder, it will be considered a "privacy violation" by packwerk. + You can prevent other packs from using private API by using package_protections. - Want to move something into this folder? - Try running: `bin/use_packwerk make_public packs/organisms/path/to/file.rb` + Want to find how your private API is being used today? + Try running: `bin/use_packwerk list_top_privacy_violations packs/organisms` - One more thing -- feel free to delete this file and replace it with a README.md describing your package in the main package directory. + Want to move something into this folder? + Try running: `bin/use_packwerk make_public packs/organisms/path/to/file.rb` - See #{UsePackwerk.config.documentation_link} for more info! - TODO - end + One more thing -- feel free to delete this file and replace it with a README.md describing your package in the main package directory. - let(:create_pack) do - UsePackwerk.create_pack!( - pack_name: 'packs/organisms', - enforce_privacy: true, - ) - end - - context 'app has public dir but nothing inside of it' do - before { app_with_nothing_in_public_dir } - - it 'adds a TODO.md file letting someone know what to do with it' do - create_pack - actual_todo = packages.first.directory.join('app/public/TODO.md').read - expect(actual_todo).to eq expected_todo + See #{UsePackwerk.config.documentation_link} for more info! + TODO end - end - context 'app has no public dir' do - before { app_with_no_public_dir } - - it 'adds a TODO.md file letting someone know what to do with it' do - create_pack - actual_todo = packages.first.directory.join('app/public/TODO.md').read - expect(actual_todo).to eq expected_todo + let(:create_pack) do + UsePackwerk.create_pack!( + pack_name: 'packs/organisms', + enforce_privacy: true, + ) end - end - context 'app with one file in public dir' do - before { app_with_file_in_public_dir } + context 'app has public dir but nothing inside of it' do + before { app_with_nothing_in_public_dir } - it 'adds a TODO.md file letting someone know what to do with it' do - create_pack - todo_file = packages.first.directory.join('app/public/TODO.md') - expect(todo_file.exist?).to eq false + it 'adds a TODO.md file letting someone know what to do with it' do + create_pack + actual_todo = packages.first.directory.join('app/public/TODO.md').read + expect(actual_todo).to eq expected_todo + end end - end - end - describe 'setting the README' do - let(:expected_readme_todo) do - <<~EXPECTED - Welcome to `packs/organisms`! + context 'app has no public dir' do + before { app_with_no_public_dir } - If you're the author, please consider replacing this file with a README.md, which may contain: - - What your pack is and does - - How you expect people to use your pack - - Example usage of your pack's public API (which lives in `packs/organisms/app/public`) - - Limitations, risks, and important considerations of usage - - How to get in touch with eng and other stakeholders for questions or issues pertaining to this pack (note: it is recommended to add ownership in `packs/organisms/package.yml` under the `owner` metadata key) - - What SLAs/SLOs (service level agreements/objectives), if any, your package provides - - When in doubt, keep it simple - - Anything else you may want to include! + it 'adds a TODO.md file letting someone know what to do with it' do + create_pack + actual_todo = packages.first.directory.join('app/public/TODO.md').read + expect(actual_todo).to eq expected_todo + end + end - README.md files are under version control and should change as your public API changes. + context 'app with one file in public dir' do + before { app_with_file_in_public_dir } - See #{UsePackwerk.config.documentation_link} for more info! - EXPECTED + it 'adds a TODO.md file letting someone know what to do with it' do + create_pack + todo_file = packages.first.directory.join('app/public/TODO.md') + expect(todo_file.exist?).to eq false + end + end end - let(:create_pack) do - UsePackwerk.create_pack!( - pack_name: 'packs/organisms', - enforce_privacy: true, - ) - end + describe 'setting the README' do + let(:expected_readme_todo) do + <<~EXPECTED + Welcome to `packs/organisms`! - context 'app has no packs' do - before do - write_file('package.yml', <<~CONTENTS) - enforce_privacy: true - enforce_dependencies: true - CONTENTS - end + If you're the author, please consider replacing this file with a README.md, which may contain: + - What your pack is and does + - How you expect people to use your pack + - Example usage of your pack's public API (which lives in `packs/organisms/app/public`) + - Limitations, risks, and important considerations of usage + - How to get in touch with eng and other stakeholders for questions or issues pertaining to this pack (note: it is recommended to add ownership in `packs/organisms/package.yml` under the `owner` metadata key) + - What SLAs/SLOs (service level agreements/objectives), if any, your package provides + - When in doubt, keep it simple + - Anything else you may want to include! - it 'adds a README_TODO.md file as a placeholder' do - create_pack + README.md files are under version control and should change as your public API changes. - actual_readme_todo = only_nonroot_package.directory.join('README_TODO.md') - expect(actual_readme_todo.read).to eq expected_readme_todo + See #{UsePackwerk.config.documentation_link} for more info! + EXPECTED end - end - context 'app has one pack without a README' do - before do - write_file('packs/organisms/package.yml', <<~CONTENTS) - enforce_privacy: true - enforce_dependencies: true - metadata: - protections: - prevent_this_package_from_violating_its_stated_dependencies: fail_on_new - prevent_other_packages_from_using_this_packages_internals: fail_on_new - prevent_this_package_from_exposing_an_untyped_api: fail_on_new - prevent_this_package_from_creating_other_namespaces: fail_on_new - CONTENTS + let(:create_pack) do + UsePackwerk.create_pack!( + pack_name: 'packs/organisms', + enforce_privacy: true, + ) end - it 'adds a README_TODO.md file as a placeholder' do - create_pack - actual_readme_todo = packages.first.directory.join('README_TODO.md') - expect(actual_readme_todo.read).to eq expected_readme_todo - end - end + context 'app has no packs' do + before do + write_file('package.yml', <<~CONTENTS) + enforce_privacy: true + enforce_dependencies: true + CONTENTS + end - context 'app has one pack with an outdated README_TODO.md' do - before do - write_file('packs/organisms/README_TODO.md', <<~CONTENTS) - This is outdated! - CONTENTS + it 'adds a README_TODO.md file as a placeholder' do + create_pack - write_file('packs/organisms/package.yml', <<~CONTENTS) - enforce_privacy: true - enforce_dependencies: true - metadata: - protections: - prevent_this_package_from_violating_its_stated_dependencies: fail_on_new - prevent_other_packages_from_using_this_packages_internals: fail_on_new - prevent_this_package_from_exposing_an_untyped_api: fail_on_new - prevent_this_package_from_creating_other_namespaces: fail_on_new - CONTENTS + actual_readme_todo = only_nonroot_package.directory.join('README_TODO.md') + expect(actual_readme_todo.read).to eq expected_readme_todo + end end - it 'adds a README_TODO.md file as a placeholder' do - actual_readme_todo = packages.first.directory.join('README_TODO.md') - expect(actual_readme_todo.read).to eq "This is outdated!\n" - create_pack + context 'app has one pack without a README' do + before do + write_file('packs/organisms/package.yml', <<~CONTENTS) + enforce_privacy: true + enforce_dependencies: true + metadata: + protections: + prevent_this_package_from_violating_its_stated_dependencies: fail_on_new + prevent_other_packages_from_using_this_packages_internals: fail_on_new + prevent_this_package_from_exposing_an_untyped_api: fail_on_new + prevent_this_package_from_creating_other_namespaces: fail_on_new + CONTENTS + end - expect(actual_readme_todo.read).to eq expected_readme_todo + it 'adds a README_TODO.md file as a placeholder' do + create_pack + actual_readme_todo = packages.first.directory.join('README_TODO.md') + expect(actual_readme_todo.read).to eq expected_readme_todo + end end - end - context 'app has one pack with a README.md' do - before do - write_file('packs/organisms/package.yml', <<~CONTENTS) - enforce_privacy: true - enforce_dependencies: true - metadata: - protections: - prevent_this_package_from_violating_its_stated_dependencies: fail_on_new - prevent_other_packages_from_using_this_packages_internals: fail_on_new - prevent_this_package_from_exposing_an_untyped_api: fail_on_new - prevent_this_package_from_creating_other_namespaces: fail_on_new - CONTENTS + context 'app has one pack with an outdated README_TODO.md' do + before do + write_file('packs/organisms/README_TODO.md', <<~CONTENTS) + This is outdated! + CONTENTS + + write_file('packs/organisms/package.yml', <<~CONTENTS) + enforce_privacy: true + enforce_dependencies: true + metadata: + protections: + prevent_this_package_from_violating_its_stated_dependencies: fail_on_new + prevent_other_packages_from_using_this_packages_internals: fail_on_new + prevent_this_package_from_exposing_an_untyped_api: fail_on_new + prevent_this_package_from_creating_other_namespaces: fail_on_new + CONTENTS + end - write_file('packs/organisms/README.md', <<~CONTENTS) - This is a readme! - CONTENTS + it 'adds a README_TODO.md file as a placeholder' do + actual_readme_todo = packages.first.directory.join('README_TODO.md') + expect(actual_readme_todo.read).to eq "This is outdated!\n" + create_pack + + expect(actual_readme_todo.read).to eq expected_readme_todo + end end - it 'adds a README_TODO.md file as a placeholder' do - actual_readme_todo = packages.first.directory.join('README_TODO.md') - create_pack - expect(actual_readme_todo.exist?).to eq false + context 'app has one pack with a README.md' do + before do + write_file('packs/organisms/package.yml', <<~CONTENTS) + enforce_privacy: true + enforce_dependencies: true + metadata: + protections: + prevent_this_package_from_violating_its_stated_dependencies: fail_on_new + prevent_other_packages_from_using_this_packages_internals: fail_on_new + prevent_this_package_from_exposing_an_untyped_api: fail_on_new + prevent_this_package_from_creating_other_namespaces: fail_on_new + CONTENTS + + write_file('packs/organisms/README.md', <<~CONTENTS) + This is a readme! + CONTENTS + end + + it 'adds a README_TODO.md file as a placeholder' do + actual_readme_todo = packages.first.directory.join('README_TODO.md') + create_pack + expect(actual_readme_todo.exist?).to eq false + end end end - end - context 'pack is in gems' do - let(:pack_name) { 'gems/my_sick_new_pack' } + context 'pack is in gems' do + let(:pack_name) { 'gems/my_sick_new_pack' } - it 'can move files from a monolith into a package' do - complex_app + it 'can move files from a monolith into a package' do + complex_app - expected_files_before = [ - # Files in monolith - 'app/services/horse_like/zebra.rb', - 'app/services/horse_like/donkey.rb', - 'app/services/horse_like/horse.rb', - 'app/services/horse_like/zebra.rb', - 'app/services/fish_like/small_ones/goldfish.rb', - 'app/services/fish_like/small_ones/seahorse.rb', - 'app/services/fish_like/big_ones/whale.rb', - # Specs in monolith - 'spec/services/dog_like/golden_retriever_spec.rb', - 'spec/services/fish_like/big_ones/whale_spec.rb', - 'spec/services/horse_like/donkey_spec.rb', - ] + expected_files_before = [ + # Files in monolith + 'app/services/horse_like/zebra.rb', + 'app/services/horse_like/donkey.rb', + 'app/services/horse_like/horse.rb', + 'app/services/horse_like/zebra.rb', + 'app/services/fish_like/small_ones/goldfish.rb', + 'app/services/fish_like/small_ones/seahorse.rb', + 'app/services/fish_like/big_ones/whale.rb', + # Specs in monolith + 'spec/services/dog_like/golden_retriever_spec.rb', + 'spec/services/fish_like/big_ones/whale_spec.rb', + 'spec/services/horse_like/donkey_spec.rb', + ] + + expect_files_to_exist expected_files_before - expect_files_to_exist expected_files_before + create_pack + move_to_pack + expect_files_to_not_exist expected_files_before + + expected_files_after = [ + 'gems/my_sick_new_pack/app/services/horse_like/zebra.rb', + 'gems/my_sick_new_pack/app/services/horse_like/donkey.rb', + 'gems/my_sick_new_pack/app/services/horse_like/horse.rb', + 'gems/my_sick_new_pack/app/services/horse_like/zebra.rb', + 'gems/my_sick_new_pack/app/services/fish_like/small_ones/goldfish.rb', + 'gems/my_sick_new_pack/app/services/fish_like/small_ones/seahorse.rb', + 'gems/my_sick_new_pack/app/services/fish_like/big_ones/whale.rb', + 'gems/my_sick_new_pack/spec/services/dog_like/golden_retriever_spec.rb', + 'gems/my_sick_new_pack/spec/services/fish_like/big_ones/whale_spec.rb', + 'gems/my_sick_new_pack/spec/services/horse_like/donkey_spec.rb', + ] + + expect_files_to_exist expected_files_after + end - create_pack - move_to_pack - expect_files_to_not_exist expected_files_before + it 'can move files from one pack to another pack' do + complex_app - expected_files_after = [ - 'gems/my_sick_new_pack/app/services/horse_like/zebra.rb', - 'gems/my_sick_new_pack/app/services/horse_like/donkey.rb', - 'gems/my_sick_new_pack/app/services/horse_like/horse.rb', - 'gems/my_sick_new_pack/app/services/horse_like/zebra.rb', - 'gems/my_sick_new_pack/app/services/fish_like/small_ones/goldfish.rb', - 'gems/my_sick_new_pack/app/services/fish_like/small_ones/seahorse.rb', - 'gems/my_sick_new_pack/app/services/fish_like/big_ones/whale.rb', - 'gems/my_sick_new_pack/spec/services/dog_like/golden_retriever_spec.rb', - 'gems/my_sick_new_pack/spec/services/fish_like/big_ones/whale_spec.rb', - 'gems/my_sick_new_pack/spec/services/horse_like/donkey_spec.rb', - ] + expected_files_before = [ + # Files in packs + 'packs/organisms/app/services/bird_like/eagle.rb', + 'packs/organisms/app/services/bird_like/swan.rb', + 'packs/organisms/app/services/bug_like/fly.rb', + # Specs in packs + 'packs/organisms/spec/services/bird_like/eagle_spec.rb', + 'packs/organisms/spec/services/bug_like/fly_spec.rb', + ] - expect_files_to_exist expected_files_after - end + expect_files_to_exist expected_files_before - it 'can move files from one pack to another pack' do - complex_app + create_pack + move_to_pack - expected_files_before = [ - # Files in packs - 'packs/organisms/app/services/bird_like/eagle.rb', - 'packs/organisms/app/services/bird_like/swan.rb', - 'packs/organisms/app/services/bug_like/fly.rb', - # Specs in packs - 'packs/organisms/spec/services/bird_like/eagle_spec.rb', - 'packs/organisms/spec/services/bug_like/fly_spec.rb', - ] + expect_files_to_not_exist expected_files_before - expect_files_to_exist expected_files_before + expected_files_after = [ + 'gems/my_sick_new_pack/app/services/bird_like/eagle.rb', + 'gems/my_sick_new_pack/app/services/bird_like/swan.rb', + 'gems/my_sick_new_pack/app/services/bug_like/fly.rb', + 'gems/my_sick_new_pack/spec/services/bird_like/eagle_spec.rb', + 'gems/my_sick_new_pack/spec/services/bug_like/fly_spec.rb', + ] - create_pack - move_to_pack + expect_files_to_exist expected_files_after + end - expect_files_to_not_exist expected_files_before + it 'can move files from one one gem to another' do + complex_app - expected_files_after = [ - 'gems/my_sick_new_pack/app/services/bird_like/eagle.rb', - 'gems/my_sick_new_pack/app/services/bird_like/swan.rb', - 'gems/my_sick_new_pack/app/services/bug_like/fly.rb', - 'gems/my_sick_new_pack/spec/services/bird_like/eagle_spec.rb', - 'gems/my_sick_new_pack/spec/services/bug_like/fly_spec.rb', - ] + expected_files_before = [ 'gems/my_gem/app/services/my_gem_service.rb' ] - expect_files_to_exist expected_files_after - end + expect_files_to_exist expected_files_before - it 'can move files from one one gem to another' do - complex_app - expected_files_before = [ 'gems/my_gem/app/services/my_gem_service.rb' ] + UsePackwerk.create_pack!(pack_name: pack_name) - expect_files_to_exist expected_files_before + UsePackwerk.move_to_pack!( + pack_name: pack_name, + paths_relative_to_root: ['gems/my_gem/app/services/my_gem_service.rb'], + ) + expect_files_to_not_exist expected_files_before - UsePackwerk.create_pack!(pack_name: pack_name) + expected_files_after = [ + 'gems/my_sick_new_pack/app/services/my_gem_service.rb', + ] - UsePackwerk.move_to_pack!( - pack_name: pack_name, - paths_relative_to_root: ['gems/my_gem/app/services/my_gem_service.rb'], - ) + expect_files_to_exist expected_files_after + end + end - expect_files_to_not_exist expected_files_before + context 'in a pack with various ownership' do + before do + write_file('app/services/owned_by_chefs_2/sandwich.rb', <<~CONTENTS) + # typed: false + + # content + CONTENTS - expected_files_after = [ - 'gems/my_sick_new_pack/app/services/my_gem_service.rb', - ] + write_file('app/services/owned_by_chefs/sandwich.rb', <<~CONTENTS) + # @team Chefs + + # content + CONTENTS - expect_files_to_exist expected_files_after - end - end + write_file('app/services/owned_by_artists/paintbrush.rb', <<~CONTENTS) + # @team Artists + + # content + CONTENTS - context 'in a pack with various ownership' do - before do - write_file('app/services/owned_by_chefs_2/sandwich.rb', <<~CONTENTS) - # typed: false - - # content - CONTENTS + write_file('config/teams/art/artists.yml', <<~CONTENTS) + name: Artists + CONTENTS - write_file('app/services/owned_by_chefs/sandwich.rb', <<~CONTENTS) - # @team Chefs - - # content - CONTENTS + write_file('config/teams/food/chefs.yml', <<~CONTENTS) + name: Chefs + owned_globs: + - app/services/owned_by_chefs_2/** + - spec/services/owned_by_chefs_2/** + CONTENTS - write_file('app/services/owned_by_artists/paintbrush.rb', <<~CONTENTS) - # @team Artists - - # content - CONTENTS + write_file('spec/services/owned_by_chefs/sandwich_spec.rb', <<~CONTENTS) + # @team Chefs + CONTENTS - write_file('config/teams/art/artists.yml', <<~CONTENTS) - name: Artists - CONTENTS + write_file('spec/services/owned_by_artists/paintbrush_spec.rb', <<~CONTENTS) + # @team Artists + CONTENTS - write_file('config/teams/food/chefs.yml', <<~CONTENTS) - name: Chefs - owned_globs: - - app/services/owned_by_chefs_2/** - - spec/services/owned_by_chefs_2/** - CONTENTS + write_file('package.yml', <<~CONTENTS) + enforce_dependencies: true + enforce_privacy: true + metadata: + owner: Artists + CONTENTS - write_file('spec/services/owned_by_chefs/sandwich_spec.rb', <<~CONTENTS) - # @team Chefs - CONTENTS + write_file('packs/owned_by_artists/app/public/paint.rb', <<~CONTENTS) + # typed: strict + + # content + CONTENTS - write_file('spec/services/owned_by_artists/paintbrush_spec.rb', <<~CONTENTS) - # @team Artists - CONTENTS + write_file('packs/owned_by_artists/spec/public/paint_spec.rb', <<~CONTENTS) + # typed: strict + CONTENTS + end - write_file('package.yml', <<~CONTENTS) - enforce_dependencies: true - enforce_privacy: true - metadata: - owner: Artists - CONTENTS + let(:create_pack) do + UsePackwerk.create_pack!( + pack_name: pack_name, + ) + end - write_file('packs/owned_by_artists/app/public/paint.rb', <<~CONTENTS) - # typed: strict - - # content - CONTENTS + let(:move_to_pack) do + UsePackwerk.move_to_pack!( + pack_name: pack_name, + paths_relative_to_root: %w( + app/services/owned_by_chefs/sandwich.rb + app/services/owned_by_chefs_2/sandwich.rb + app/services/owned_by_artists/paintbrush.rb + packs/owned_by_artists/app/public/paint.rb + ), + per_file_processors: [UsePackwerk::RubocopPostProcessor.new, UsePackwerk::CodeOwnershipPostProcessor.new], + ) + end - write_file('packs/owned_by_artists/spec/public/paint_spec.rb', <<~CONTENTS) - # typed: strict - CONTENTS - end + it 'prints out the right ownership' do + logged_output = "" - let(:create_pack) do - UsePackwerk.create_pack!( - pack_name: pack_name, - ) - end + expect(UsePackwerk::Logging).to receive(:print).at_least(:once) do |string| + logged_output += string + logged_output += "\n" + end - let(:move_to_pack) do - UsePackwerk.move_to_pack!( - pack_name: pack_name, - paths_relative_to_root: %w( - app/services/owned_by_chefs/sandwich.rb - app/services/owned_by_chefs_2/sandwich.rb - app/services/owned_by_artists/paintbrush.rb - packs/owned_by_artists/app/public/paint.rb - ), - per_file_processors: [UsePackwerk::RubocopPostProcessor.new, UsePackwerk::CodeOwnershipPostProcessor.new], - ) - end + create_pack + move_to_pack - it 'prints out the right ownership' do - logged_output = "" + expected_logged_output = <<~OUTPUT + This section contains info about the current ownership distribution of the moved files. + Artists - 4 files + Chefs - 3 files + OUTPUT - expect(UsePackwerk::Logging).to receive(:print).at_least(:once) do |string| - logged_output += string - logged_output += "\n" + expect(logged_output).to include expected_logged_output end + + it 'removes file annotations if the destination pack has file annotations' do + logged_output = "" - create_pack - move_to_pack + expect(UsePackwerk::Logging).to receive(:print).at_least(:once) do |string| + logged_output += string + logged_output += "\n" + end - expected_logged_output = <<~OUTPUT - This section contains info about the current ownership distribution of the moved files. - Artists - 4 files - Chefs - 3 files - OUTPUT + create_pack + ParsePackwerk.bust_cache! - expect(logged_output).to include expected_logged_output - end - - it 'removes file annotations if the destination pack has file annotations' do - logged_output = "" + expect(Pathname.new('app/services/owned_by_chefs/sandwich.rb').read).to eq <<~RUBY + # @team Chefs - expect(UsePackwerk::Logging).to receive(:print).at_least(:once) do |string| - logged_output += string - logged_output += "\n" - end + # content + RUBY + expect(Pathname.new('app/services/owned_by_chefs_2/sandwich.rb').read).to eq <<~RUBY + # typed: false - create_pack - ParsePackwerk.bust_cache! + # content + RUBY + expect(Pathname.new('app/services/owned_by_artists/paintbrush.rb').read).to eq <<~RUBY + # @team Artists - expect(Pathname.new('app/services/owned_by_chefs/sandwich.rb').read).to eq <<~RUBY - # @team Chefs + # content + RUBY + expect(Pathname.new('packs/owned_by_artists/app/public/paint.rb').read).to eq <<~RUBY + # typed: strict - # content - RUBY - expect(Pathname.new('app/services/owned_by_chefs_2/sandwich.rb').read).to eq <<~RUBY - # typed: false + # content + RUBY - # content - RUBY - expect(Pathname.new('app/services/owned_by_artists/paintbrush.rb').read).to eq <<~RUBY - # @team Artists + # Set the package to be owned by `Artists`` + team = instance_double(CodeTeams::Team) + package = ParsePackwerk.all.find {|p| p.name == pack_name} + allow(CodeOwnership).to receive(:for_package).with(anything).and_return(team) + bust_cache_and_configure_code_ownership! - # content - RUBY - expect(Pathname.new('packs/owned_by_artists/app/public/paint.rb').read).to eq <<~RUBY - # typed: strict + move_to_pack - # content - RUBY + expected_logged_output = <<~OUTPUT + This section contains info about the current ownership distribution of the moved files. + Artists - 4 files + Chefs - 3 files + Since the destination package has package-based ownership, file-annotations were removed from moved files. + OUTPUT - # Set the package to be owned by `Artists`` - team = instance_double(CodeTeams::Team) - package = ParsePackwerk.all.find {|p| p.name == pack_name} - allow(CodeOwnership).to receive(:for_package).with(anything).and_return(team) - bust_cache_and_configure_code_ownership! + expect(logged_output).to include expected_logged_output - move_to_pack + expect(Pathname.new('packs/animals/app/services/owned_by_chefs/sandwich.rb').read).to eq (<<~RUBY) + # content + RUBY - expected_logged_output = <<~OUTPUT - This section contains info about the current ownership distribution of the moved files. - Artists - 4 files - Chefs - 3 files - Since the destination package has package-based ownership, file-annotations were removed from moved files. - OUTPUT + expect(Pathname.new('packs/animals/app/services/owned_by_chefs_2/sandwich.rb').read).to eq <<~RUBY + # typed: false - expect(logged_output).to include expected_logged_output + # content + RUBY + expect(Pathname.new('packs/animals/app/services/owned_by_artists/paintbrush.rb').read).to eq (<<~RUBY) + # content + RUBY + expect(Pathname.new('packs/animals/app/public/paint.rb').read).to eq <<~RUBY + # typed: strict - expect(Pathname.new('packs/animals/app/services/owned_by_chefs/sandwich.rb').read).to eq (<<~RUBY) - # content - RUBY - - expect(Pathname.new('packs/animals/app/services/owned_by_chefs_2/sandwich.rb').read).to eq <<~RUBY - # typed: false + # content + RUBY + end + end - # content - RUBY - expect(Pathname.new('packs/animals/app/services/owned_by_artists/paintbrush.rb').read).to eq (<<~RUBY) - # content - RUBY - expect(Pathname.new('packs/animals/app/public/paint.rb').read).to eq <<~RUBY - # typed: strict + context 'files moved are tasks in lib' do + let(:move_to_pack) do + UsePackwerk.move_to_pack!( + pack_name: pack_name, + paths_relative_to_root: [ + 'lib/tasks/my_task.rake', + 'packs/organisms/lib/tasks/my_organism_task.rake', + ], + ) + end - # content - RUBY - end - end + it 'can move files from lib from one pack to another pack' do + complex_app - context 'files moved are tasks in lib' do - let(:move_to_pack) do - UsePackwerk.move_to_pack!( - pack_name: pack_name, - paths_relative_to_root: [ + expected_files_before = [ 'lib/tasks/my_task.rake', + 'spec/lib/tasks/my_task_spec.rb', 'packs/organisms/lib/tasks/my_organism_task.rake', - ], - ) - end - - it 'can move files from lib from one pack to another pack' do - complex_app + 'packs/organisms/spec/lib/tasks/my_organism_task_spec.rb', + ] - expected_files_before = [ - 'lib/tasks/my_task.rake', - 'spec/lib/tasks/my_task_spec.rb', - 'packs/organisms/lib/tasks/my_organism_task.rake', - 'packs/organisms/spec/lib/tasks/my_organism_task_spec.rb', - ] + expect_files_to_exist expected_files_before - expect_files_to_exist expected_files_before - - create_pack - move_to_pack + create_pack + move_to_pack - expect_files_to_not_exist expected_files_before + expect_files_to_not_exist expected_files_before - expected_files_after = [ - 'packs/animals/lib/tasks/my_task.rake', - 'packs/animals/spec/lib/tasks/my_task_spec.rb', - 'packs/animals/lib/tasks/my_organism_task.rake', - 'packs/animals/spec/lib/tasks/my_organism_task_spec.rb', - ] + expected_files_after = [ + 'packs/animals/lib/tasks/my_task.rake', + 'packs/animals/spec/lib/tasks/my_task_spec.rb', + 'packs/animals/lib/tasks/my_organism_task.rake', + 'packs/animals/spec/lib/tasks/my_organism_task_spec.rb', + ] - expect_files_to_exist expected_files_after + expect_files_to_exist expected_files_after + end end end end From fe960a23683665fc21d18cb2cdeaf17a25996849 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 07:48:48 -0400 Subject: [PATCH 06/19] add new tests for child => parent pack file move --- spec/support/app_fixtures.rb | 19 +++++++++++++++++ spec/use_packwerk_spec.rb | 41 ++++++++++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/spec/support/app_fixtures.rb b/spec/support/app_fixtures.rb index 8c5a2df..f7ed555 100644 --- a/spec/support/app_fixtures.rb +++ b/spec/support/app_fixtures.rb @@ -155,6 +155,25 @@ def empty_function; end # so rubocop does not complain def empty_function; end CONTENTS + + write_file('packs/organisms/birds/package.yml', <<~CONTENTS) + enforce_privacy: true + enforce_dependencies: true + metadata: + protections: + prevent_this_package_from_violating_its_stated_dependencies: fail_on_new + prevent_other_packages_from_using_this_packages_internals: fail_on_new + prevent_this_package_from_exposing_an_untyped_api: fail_on_new + prevent_this_package_from_creating_other_namespaces: fail_on_new + CONTENTS + + write_file('packs/organisms/birds/app/services/emu.rb', <<~CONTENTS) + CONTENTS + write_file('packs/organisms/birds/spec/services/emu_spec.rb', <<~CONTENTS) + # typed: true + # so rubocop does not complain + def empty_function; end + CONTENTS end let(:app_with_nothing_in_public_dir) do diff --git a/spec/use_packwerk_spec.rb b/spec/use_packwerk_spec.rb index 3bb023c..9c6150c 100644 --- a/spec/use_packwerk_spec.rb +++ b/spec/use_packwerk_spec.rb @@ -254,10 +254,12 @@ def bust_cache_and_configure_code_ownership! 'app/services/fish_like/small_ones', 'app/services/fish_like/big_ones', 'app/services/dog_like/golden_retriever.rb', - # Files in packs + # Files in parent packs 'packs/organisms/app/services/bird_like/eagle.rb', 'packs/organisms/app/services/bird_like/swan.rb', 'packs/organisms/app/services/bug_like/fly.rb', + # Files in child packs + 'packs/organisms/birds/app/services/emu.rb', ], per_file_processors: [UsePackwerk::RubocopPostProcessor.new, UsePackwerk::CodeOwnershipPostProcessor.new], ) @@ -313,7 +315,38 @@ def bust_cache_and_configure_code_ownership! expect_files_to_exist expected_files_after end - it 'can move files from one pack to another pack' do + it 'can move files from a parent pack to another parent pack' do + complex_app + + expected_files_before = [ + # Files in packs + 'packs/organisms/app/services/bird_like/eagle.rb', + 'packs/organisms/app/services/bird_like/swan.rb', + 'packs/organisms/app/services/bug_like/fly.rb', + # Specs in packs + 'packs/organisms/spec/services/bird_like/eagle_spec.rb', + 'packs/organisms/spec/services/bug_like/fly_spec.rb', + ] + + expect_files_to_exist expected_files_before + + create_pack + move_to_pack + + expect_files_to_not_exist expected_files_before + + expected_files_after = [ + 'packs/animals/app/services/bird_like/eagle.rb', + 'packs/animals/app/services/bird_like/swan.rb', + 'packs/animals/app/services/bug_like/fly.rb', + 'packs/animals/spec/services/bird_like/eagle_spec.rb', + 'packs/animals/spec/services/bug_like/fly_spec.rb', + ] + + expect_files_to_exist expected_files_after + end + + it 'can move files from a child pack to a parent pack' do complex_app expected_files_before = [ @@ -321,9 +354,11 @@ def bust_cache_and_configure_code_ownership! 'packs/organisms/app/services/bird_like/eagle.rb', 'packs/organisms/app/services/bird_like/swan.rb', 'packs/organisms/app/services/bug_like/fly.rb', + 'packs/organisms/birds/app/services/emu.rb', # Specs in packs 'packs/organisms/spec/services/bird_like/eagle_spec.rb', 'packs/organisms/spec/services/bug_like/fly_spec.rb', + 'packs/organisms/birds/spec/services/emu_spec.rb', ] expect_files_to_exist expected_files_before @@ -339,6 +374,8 @@ def bust_cache_and_configure_code_ownership! 'packs/animals/app/services/bug_like/fly.rb', 'packs/animals/spec/services/bird_like/eagle_spec.rb', 'packs/animals/spec/services/bug_like/fly_spec.rb', + 'packs/animals/app/services/emu.rb', + 'packs/animals/spec/services/emu_spec.rb', ] expect_files_to_exist expected_files_after From 86942c63b23475285ee8d909f8fa60d429d81237 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 07:49:58 -0400 Subject: [PATCH 07/19] fix test --- spec/use_packwerk_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/use_packwerk_spec.rb b/spec/use_packwerk_spec.rb index 9c6150c..7ac52ff 100644 --- a/spec/use_packwerk_spec.rb +++ b/spec/use_packwerk_spec.rb @@ -1334,7 +1334,7 @@ def bust_cache_and_configure_code_ownership! it 'moves the file into the public directory' do UsePackwerk.create_pack!(pack_name: 'gems/my_gem') - expected_file = only_nonroot_package.directory.join('app/public/my_gem_service.rb') + expected_file = ParsePackwerk.find('gems/my_gem').directory.join('app/public/my_gem_service.rb') expect(expected_file).to_not exist make_public From c11b2aa820e8e5fb4372a6a4245d8f2a389a396b Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 08:06:45 -0400 Subject: [PATCH 08/19] work on implementation --- .../private/file_move_operation.rb | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/lib/use_packwerk/private/file_move_operation.rb b/lib/use_packwerk/private/file_move_operation.rb index fd3e08f..17f6570 100644 --- a/lib/use_packwerk/private/file_move_operation.rb +++ b/lib/use_packwerk/private/file_move_operation.rb @@ -16,19 +16,31 @@ def origin_pack sig { params(origin_pathname: Pathname, new_package_root: Pathname).returns(Pathname) } def self.destination_pathname_for_package_move(origin_pathname, new_package_root) - parts = origin_pathname.to_s.split('/') - toplevel_directory = parts[0] + origin_pack = T.must(ParsePackwerk.package_from_path(origin_pathname)) - case toplevel_directory.to_s - # This allows us to move files from monolith to packs - when 'app', 'spec', 'lib' + require 'pry' + binding.pry if origin_pathname.to_s.include?('eagle') + new_implementation = nil + if origin_pack.name == ParsePackwerk::ROOT_PACKAGE_NAME new_package_root.join(origin_pathname).cleanpath - # This allows us to move files from packs to packs - when *PERMITTED_PACK_LOCATIONS # parts looks like ['packs', 'organisms', 'app', 'services', 'bird_like', 'eagle.rb'] - new_package_root.join(T.must(parts[2..]).join('/')).cleanpath else - raise StandardError.new("Don't know how to find destination path for #{origin_pathname.inspect}") + Pathname.new(origin_pathname.to_s.gsub(origin_pack.name, new_package_root.to_s)).cleanpath end + + # parts = origin_pathname.to_s.split('/') + # toplevel_directory = parts[0] + # case toplevel_directory.to_s + # # This allows us to move files from monolith to packs + # when 'app', 'spec', 'lib' + # new_package_root.join(origin_pathname).cleanpath + # # This allows us to move files from packs to packs + # when *PERMITTED_PACK_LOCATIONS # parts looks like ['packs', 'organisms', 'app', 'services', 'bird_like', 'eagle.rb'] + # new_package_root.join(T.must(parts[2..]).join('/')).cleanpath + # else + # raise StandardError.new("Don't know how to find destination path for #{origin_pathname.inspect}") + # end + + # raise end sig { params(origin_pathname: Pathname).returns(Pathname) } From bbf83223c16b0a297e45d8eb57fb004ef5ec3940 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 08:09:40 -0400 Subject: [PATCH 09/19] add package.yml for organisms --- spec/support/app_fixtures.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/support/app_fixtures.rb b/spec/support/app_fixtures.rb index f7ed555..48fa3cd 100644 --- a/spec/support/app_fixtures.rb +++ b/spec/support/app_fixtures.rb @@ -150,6 +150,17 @@ def empty_function; end - packs/organisms/app/services/bug_like/fly.rb CONTENTS + write_file('packs/organisms/package.yml', <<~CONTENTS) + enforce_privacy: true + enforce_dependencies: true + metadata: + protections: + prevent_this_package_from_violating_its_stated_dependencies: fail_on_new + prevent_other_packages_from_using_this_packages_internals: fail_on_new + prevent_this_package_from_exposing_an_untyped_api: fail_on_new + prevent_this_package_from_creating_other_namespaces: fail_on_new + CONTENTS + write_file('gems/my_gem/app/services/my_gem_service.rb', <<~CONTENTS) # typed: strict # so rubocop does not complain From 62793d0cd43a0be883c498c4409d63179bd87259 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 08:15:12 -0400 Subject: [PATCH 10/19] wip --- spec/use_packwerk_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/use_packwerk_spec.rb b/spec/use_packwerk_spec.rb index 7ac52ff..5fb0cd7 100644 --- a/spec/use_packwerk_spec.rb +++ b/spec/use_packwerk_spec.rb @@ -1024,6 +1024,13 @@ def bust_cache_and_configure_code_ownership! owner: Artists CONTENTS + write_file('packs/owned_by_artists/package.yml', <<~CONTENTS) + enforce_dependencies: true + enforce_privacy: true + metadata: + owner: Artists + CONTENTS + write_file('packs/owned_by_artists/app/public/paint.rb', <<~CONTENTS) # typed: strict From fa679b402f73d525a442bfccfca1899121afb39a Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 08:21:14 -0400 Subject: [PATCH 11/19] fix typo --- spec/use_packwerk_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/use_packwerk_spec.rb b/spec/use_packwerk_spec.rb index 5fb0cd7..461d0d0 100644 --- a/spec/use_packwerk_spec.rb +++ b/spec/use_packwerk_spec.rb @@ -953,14 +953,13 @@ def bust_cache_and_configure_code_ownership! expect_files_to_exist expected_files_after end - it 'can move files from one one gem to another' do + it 'can move files from one gem to another' do complex_app expected_files_before = [ 'gems/my_gem/app/services/my_gem_service.rb' ] expect_files_to_exist expected_files_before - UsePackwerk.create_pack!(pack_name: pack_name) UsePackwerk.move_to_pack!( From f0ee6a352c1ff31fcc574d98671d2f38a55db112 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 08:24:37 -0400 Subject: [PATCH 12/19] fix tests --- .../private/file_move_operation.rb | 2 -- spec/spec_helper.rb | 1 + spec/support/app_fixtures.rb | 28 +++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/lib/use_packwerk/private/file_move_operation.rb b/lib/use_packwerk/private/file_move_operation.rb index 17f6570..e06be43 100644 --- a/lib/use_packwerk/private/file_move_operation.rb +++ b/lib/use_packwerk/private/file_move_operation.rb @@ -18,8 +18,6 @@ def origin_pack def self.destination_pathname_for_package_move(origin_pathname, new_package_root) origin_pack = T.must(ParsePackwerk.package_from_path(origin_pathname)) - require 'pry' - binding.pry if origin_pathname.to_s.include?('eagle') new_implementation = nil if origin_pack.name == ParsePackwerk::ROOT_PACKAGE_NAME new_package_root.join(origin_pathname).cleanpath diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7118ac6..3de3bc6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,6 +4,7 @@ require 'use_packwerk' require 'tmpdir' require_relative 'support/app_fixtures' +require 'pry' RSpec.configure do |config| # Enable flags like --only-failures and --next-failure diff --git a/spec/support/app_fixtures.rb b/spec/support/app_fixtures.rb index 48fa3cd..3f8ccbf 100644 --- a/spec/support/app_fixtures.rb +++ b/spec/support/app_fixtures.rb @@ -167,6 +167,23 @@ def empty_function; end def empty_function; end CONTENTS + write_file('gems/my_gem/app/services/my_gem_service.rb', <<~CONTENTS) + # typed: strict + # so rubocop does not complain + def empty_function; end + CONTENTS + + write_file('gems/my_gem/package.yml', <<~CONTENTS) + enforce_privacy: true + enforce_dependencies: true + metadata: + protections: + prevent_this_package_from_violating_its_stated_dependencies: fail_on_new + prevent_other_packages_from_using_this_packages_internals: fail_on_new + prevent_this_package_from_exposing_an_untyped_api: fail_on_new + prevent_this_package_from_creating_other_namespaces: fail_on_new + CONTENTS + write_file('packs/organisms/birds/package.yml', <<~CONTENTS) enforce_privacy: true enforce_dependencies: true @@ -243,6 +260,17 @@ def empty_function; end end let(:app_with_files_and_directories_with_same_names) do + write_file('package.yml', <<~CONTENTS) + enforce_privacy: true + enforce_dependencies: true + metadata: + protections: + prevent_this_package_from_violating_its_stated_dependencies: fail_on_new + prevent_other_packages_from_using_this_packages_internals: fail_on_new + prevent_this_package_from_exposing_an_untyped_api: fail_on_new + prevent_this_package_from_creating_other_namespaces: fail_on_new + CONTENTS + write_file('app/services/salads/types/cobb.rb', <<~CONTENTS) # typed: strict # so rubocop does not complain From 7ef105b4e687b39a1bef4b8c272a48b125874384 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 08:26:31 -0400 Subject: [PATCH 13/19] add tests for moving files between nested packs --- spec/use_packwerk_spec.rb | 133 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/spec/use_packwerk_spec.rb b/spec/use_packwerk_spec.rb index 461d0d0..8bed185 100644 --- a/spec/use_packwerk_spec.rb +++ b/spec/use_packwerk_spec.rb @@ -1188,6 +1188,139 @@ def bust_cache_and_configure_code_ownership! end end end + + context 'pack is nested' do + let(:pack_name) { 'packs/fruits/apples' } + + context 'pack not yet created' do + it 'errors' do + expect { move_to_pack }.to raise_error("Can not find package with name packs/fruits/apples. Make sure the argument is of the form `packs/my_pack/`") + end + end + + it 'can move files from a monolith into a child package' do + complex_app + + expected_files_before = [ + # Files in monolith + 'app/services/horse_like/zebra.rb', + 'app/services/horse_like/donkey.rb', + 'app/services/horse_like/horse.rb', + 'app/services/horse_like/zebra.rb', + 'app/services/fish_like/small_ones/goldfish.rb', + 'app/services/fish_like/small_ones/seahorse.rb', + 'app/services/fish_like/big_ones/whale.rb', + # Specs in monolith + 'spec/services/dog_like/golden_retriever_spec.rb', + 'spec/services/fish_like/big_ones/whale_spec.rb', + 'spec/services/horse_like/donkey_spec.rb', + ] + + expect_files_to_exist expected_files_before + + create_pack + move_to_pack + + expect_files_to_not_exist expected_files_before + + expected_files_after = [ + 'packs/fruits/apples/app/services/horse_like/zebra.rb', + 'packs/fruits/apples/app/services/horse_like/donkey.rb', + 'packs/fruits/apples/app/services/horse_like/horse.rb', + 'packs/fruits/apples/app/services/horse_like/zebra.rb', + 'packs/fruits/apples/app/services/fish_like/small_ones/goldfish.rb', + 'packs/fruits/apples/app/services/fish_like/small_ones/seahorse.rb', + 'packs/fruits/apples/app/services/fish_like/big_ones/whale.rb', + 'packs/fruits/apples/spec/services/dog_like/golden_retriever_spec.rb', + 'packs/fruits/apples/spec/services/fish_like/big_ones/whale_spec.rb', + 'packs/fruits/apples/spec/services/horse_like/donkey_spec.rb', + ] + + expect_files_to_exist expected_files_after + end + + it 'can move files from one pack to another pack' do + complex_app + + expected_files_before = [ + # Files in packs + 'packs/organisms/app/services/bird_like/eagle.rb', + 'packs/organisms/app/services/bird_like/swan.rb', + 'packs/organisms/app/services/bug_like/fly.rb', + # Specs in packs + 'packs/organisms/spec/services/bird_like/eagle_spec.rb', + 'packs/organisms/spec/services/bug_like/fly_spec.rb', + ] + + expect_files_to_exist expected_files_before + + create_pack + move_to_pack + + expect_files_to_not_exist expected_files_before + + expected_files_after = [ + 'packs/fruits/apples/app/services/bird_like/eagle.rb', + 'packs/fruits/apples/app/services/bird_like/swan.rb', + 'packs/fruits/apples/app/services/bug_like/fly.rb', + 'packs/fruits/apples/spec/services/bird_like/eagle_spec.rb', + 'packs/fruits/apples/spec/services/bug_like/fly_spec.rb', + ] + + expect_files_to_exist expected_files_after + end + + context 'directory moves have trailing slashes' do + let(:move_to_pack) do + UsePackwerk.move_to_pack!( + pack_name: pack_name, + paths_relative_to_root: [ + # Files in monolith + 'app/services/horse_like/', + 'app/services/fish_like/small_ones/', + 'app/services/fish_like/big_ones/', + 'app/services/dog_like/golden_retriever.rb', + # Files in packs + 'packs/organisms/app/services/bird_like/eagle.rb', + 'packs/organisms/app/services/bird_like/swan.rb', + 'packs/organisms/app/services/bug_like/fly.rb', + ], + per_file_processors: [UsePackwerk::RubocopPostProcessor.new, UsePackwerk::CodeOwnershipPostProcessor.new], + ) + end + + it 'can move files from one pack to another pack' do + complex_app + + expected_files_before = [ + # Files in packs + 'packs/organisms/app/services/bird_like/eagle.rb', + 'packs/organisms/app/services/bird_like/swan.rb', + 'packs/organisms/app/services/bug_like/fly.rb', + # Specs in packs + 'packs/organisms/spec/services/bird_like/eagle_spec.rb', + 'packs/organisms/spec/services/bug_like/fly_spec.rb', + ] + + expect_files_to_exist expected_files_before + + create_pack + move_to_pack + + expect_files_to_not_exist expected_files_before + + expected_files_after = [ + 'packs/fruits/apples/app/services/bird_like/eagle.rb', + 'packs/fruits/apples/app/services/bird_like/swan.rb', + 'packs/fruits/apples/app/services/bug_like/fly.rb', + 'packs/fruits/apples/spec/services/bird_like/eagle_spec.rb', + 'packs/fruits/apples/spec/services/bug_like/fly_spec.rb', + ] + + expect_files_to_exist expected_files_after + end + end + end end describe '.make_public!' do From 64dbc8ccbc84873d49e9a08aca1e30fe7b5f77ec Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 08:29:09 -0400 Subject: [PATCH 14/19] add test for public --- spec/use_packwerk_spec.rb | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/spec/use_packwerk_spec.rb b/spec/use_packwerk_spec.rb index 8bed185..bd5b70d 100644 --- a/spec/use_packwerk_spec.rb +++ b/spec/use_packwerk_spec.rb @@ -1382,6 +1382,32 @@ def bust_cache_and_configure_code_ownership! expect_files_to_exist expected_files_after end + it 'can make files in a nested pack public' do + UsePackwerk.create_pack!(pack_name: 'packs/fruits/apples') + write_file('packs/fruits/apples/app/services/apple.rb') + write_file('packs/fruits/apples/spec/services/apple_spec.rb') + + expected_files_before = [ + 'packs/fruits/apples/app/services/apple.rb', + 'packs/fruits/apples/spec/services/apple_spec.rb' + ] + + expect_files_to_exist expected_files_before + + UsePackwerk.make_public!( + paths_relative_to_root: [ 'packs/fruits/apples/app/services/apple.rb' ] + ) + + expect_files_to_not_exist expected_files_before + + expected_files_after = [ + 'packs/fruits/apples/app/public/apple.rb', + 'packs/fruits/apples/spec/public/apple_spec.rb' + ] + + expect_files_to_exist expected_files_after + end + context 'app has public dir but nothing inside of it' do before { app_with_nothing_in_public_dir } let(:file_to_make_public) { 'packs/organisms/app/services/swan.rb' } From 8bdf41f3bc4e93ed12bdf0506016eb61da3d3c33 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 08:38:52 -0400 Subject: [PATCH 15/19] fix specs --- lib/use_packwerk/private.rb | 14 +--------- .../private/file_move_operation.rb | 27 +++++++++++-------- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/lib/use_packwerk/private.rb b/lib/use_packwerk/private.rb index 117d516..2d1e0b4 100644 --- a/lib/use_packwerk/private.rb +++ b/lib/use_packwerk/private.rb @@ -197,19 +197,7 @@ def self.make_public!(paths_relative_to_root:, per_file_processors:) file_move_operations = file_paths.map do |path| - parts = path.to_s.split('/') - first_part_of_path = T.must(parts[0]) - - if Pathname.new(first_part_of_path).dirname.join(ParsePackwerk::PACKAGE_YML_NAME).exist? - package_location = Pathname.new('.') - elsif PERMITTED_PACK_LOCATIONS.include?(first_part_of_path) - package_location = Pathname.new(first_part_of_path).join(T.must(parts[1])) - else - raise StandardError.new('Can only make files in the monolith or in existing packs public') - end - - package = ParsePackwerk::Package.from(package_location.join(ParsePackwerk::PACKAGE_YML_NAME)) - + package = T.must(ParsePackwerk.package_from_path(path)) origin_pathname = Pathname.new(path).cleanpath FileMoveOperation.new( diff --git a/lib/use_packwerk/private/file_move_operation.rb b/lib/use_packwerk/private/file_move_operation.rb index e06be43..72390bf 100644 --- a/lib/use_packwerk/private/file_move_operation.rb +++ b/lib/use_packwerk/private/file_move_operation.rb @@ -43,20 +43,25 @@ def self.destination_pathname_for_package_move(origin_pathname, new_package_root sig { params(origin_pathname: Pathname).returns(Pathname) } def self.destination_pathname_for_new_public_api(origin_pathname) - parts = origin_pathname.to_s.split('/') - toplevel_directory = Pathname.new(parts[0]) - case toplevel_directory.to_s - # This allows us to make API in the monolith public - when 'app', 'spec' - toplevel_directory.join('public').join(T.must(parts[2..]).join('/')).cleanpath - # This allows us to make API in existing packs public - when *PERMITTED_PACK_LOCATIONS # parts looks like ['packs', 'organisms', 'app', 'services', 'bird_like', 'eagle.rb'] - pack_name = Pathname.new(parts[1]) - toplevel_directory.join(pack_name).join('app/public').join(T.must(parts[4..]).join('/')).cleanpath + origin_pack = T.must(ParsePackwerk.package_from_path(origin_pathname)) + if origin_pack.name == ParsePackwerk::ROOT_PACKAGE_NAME + filepath_without_pack_name = origin_pathname.to_s else - raise StandardError.new("Don't know how to find destination path for #{origin_pathname.inspect}") + filepath_without_pack_name = origin_pathname.to_s.gsub("#{origin_pack.name}/", '') end + + # We join the pack name with the rest of the path... + path_parts = filepath_without_pack_name.split("/") + Pathname.new(origin_pack.name).join( + # ... keeping the "app" or "spec" + T.must(path_parts[0]), + # ... substituting "controllers," "services," etc. with "public" + 'public', + # ... then the rest is the same + T.must(path_parts[2..]).join("/") + # and we take the cleanpath so `./app/...` becomes `app/...` + ).cleanpath end sig { returns(FileMoveOperation) } From f2db18ebba491570dc14df20faf0d003be73b234 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 08:39:02 -0400 Subject: [PATCH 16/19] remove pry --- spec/spec_helper.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3de3bc6..7118ac6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,7 +4,6 @@ require 'use_packwerk' require 'tmpdir' require_relative 'support/app_fixtures' -require 'pry' RSpec.configure do |config| # Enable flags like --only-failures and --next-failure From 53a4e0025a27cf5c55e4f7211c825e707336f773 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 08:39:11 -0400 Subject: [PATCH 17/19] remove old implementation --- lib/use_packwerk/private/file_move_operation.rb | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/lib/use_packwerk/private/file_move_operation.rb b/lib/use_packwerk/private/file_move_operation.rb index 72390bf..9898222 100644 --- a/lib/use_packwerk/private/file_move_operation.rb +++ b/lib/use_packwerk/private/file_move_operation.rb @@ -24,21 +24,6 @@ def self.destination_pathname_for_package_move(origin_pathname, new_package_root else Pathname.new(origin_pathname.to_s.gsub(origin_pack.name, new_package_root.to_s)).cleanpath end - - # parts = origin_pathname.to_s.split('/') - # toplevel_directory = parts[0] - # case toplevel_directory.to_s - # # This allows us to move files from monolith to packs - # when 'app', 'spec', 'lib' - # new_package_root.join(origin_pathname).cleanpath - # # This allows us to move files from packs to packs - # when *PERMITTED_PACK_LOCATIONS # parts looks like ['packs', 'organisms', 'app', 'services', 'bird_like', 'eagle.rb'] - # new_package_root.join(T.must(parts[2..]).join('/')).cleanpath - # else - # raise StandardError.new("Don't know how to find destination path for #{origin_pathname.inspect}") - # end - - # raise end sig { params(origin_pathname: Pathname).returns(Pathname) } From e5a5debad0f408b861db0d1bbfa9951465b51b47 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 08:39:31 -0400 Subject: [PATCH 18/19] bump version --- Gemfile.lock | 4 ++-- use_packwerk.gemspec | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 1340780..cbf0b69 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - use_packwerk (0.51.1) + use_packwerk (0.52.0) code_ownership colorize package_protections @@ -83,7 +83,7 @@ GEM rubocop-ast (>= 1.19.1, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 3.0) - rubocop-ast (1.19.1) + rubocop-ast (1.21.0) parser (>= 3.1.1.0) rubocop-sorbet (0.6.11) rubocop (>= 0.90.0) diff --git a/use_packwerk.gemspec b/use_packwerk.gemspec index b7932d0..93f11ba 100644 --- a/use_packwerk.gemspec +++ b/use_packwerk.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |spec| spec.name = 'use_packwerk' - spec.version = '0.51.1' + spec.version = '0.52.0' spec.authors = ['Gusto Engineers'] spec.email = ['dev@gusto.com'] From 188470607f85be435448f5fc8d16ae5b869cee0e Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 15 Aug 2022 08:47:37 -0400 Subject: [PATCH 19/19] type check --- sorbet/rbi/gems/package_protections@1.4.0.rbi | 83 ------------------- 1 file changed, 83 deletions(-) diff --git a/sorbet/rbi/gems/package_protections@1.4.0.rbi b/sorbet/rbi/gems/package_protections@1.4.0.rbi index 3d5665a..379720d 100644 --- a/sorbet/rbi/gems/package_protections@1.4.0.rbi +++ b/sorbet/rbi/gems/package_protections@1.4.0.rbi @@ -578,89 +578,6 @@ end module RuboCop; end module RuboCop::Cop; end -RuboCop::Cop::IgnoredMethods = RuboCop::Cop::AllowedMethods -RuboCop::Cop::IgnoredPattern = RuboCop::Cop::AllowedPattern module RuboCop::Cop::PackageProtections; end -class RuboCop::Cop::PackageProtections::NamespacedUnderPackageName < ::RuboCop::Cop::Base - include ::RuboCop::Cop::RangeHelp - include ::PackageProtections::ProtectionInterface - include ::PackageProtections::RubocopProtectionInterface - - sig { override.returns(::String) } - def cop_name; end - - sig { params(package: ::PackageProtections::ProtectedPackage).returns(T::Hash[T.untyped, T.untyped]) } - def custom_cop_config(package); end - - sig { override.returns(::String) } - def humanized_protection_description; end - - sig { override.returns(::String) } - def humanized_protection_name; end - - sig { override.returns(::String) } - def identifier; end - - sig { override.returns(T::Array[::String]) } - def included_globs_for_pack; end - - sig { override.params(file: ::String).returns(::String) } - def message_for_fail_on_any(file); end - - def on_new_investigation; end - - sig do - override - .params( - behavior: ::PackageProtections::ViolationBehavior, - package: ::ParsePackwerk::Package - ).returns(T.nilable(::String)) - end - def unmet_preconditions_for_behavior(behavior, package); end - - private - - def get_actual_namespace(remaining_file_path, relative_filepath, package_name); end - def get_allowed_namespaces(package_name); end - def root_pathname; end -end - -RuboCop::Cop::PackageProtections::NamespacedUnderPackageName::IDENTIFIER = T.let(T.unsafe(nil), String) - -class RuboCop::Cop::PackageProtections::TypedPublicApi < ::RuboCop::Cop::Sorbet::StrictSigil - include ::PackageProtections::ProtectionInterface - include ::PackageProtections::RubocopProtectionInterface - - sig { override.returns(::String) } - def cop_name; end - - sig { override.returns(::String) } - def humanized_protection_description; end - - sig { override.returns(::String) } - def humanized_protection_name; end - - sig { override.returns(::String) } - def identifier; end - - sig { override.returns(T::Array[::String]) } - def included_globs_for_pack; end - - sig { override.params(file: ::String).returns(::String) } - def message_for_fail_on_any(file); end - - sig do - override - .params( - behavior: ::PackageProtections::ViolationBehavior, - package: ::ParsePackwerk::Package - ).returns(T.nilable(::String)) - end - def unmet_preconditions_for_behavior(behavior, package); end -end - RuboCop::Cop::PackageProtections::TypedPublicApi::IDENTIFIER = T.let(T.unsafe(nil), String) -RuboCop::NodePattern = RuboCop::AST::NodePattern -RuboCop::ProcessedSource = RuboCop::AST::ProcessedSource -RuboCop::Token = RuboCop::AST::Token