Tidy up references to code_ownership configuration in test (#51)

* Tidy up test references to config/code_ownership.yml * Ignore unowned globs earlier * bump version * small nit
rubyatscale · Apr 10, 2023 · ce31f6a · ce31f6a
1 parent 5b5fb1c
commit ce31f6a
Show file tree

Hide file tree

Showing 16 changed files with 41 additions and 78 deletions.
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    code_ownership (1.32.5)
+    code_ownership (1.32.6)
       code_teams (~> 1.0)
       packs
       sorbet-runtime

diff --git a/code_ownership.gemspec b/code_ownership.gemspec
@@ -1,6 +1,6 @@
 Gem::Specification.new do |spec|
   spec.name          = "code_ownership"
-  spec.version       = '1.32.5'
+  spec.version       = '1.32.6'
   spec.authors       = ['Gusto Engineers']
   spec.email         = ['[email protected]']
   spec.summary       = 'A gem to help engineering teams declare ownership of code'

diff --git a/lib/code_ownership/private.rb b/lib/code_ownership/private.rb
@@ -76,7 +76,7 @@ def self.path_from_klass(klass)
     sig { returns(T::Array[String]) }
     def self.tracked_files
       @tracked_files ||= T.let(@tracked_files, T.nilable(T::Array[String]))
-      @tracked_files ||= Dir.glob(configuration.owned_globs)
+      @tracked_files ||= Dir.glob(configuration.owned_globs) - Dir.glob(configuration.unowned_globs)
     end
 
     sig { params(team_name: String, location_of_reference: String).returns(CodeTeams::Team) }

diff --git a/lib/code_ownership/private/validations/files_have_owners.rb b/lib/code_ownership/private/validations/files_have_owners.rb
@@ -10,22 +10,19 @@ class FilesHaveOwners
 
         sig { override.params(files: T::Array[String], autocorrect: T::Boolean, stage_changes: T::Boolean).returns(T::Array[String]) }
         def validation_errors(files:, autocorrect: true, stage_changes: true)
-          allow_list = Dir.glob(Private.configuration.unowned_globs)
           files_by_mapper = Private.glob_cache.files_by_mapper
 
           files_not_mapped_at_all = files.select do |file|
             files_by_mapper.fetch(file, []).count == 0
           end
 
-          files_without_owners = files_not_mapped_at_all - allow_list
-
           errors = T.let([], T::Array[String])
 
-          if files_without_owners.any?
+          if files_not_mapped_at_all.any?
             errors << <<~MSG
               Some files are missing ownership:
 
-              #{files_without_owners.map { |file| "- #{file}" }.join("\n")}
+              #{files_not_mapped_at_all.map { |file| "- #{file}" }.join("\n")}
             MSG
           end
 

diff --git a/spec/lib/code_ownership/cli_spec.rb b/spec/lib/code_ownership/cli_spec.rb
@@ -5,11 +5,7 @@
     let(:argv) { ['validate'] }
 
     before do
-      write_file('config/code_ownership.yml', <<~YML)
-        owned_globs:
-          - 'app/**/*.rb'
-      YML
-
+      write_configuration
       write_file('app/services/my_file.rb')
       write_file('frontend/javascripts/my_file.jsx')
     end
@@ -19,7 +15,7 @@
         expect(CodeOwnership).to receive(:validate!) do |args| # rubocop:disable RSpec/MessageSpies
           expect(args[:autocorrect]).to eq true
           expect(args[:stage_changes]).to eq true
-          expect(args[:files]).to match_array(['app/services/my_file.rb'])
+          expect(args[:files]).to match_array(["app/services/my_file.rb", "frontend/javascripts/my_file.jsx"])
         end
         subject
       end
@@ -29,10 +25,7 @@
 
   describe 'for_file' do
     before do
-      write_file('config/code_ownership.yml', <<~YML)
-        owned_globs:
-          - 'app/**/*.rb'
-      YML
+      write_configuration
 
       write_file('app/services/my_file.rb')
       write_file('config/teams/my_team.yml', <<~YML)

diff --git a/spec/lib/code_ownership/private/extension_loader_spec.rb b/spec/lib/code_ownership/private/extension_loader_spec.rb
@@ -4,12 +4,7 @@ module CodeOwnership
     let(:codeowners_validation) { Private::Validations::GithubCodeownersUpToDate }
 
     before do
-      write_file('config/code_ownership.yml', <<~YML)
-        owned_globs:
-          - app/**/*.rb
-        require:
-          - ./lib/my_extension.rb
-      YML
+      write_configuration('require' => ['./lib/my_extension.rb'])
 
       write_file('config/teams/bar.yml', <<~CONTENTS)
         name: Bar
@@ -31,7 +26,7 @@ class MyExtension
               returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
           end
           def map_files_to_owners(files) # rubocop:disable Lint/UnusedMethodArgument
-            files.map{|f| [f, CodeTeams.all.last]}.to_h
+            files.select{|f| Pathname.new(f).extname == '.rb'}.map{|f| [f, CodeTeams.all.last]}.to_h
           end
 
           sig do
@@ -46,7 +41,7 @@ def map_file_to_owner(file)
             override.returns(T::Hash[String, T.nilable(::CodeTeams::Team)])
           end
           def codeowners_lines_to_owners
-            Dir.glob('**/*.*').map{|f| [f, CodeTeams.all.last]}.to_h
+            Dir.glob('**/*.rb').map{|f| [f, CodeTeams.all.last]}.to_h
           end
 
           sig { override.returns(String) }
@@ -105,8 +100,6 @@ def bust_caches!
 
           # My special extension
           /app/services/my_ownable_file.rb @org/my-team
-          /config/code_ownership.yml @org/my-team
-          /config/teams/bar.yml @org/my-team
           /lib/my_extension.rb @org/my-team
         EXPECTED
       end

diff --git a/spec/lib/code_ownership/private/ownership_mappers/file_annotations_spec.rb b/spec/lib/code_ownership/private/ownership_mappers/file_annotations_spec.rb
@@ -2,7 +2,7 @@ module CodeOwnership
   RSpec.describe Private::OwnershipMappers::FileAnnotations do
     describe '.for_team' do
       before do
-        create_configuration
+        write_configuration
         write_file('config/teams/bar.yml', <<~CONTENTS)
           name: Bar
         CONTENTS
@@ -36,7 +36,7 @@ module CodeOwnership
     describe '.for_file' do
       context 'ruby owned file' do
         before do
-          create_configuration
+          write_configuration
           write_file('config/teams/bar.yml', <<~CONTENTS)
             name: Bar
           CONTENTS
@@ -53,7 +53,7 @@ module CodeOwnership
 
       context 'javascript owned file' do
         before do
-          create_configuration
+          write_configuration
           write_file('config/teams/bar.yml', <<~CONTENTS)
             name: Bar
           CONTENTS
@@ -80,7 +80,7 @@ module CodeOwnership
         write_file('config/teams/foo.yml', <<~CONTENTS)
           name: Foo
         CONTENTS
-        create_minimal_configuration
+        write_configuration
       end
 
       context 'ruby file has no annotation' do

diff --git a/spec/lib/code_ownership/private/ownership_mappers/js_package_ownership_spec.rb b/spec/lib/code_ownership/private/ownership_mappers/js_package_ownership_spec.rb
@@ -3,7 +3,7 @@ module CodeOwnership
     describe 'CodeOwnership.validate!' do
       context 'application has invalid JSON in package' do
         before do
-          write_file('config/code_ownership.yml', {}.to_yaml)
+          write_configuration
 
           write_file('frontend/javascripts/my_package/package.json', <<~CONTENTS)
             { syntax error!!!
@@ -27,7 +27,7 @@ module CodeOwnership
 
     describe 'CodeOwnershp.for_file' do
       before do
-        create_configuration
+        write_configuration
 
         write_file('frontend/javascripts/packages/my_other_package/package.json', <<~CONTENTS)
           {

diff --git a/spec/lib/code_ownership/private/ownership_mappers/package_ownership_spec.rb b/spec/lib/code_ownership/private/ownership_mappers/package_ownership_spec.rb
@@ -1,7 +1,7 @@
 module CodeOwnership
   RSpec.describe Private::OwnershipMappers::PackageOwnership do
     before do
-      create_configuration
+      write_configuration
       write_file('config/teams/bar.yml', <<~CONTENTS)
         name: Bar
       CONTENTS

diff --git a/spec/lib/code_ownership/private/ownership_mappers/team_globs_spec.rb b/spec/lib/code_ownership/private/ownership_mappers/team_globs_spec.rb
@@ -1,6 +1,6 @@
 module CodeOwnership
   RSpec.describe Private::OwnershipMappers::TeamGlobs do
-    before { create_configuration }
+    before { write_configuration }
 
     describe 'CodeOwnership.for_file' do
       before do
@@ -27,10 +27,7 @@ module CodeOwnership
     describe 'CodeOwnership.validate!' do
       context 'two teams own the same exact glob' do
         before do
-          write_file('config/code_ownership.yml', <<~YML)
-            owned_globs:
-              - '{app,components,config,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx}'
-          YML
+          write_configuration
 
           write_file('packs/my_pack/owned_file.rb')
           write_file('frontend/javascripts/blah/my_file.rb')

diff --git a/spec/lib/code_ownership/private/ownership_mappers/team_yml_ownership_spec.rb b/spec/lib/code_ownership/private/ownership_mappers/team_yml_ownership_spec.rb
@@ -1,7 +1,7 @@
 module CodeOwnership
   RSpec.describe Private::OwnershipMappers::TeamYmlOwnership do
     before do
-      create_configuration
+      write_configuration
       write_file('config/teams/bar.yml', <<~CONTENTS)
         name: Bar
       CONTENTS

diff --git a/spec/lib/code_ownership/private/validations/files_have_owners_spec.rb b/spec/lib/code_ownership/private/validations/files_have_owners_spec.rb
@@ -8,7 +8,7 @@ module CodeOwnership
           write_file('Gemfile', <<~CONTENTS)
           CONTENTS
 
-          create_minimal_configuration
+          write_configuration
         end
 
         it 'does not raise an error' do
@@ -24,7 +24,7 @@ module CodeOwnership
 
         context 'the file is not in unowned_globs' do
           before do
-            create_minimal_configuration
+            write_configuration
           end
 
           it 'lets the user know the file must have ownership' do
@@ -50,12 +50,7 @@ module CodeOwnership
 
         context 'that file is in unowned_globs' do
           before do
-            write_file('config/code_ownership.yml', <<~YML)
-              owned_globs:
-                - 'app/**/*.rb'
-              unowned_globs:
-                - app/missing_ownership.rb
-            YML
+            write_configuration('unowned_globs' => ['app/missing_ownership.rb', 'config/code_ownership.yml'])
           end
 
           it 'does not raise an error' do

diff --git a/spec/lib/code_ownership/private/validations/files_have_unique_owners_spec.rb b/spec/lib/code_ownership/private/validations/files_have_unique_owners_spec.rb
@@ -3,10 +3,7 @@ module CodeOwnership
     describe 'CodeOwnership.validate!' do
       context 'a file in owned_globs has ownership defined in multiple ways' do
         before do
-          write_file('config/code_ownership.yml', <<~YML)
-            owned_globs:
-              - '{app,components,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx,json,yml}'
-          YML
+          write_configuration
 
           write_file('app/services/some_other_file.rb', <<~YML)
             # @team Bar

diff --git a/spec/lib/code_ownership/private/validations/github_codeowners_up_to_date_spec.rb b/spec/lib/code_ownership/private/validations/github_codeowners_up_to_date_spec.rb
@@ -5,7 +5,7 @@ module CodeOwnership
 
       context 'run with autocorrect' do
         before do
-          create_minimal_configuration
+          write_configuration
         end
 
         context 'in an empty application' do
@@ -157,7 +157,7 @@ module CodeOwnership
 
         context 'run without staging changes' do
           before do
-            create_minimal_configuration
+            write_configuration
           end
 
           it 'does not stage the changes to the codeowners file' do
@@ -180,7 +180,7 @@ module CodeOwnership
 
       context 'run without autocorrect' do
         before do
-          create_minimal_configuration
+          write_configuration
         end
 
         context 'in an empty application' do
@@ -469,11 +469,7 @@ module CodeOwnership
 
       context 'code_ownership.yml has skip_codeowners_validation set' do
         before do
-          write_file('config/code_ownership.yml', <<~YML)
-            owned_globs:
-              - app/**/*.rb
-            skip_codeowners_validation: true
-          YML
+          write_configuration('skip_codeowners_validation' => true)
         end
 
         it 'skips validating the codeowners file' do

diff --git a/spec/lib/code_ownership_spec.rb b/spec/lib/code_ownership_spec.rb
@@ -7,7 +7,7 @@
           name: Bar
         CONTENTS
 
-        create_minimal_configuration
+        write_configuration
       end
 
       context 'invalid team in a file annotation' do
@@ -88,7 +88,7 @@
   describe '.for_backtrace' do
     before do
       create_files_with_defined_classes
-      create_minimal_configuration
+      write_configuration
     end
 
     context 'excluded_teams is not passed in as an input parameter' do
@@ -111,7 +111,7 @@
 
   describe '.first_owned_file_for_backtrace' do
     before do
-      create_minimal_configuration
+      write_configuration
       create_files_with_defined_classes
     end
 
@@ -145,7 +145,7 @@
   describe '.for_class' do
     before do
       create_files_with_defined_classes
-      create_minimal_configuration
+      write_configuration
     end
 
     it 'can find the right owner for a class' do

diff --git a/spec/support/application_fixtures.rb b/spec/support/application_fixtures.rb
@@ -1,22 +1,18 @@
 RSpec.shared_context 'application fixtures' do
   let(:codeowners_path) { Pathname.pwd.join('.github/CODEOWNERS') }
 
-  let(:create_configuration) do
-    write_file('config/code_ownership.yml', <<~YML)
-      owned_globs:
-        - '{app,components,config,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx}'
-    YML
-  end
 
-  let(:create_minimal_configuration) do
-    write_file('config/code_ownership.yml', <<~YML)
-      owned_globs:
-        - app/**/*.{rb,jsx}
-    YML
+  def write_configuration(owned_globs: nil, **kwargs)
+    owned_globs ||= ['{app,components,config,frontend,lib,packs,spec}/**/*.{rb,rake,js,jsx,ts,tsx,json,yml}']
+    config = {
+      'owned_globs' => owned_globs,
+      'unowned_globs' => ['config/code_ownership.yml']
+    }.merge(kwargs)
+    write_file('config/code_ownership.yml', config.to_yaml)
   end
 
   let(:create_non_empty_application) do
-    create_configuration
+    write_configuration
 
     write_file('frontend/javascripts/packages/my_package/owned_file.jsx', <<~CONTENTS)
       // @team Bar
@@ -26,7 +22,6 @@
       # @team Bar
     CONTENTS
 
-
     write_file('frontend/javascripts/packages/my_other_package/package.json', <<~CONTENTS)
       {
         "name": "@gusto/my_package",