From 82204b22f20bf78f7e541e24aa224fc5bb14fcdc Mon Sep 17 00:00:00 2001 From: Andrew Tribone Date: Tue, 12 Dec 2023 09:32:50 -1000 Subject: [PATCH] fix: go back to old way of creating paths in directory ownership --- Gemfile.lock | 2 +- code_ownership.gemspec | 2 +- .../ownership_mappers/directory_ownership.rb | 17 ++++++++++----- .../directory_ownership_spec.rb | 21 ++++++++++++++----- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 31dfeec..b13a96d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - code_ownership (1.36.0) + code_ownership (1.36.1) code_teams (~> 1.0) packs-specification sorbet-runtime (>= 0.5.10821) diff --git a/code_ownership.gemspec b/code_ownership.gemspec index 2d6b4b0..5243ed2 100644 --- a/code_ownership.gemspec +++ b/code_ownership.gemspec @@ -1,6 +1,6 @@ Gem::Specification.new do |spec| spec.name = 'code_ownership' - spec.version = '1.36.0' + spec.version = '1.36.1' spec.authors = ['Gusto Engineers'] spec.email = ['dev@gusto.com'] spec.summary = 'A gem to help engineering teams declare ownership of code' diff --git a/lib/code_ownership/private/ownership_mappers/directory_ownership.rb b/lib/code_ownership/private/ownership_mappers/directory_ownership.rb index 1ac8af9..6052a30 100644 --- a/lib/code_ownership/private/ownership_mappers/directory_ownership.rb +++ b/lib/code_ownership/private/ownership_mappers/directory_ownership.rb @@ -10,8 +10,6 @@ class DirectoryOwnership include Mapper CODEOWNERS_DIRECTORY_FILE_NAME = '.codeowner' - RELATIVE_ROOT = Pathname('.').freeze - ABSOLUTE_ROOT = Pathname('/').freeze @@directory_cache = T.let({}, T::Hash[String, T.nilable(CodeTeams::Team)]) # rubocop:disable Style/ClassVars @@ -88,16 +86,25 @@ def map_file_to_relevant_owner(file) if File.directory?(file) team = get_team_from_codeowners_file_within_directory(file_path) + return team unless team.nil? end - while team.nil? && file_path != RELATIVE_ROOT && file_path != ABSOLUTE_ROOT - file_path = file_path.parent - team = get_team_from_codeowners_file_within_directory(file_path) + path_components = file_path.each_filename.to_a + if file_path.absolute? + path_components = ['/', *path_components] + end + + (path_components.length - 1).downto(0).each do |i| + team = get_team_from_codeowners_file_within_directory( + Pathname.new(File.join(*T.unsafe(path_components[0...i]))) + ) + return team unless team.nil? end team end + sig { params(directory: Pathname).returns(T.nilable(CodeTeams::Team)) } def get_team_from_codeowners_file_within_directory(directory) potential_codeowners_file = directory.join(CODEOWNERS_DIRECTORY_FILE_NAME) diff --git a/spec/lib/code_ownership/private/ownership_mappers/directory_ownership_spec.rb b/spec/lib/code_ownership/private/ownership_mappers/directory_ownership_spec.rb index ef1009b..366bd1e 100644 --- a/spec/lib/code_ownership/private/ownership_mappers/directory_ownership_spec.rb +++ b/spec/lib/code_ownership/private/ownership_mappers/directory_ownership_spec.rb @@ -1,5 +1,5 @@ module CodeOwnership - RSpec.describe Private::OwnershipMappers::JsPackageOwnership do + RSpec.describe Private::OwnershipMappers::DirectoryOwnership do describe 'CodeOwnershp.for_file' do before do write_configuration @@ -14,17 +14,28 @@ module CodeOwnership CONTENTS end + subject { described_class.new } + + before do + subject.bust_caches! + end + it 'can find the owner of files in team-owned directory' do - expect(CodeOwnership.for_file('a/b/b_file.jsx').name).to eq 'Bar' + expect(subject.map_file_to_owner('a/b/b_file.jsx').name).to eq 'Bar' end it 'can find the owner of files in a sub-directory of a team-owned directory' do - expect(CodeOwnership.for_file('a/b/c/c_file.jsx').name).to eq 'Bar' + expect(subject.map_file_to_owner('a/b/c/c_file.jsx').name).to eq 'Bar' + end + + it 'returns null when no team is found' do + expect(subject.map_file_to_owner('tmp/tmp/foo.txt')).to be_nil + expect(subject.map_file_to_owner(Pathname.pwd.join('tmp/tmp/foo.txt').to_s)).to be_nil end it 'looks for codeowner file within directory' do - expect(CodeOwnership.for_file('a/b').name).to eq 'Bar' - expect(CodeOwnership.for_file(Pathname.pwd.join('a/b').to_s).name).to eq 'Bar' + expect(subject.map_file_to_owner('a/b').name).to eq 'Bar' + expect(subject.map_file_to_owner(Pathname.pwd.join('a/b').to_s).name).to eq 'Bar' end end end