From e6b702887dae4de4ce6b403c242e9378dc2d814c Mon Sep 17 00:00:00 2001 From: Andrey Novikov Date: Mon, 2 Mar 2026 23:52:30 +0900 Subject: [PATCH 1/4] Move both controller and request specs on controller move --- lib/packs/private.rb | 4 +- lib/packs/private/file_move_operation.rb | 50 +++++++++++++++++++---- spec/packs_spec.rb | 51 ++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 9 deletions(-) diff --git a/lib/packs/private.rb b/lib/packs/private.rb index 0e63de6..bd2f08e 100644 --- a/lib/packs/private.rb +++ b/lib/packs/private.rb @@ -131,7 +131,7 @@ def self.move_to_pack!(pack_name:, paths_relative_to_root:, per_file_processors: ) [ file_move_operation, - file_move_operation.spec_file_move_operation + *file_move_operation.spec_file_move_operations ] end file_move_operations.each do |file_move_operation| @@ -358,7 +358,7 @@ def self.make_public!(paths_relative_to_root:, per_file_processors:) [ file_move_operation, - file_move_operation.spec_file_move_operation + *file_move_operation.spec_file_move_operations ] end diff --git a/lib/packs/private/file_move_operation.rb b/lib/packs/private/file_move_operation.rb index 2b5439e..356b452 100644 --- a/lib/packs/private/file_move_operation.rb +++ b/lib/packs/private/file_move_operation.rb @@ -51,8 +51,8 @@ def self.destination_pathname_for_new_public_api(origin_pathname) ).cleanpath end - sig { returns(FileMoveOperation) } - def spec_file_move_operation + sig { returns(T::Array[FileMoveOperation]) } + def spec_file_move_operations path_parts = filepath_without_pack_name.split('/') folder = T.must(path_parts[0]) file_extension = T.must(filepath_without_pack_name.split('.').last) @@ -67,11 +67,20 @@ def spec_file_move_operation new_destination_pathname = spec_pathname_for_non_app(destination_pathname, file_extension, folder) end - FileMoveOperation.new( - origin_pathname: new_origin_pathname, - destination_pathname: new_destination_pathname, - destination_pack: destination_pack - ) + ops = [ + FileMoveOperation.new( + origin_pathname: new_origin_pathname, + destination_pathname: new_destination_pathname, + destination_pack: destination_pack + ) + ] + + # For controllers, also include the request spec (spec/requests/_spec.rb) if it exists. + # Request specs are named without "controller" suffix (e.g. my_controller -> my_spec) + request_spec_op = request_spec_file_move_operation + ops << request_spec_op if request_spec_op + + ops end sig { params(filepath: Pathname, pack: T.nilable(Packs::Pack)).returns(String) } @@ -106,6 +115,33 @@ def spec_pathname_for_non_app(pathname, file_extension, folder) .sub(".#{file_extension}", '_spec.rb') end + # Maps app/controllers/*_controller.rb to spec/requests/*_spec.rb. + # Returns nil if the path is not a controller path. + sig { params(pathname: Pathname).returns(T.nilable(Pathname)) } + def pathname_to_request_spec(pathname) + return nil unless pathname.to_s.end_with?('_controller.rb') + + pathname + .sub('app/controllers/', 'spec/requests/') + .sub(/_controller\.rb\z/, '_spec.rb') + .cleanpath + end + + sig { returns(T.nilable(FileMoveOperation)) } + def request_spec_file_move_operation + request_spec_origin = pathname_to_request_spec(origin_pathname) + return if request_spec_origin.nil? || !request_spec_origin.exist? + + request_spec_destination = pathname_to_request_spec(destination_pathname) + return if request_spec_destination.nil? + + FileMoveOperation.new( + origin_pathname: request_spec_origin, + destination_pathname: request_spec_destination, + destination_pack: destination_pack + ) + end + sig { params(path: Pathname).returns(FileMoveOperation) } def relative_to(path) FileMoveOperation.new( diff --git a/spec/packs_spec.rb b/spec/packs_spec.rb index a3d1060..2c2c370 100644 --- a/spec/packs_spec.rb +++ b/spec/packs_spec.rb @@ -405,6 +405,57 @@ def write_codeownership_config ]) end + it 'moves the request spec when moving a controller if spec/requests/_spec.rb exists (name = controller minus "controller")' do + write_file('app/controllers/my_controller.rb') + write_file('spec/requests/my_spec.rb') + write_package_yml('packs/my_pack') + Packs.move_to_pack!( + pack_name: 'packs/my_pack', + paths_relative_to_root: ['app/controllers/my_controller.rb'] + ) + + expect_files_to_not_exist([ + 'app/controllers/my_controller.rb', + 'spec/requests/my_spec.rb' + ]) + expect_files_to_exist([ + 'packs/my_pack/app/controllers/my_controller.rb', + 'packs/my_pack/spec/requests/my_spec.rb' + ]) + end + + it 'moves namespaced request spec when moving a namespaced controller' do + write_file('app/controllers/api/v1/users_controller.rb') + write_file('spec/requests/api/v1/users_spec.rb') + write_package_yml('packs/my_pack') + Packs.move_to_pack!( + pack_name: 'packs/my_pack', + paths_relative_to_root: ['app/controllers/api/v1/users_controller.rb'] + ) + + expect_files_to_not_exist([ + 'app/controllers/api/v1/users_controller.rb', + 'spec/requests/api/v1/users_spec.rb' + ]) + expect_files_to_exist([ + 'packs/my_pack/app/controllers/api/v1/users_controller.rb', + 'packs/my_pack/spec/requests/api/v1/users_spec.rb' + ]) + end + + it 'does not move a request spec when no file exists at spec/requests/_spec.rb' do + write_file('app/controllers/my_controller.rb') + # no spec/requests/my_spec.rb + write_package_yml('packs/my_pack') + Packs.move_to_pack!( + pack_name: 'packs/my_pack', + paths_relative_to_root: ['app/controllers/my_controller.rb'] + ) + + expect_files_to_not_exist(['app/controllers/my_controller.rb']) + expect_files_to_exist(['packs/my_pack/app/controllers/my_controller.rb']) + end + it 'can move files from non-pack packages into a pack' do target_pack = 'packs/animals' file_to_move = 'lib/tasks/donkey.rake' From 45b8f238ee9fea751a0cfe8392e8812dda84e980 Mon Sep 17 00:00:00 2001 From: Andrey Novikov Date: Thu, 12 Mar 2026 20:47:58 +0900 Subject: [PATCH 2/4] Bump version --- Gemfile.lock | 2 +- packs.gemspec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index d4097c4..babdb58 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - packs (0.2.0) + packs (0.3.0) bigdecimal code_ownership (>= 1.33.0) packs-specification diff --git a/packs.gemspec b/packs.gemspec index fec2a35..bd45953 100644 --- a/packs.gemspec +++ b/packs.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |spec| spec.name = 'packs' - spec.version = '0.2.0' + spec.version = '0.3.0' spec.authors = ['Gusto Engineers'] spec.email = ['dev@gusto.com'] From e7d2358139fd8b0e452433409d1edfa2fb1f9444 Mon Sep 17 00:00:00 2001 From: Andrey Novikov Date: Thu, 12 Mar 2026 22:14:27 +0900 Subject: [PATCH 3/4] Add spec covering `make_public!` for request specs --- lib/packs/private/file_move_operation.rb | 2 ++ spec/packs_spec.rb | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/lib/packs/private/file_move_operation.rb b/lib/packs/private/file_move_operation.rb index d1de9c2..6e3fd96 100644 --- a/lib/packs/private/file_move_operation.rb +++ b/lib/packs/private/file_move_operation.rb @@ -123,6 +123,8 @@ def pathname_to_request_spec(pathname) pathname .sub('app/controllers/', 'spec/requests/') + .sub('/app/', '/spec/') # if destionation doesn't have controller subdirectory + .sub(%r(\Aapp/), 'spec/') .sub(/_controller\.rb\z/, '_spec.rb') .cleanpath end diff --git a/spec/packs_spec.rb b/spec/packs_spec.rb index 5b546b6..0cb564a 100644 --- a/spec/packs_spec.rb +++ b/spec/packs_spec.rb @@ -1041,6 +1041,29 @@ def write_codeownership_config ) end + it 'makes a controller and its matching request spec public when spec/requests/_spec.rb exists (name = controller minus "controller")' do + write_file('app/controllers/my_controller.rb') + write_file('spec/requests/my_spec.rb') + + Packs.make_public!( + paths_relative_to_root: ['app/controllers/my_controller.rb'] + ) + + expect_files_to_not_exist( + [ + 'app/controllers/my_controller.rb', + 'spec/requests/my_spec.rb', + ] + ) + + expect_files_to_exist( + [ + 'app/public/my_controller.rb', + 'spec/public/my_spec.rb', + ] + ) + end + it 'can make directories in the monolith and their specs public' do write_file('app/services/fish_like/small_ones/goldfish.rb') write_file('app/services/fish_like/small_ones/seahorse.rb') From 8558e2ba0e95b3dc7cb3269dbe8539bdb2039fc1 Mon Sep 17 00:00:00 2001 From: Andrey Novikov Date: Thu, 12 Mar 2026 22:30:13 +0900 Subject: [PATCH 4/4] fix: rubocop --- lib/packs/private/file_move_operation.rb | 2 +- spec/packs_spec.rb | 40 ++++++++++++++---------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/lib/packs/private/file_move_operation.rb b/lib/packs/private/file_move_operation.rb index 6e3fd96..e3e3822 100644 --- a/lib/packs/private/file_move_operation.rb +++ b/lib/packs/private/file_move_operation.rb @@ -72,7 +72,7 @@ def spec_file_move_operations origin_pathname: new_origin_pathname, destination_pathname: new_destination_pathname, destination_pack: destination_pack - ) + ), ] # For controllers, also include the request spec (spec/requests/_spec.rb) if it exists. diff --git a/spec/packs_spec.rb b/spec/packs_spec.rb index 0cb564a..60d41a7 100644 --- a/spec/packs_spec.rb +++ b/spec/packs_spec.rb @@ -418,14 +418,18 @@ def write_codeownership_config paths_relative_to_root: ['app/controllers/my_controller.rb'] ) - expect_files_to_not_exist([ - 'app/controllers/my_controller.rb', - 'spec/requests/my_spec.rb' - ]) - expect_files_to_exist([ - 'packs/my_pack/app/controllers/my_controller.rb', - 'packs/my_pack/spec/requests/my_spec.rb' - ]) + expect_files_to_not_exist( + [ + 'app/controllers/my_controller.rb', + 'spec/requests/my_spec.rb', + ] + ) + expect_files_to_exist( + [ + 'packs/my_pack/app/controllers/my_controller.rb', + 'packs/my_pack/spec/requests/my_spec.rb', + ] + ) end it 'moves namespaced request spec when moving a namespaced controller' do @@ -437,14 +441,18 @@ def write_codeownership_config paths_relative_to_root: ['app/controllers/api/v1/users_controller.rb'] ) - expect_files_to_not_exist([ - 'app/controllers/api/v1/users_controller.rb', - 'spec/requests/api/v1/users_spec.rb' - ]) - expect_files_to_exist([ - 'packs/my_pack/app/controllers/api/v1/users_controller.rb', - 'packs/my_pack/spec/requests/api/v1/users_spec.rb' - ]) + expect_files_to_not_exist( + [ + 'app/controllers/api/v1/users_controller.rb', + 'spec/requests/api/v1/users_spec.rb', + ] + ) + expect_files_to_exist( + [ + 'packs/my_pack/app/controllers/api/v1/users_controller.rb', + 'packs/my_pack/spec/requests/api/v1/users_spec.rb', + ] + ) end it 'does not move a request spec when no file exists at spec/requests/_spec.rb' do