Skip to content

Commit 281dba9

Browse files
author
Alex Evanczuk
authored
Fix bug when file annotation is removed (#63)
* Add failing test * fix failing test * bump version
1 parent 828a4cd commit 281dba9

File tree

4 files changed

+65
-4
lines changed

4 files changed

+65
-4
lines changed

Gemfile.lock

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
PATH
22
remote: .
33
specs:
4-
code_ownership (1.32.15)
4+
code_ownership (1.32.16)
55
code_teams (~> 1.0)
66
packs
77
sorbet-runtime

code_ownership.gemspec

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Gem::Specification.new do |spec|
22
spec.name = 'code_ownership'
3-
spec.version = '1.32.15'
3+
spec.version = '1.32.16'
44
spec.authors = ['Gusto Engineers']
55
spec.email = ['[email protected]']
66
spec.summary = 'A gem to help engineering teams declare ownership of code'

lib/code_ownership/private/ownership_mappers/file_annotations.rb

+11-2
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,19 @@ def globs_to_owner(files)
4747
override.params(cache: GlobsToOwningTeamMap, files: T::Array[String]).returns(GlobsToOwningTeamMap)
4848
end
4949
def update_cache(cache, files)
50-
cache.merge!(globs_to_owner(files))
50+
# We map files to nil owners so that files whose annotation have been removed will be properly
51+
# overwritten (i.e. removed) from the cache.
52+
updated_cache_for_files = globs_to_owner(files)
53+
cache.merge!(updated_cache_for_files)
54+
5155
invalid_files = cache.keys.select do |file|
52-
!Private.file_tracked?(file)
56+
# If a file is not tracked, it should be removed from the cache
57+
!Private.file_tracked?(file) ||
58+
# If a file no longer has a file annotation (i.e. `globs_to_owner` doesn't map it)
59+
# it should be removed from the cache
60+
!updated_cache_for_files.key?(file)
5361
end
62+
5463
invalid_files.each do |invalid_file|
5564
cache.delete(invalid_file)
5665
end

spec/lib/code_ownership/private/validations/github_codeowners_up_to_date_spec.rb

+52
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,58 @@ module CodeOwnership
577577
end
578578
end
579579
end
580+
581+
context 'in an application with a CODEOWNERS file that has a reference to a file that has had an annotation removed' do
582+
before do
583+
write_configuration
584+
585+
write_file('packs/my_pack/had_annotation_file.rb', <<~CONTENTS)
586+
CONTENTS
587+
588+
write_file('config/teams/bar.yml', <<~CONTENTS)
589+
name: Bar
590+
github:
591+
team: '@MyOrg/bar-team'
592+
CONTENTS
593+
end
594+
595+
it 'prints out the diff' do
596+
FileUtils.mkdir('.github')
597+
codeowners_path.write <<~CODEOWNERS
598+
# STOP! - DO NOT EDIT THIS FILE MANUALLY
599+
# This file was automatically generated by "bin/codeownership validate".
600+
#
601+
# CODEOWNERS is used for GitHub to suggest code/file owners to various GitHub
602+
# teams. This is useful when developers create Pull Requests since the
603+
# code/file owner is notified. Reference GitHub docs for more details:
604+
# https://help.github.com/en/articles/about-code-owners
605+
606+
# Annotations at the top of file
607+
/packs/my_pack/had_annotation_file.rb @MyOrg/bar-team
608+
609+
# Team YML ownership
610+
/config/teams/bar.yml @MyOrg/bar-team
611+
CODEOWNERS
612+
613+
expect_any_instance_of(codeowners_validation).to_not receive(:`) # rubocop:disable RSpec/AnyInstance
614+
expect { CodeOwnership.validate!(autocorrect: false) }.to raise_error do |e|
615+
expect(e).to be_a CodeOwnership::InvalidCodeOwnershipConfigurationError
616+
expect(e.message).to eq <<~EXPECTED.chomp
617+
Some files are missing ownership:
618+
619+
- packs/my_pack/had_annotation_file.rb
620+
621+
CODEOWNERS out of date. Run `bin/codeownership validate` to update the CODEOWNERS file
622+
623+
CODEOWNERS should not contain the following lines, but it does:
624+
- "# Annotations at the top of file"
625+
- "/packs/my_pack/had_annotation_file.rb @MyOrg/bar-team"
626+
627+
See https://github.com/rubyatscale/code_ownership#README.md for more details
628+
EXPECTED
629+
end
630+
end
631+
end
580632
end
581633

582634
context 'code_ownership.yml has skip_codeowners_validation set' do

0 commit comments

Comments
 (0)