diff --git a/lib/library_version_analysis/gemfile.rb b/lib/library_version_analysis/gemfile.rb index 3845420..cf33ce1 100644 --- a/lib/library_version_analysis/gemfile.rb +++ b/lib/library_version_analysis/gemfile.rb @@ -196,6 +196,17 @@ def add_ownership_from_gemspecs(parsed_results) default_owner = LibraryVersionAnalysis::Configuration.get(:default_owner_name) Dir.glob(File.join("gems", "**", "*.gemspec")) do |gemspec_file| + # from_codeowners: false resolves ownership from the team owned_globs config + # rather than the generated .github/CODEOWNERS file. Repos that hand-curate + # CODEOWNERS (skip_codeowners_validation) otherwise return nil for every gem. + team = CodeOwnership.for_file(gemspec_file, from_codeowners: false) + if team.nil? + warn "No code owner for #{gemspec_file}; skipping gemspec ownership assignment" + next + end + + group = team.raw_hash["group"] + File.foreach(gemspec_file) do |line| scan_result = line.scan(/spec.add_.*dependency\s*"(\S*)"/) @@ -204,8 +215,6 @@ def add_ownership_from_gemspecs(parsed_results) library = scan_result[0][0] next if(parsed_results.has_key?(library) && parsed_results[library].owner != default_owner) - team = CodeOwnership.for_file(gemspec_file) - group = team.raw_hash["group"] parsed_results[library]&.owner = group.start_with?(":") ? group : ":#{group}" end end diff --git a/spec/gemfile_spec.rb b/spec/gemfile_spec.rb index 8ea9d7d..e04b6a6 100644 --- a/spec/gemfile_spec.rb +++ b/spec/gemfile_spec.rb @@ -130,6 +130,58 @@ def do_compare(result:, owner:, current_version:, latest_version:, major:, minor allow(LibraryVersionAnalysis::CheckVersionStatus).to receive(:legacy?).and_return(false) end + describe "#add_ownership_from_gemspecs" do + let(:analyzer) { LibraryVersionAnalysis::Gemfile.new("test") } + let(:gemspec_contents) do + <<~DOC + spec.add_dependency "aasm" + spec.add_development_dependency "packwerk" + DOC + end + + before(:each) do + allow(LibraryVersionAnalysis::Configuration).to receive(:get).and_call_original + allow(LibraryVersionAnalysis::Configuration).to receive(:get) + .with(:default_owner_name).and_return(:unspecified) + allow(Dir).to receive(:glob).and_call_original + allow(Dir).to receive(:glob).with(File.join("gems", "**", "*.gemspec")) + .and_yield("gems/example/example.gemspec") + allow(File).to receive(:foreach).and_call_original + allow(File).to receive(:foreach).with("gems/example/example.gemspec") + .and_yield('spec.add_dependency "aasm"') + .and_yield('spec.add_development_dependency "packwerk"') + end + + it "assigns the gemspec owner's group to each dependency" do + team = double("team", raw_hash: { "group" => "self_serve" }) + allow(CodeOwnership).to receive(:for_file) + .with("gems/example/example.gemspec", from_codeowners: false).and_return(team) + + parsed_results = { + "aasm" => LibraryVersionAnalysis::Versionline.new(owner: :unspecified), + "packwerk" => LibraryVersionAnalysis::Versionline.new(owner: :unspecified), + } + + analyzer.send(:add_ownership_from_gemspecs, parsed_results) + + expect(parsed_results["aasm"].owner).to eq(":self_serve") + expect(parsed_results["packwerk"].owner).to eq(":self_serve") + end + + it "skips the gemspec without raising when it has no code owner" do + allow(CodeOwnership).to receive(:for_file) + .with("gems/example/example.gemspec", from_codeowners: false).and_return(nil) + + parsed_results = { + "aasm" => LibraryVersionAnalysis::Versionline.new(owner: :unspecified), + } + + expect { analyzer.send(:add_ownership_from_gemspecs, parsed_results) } + .not_to raise_error + expect(parsed_results["aasm"].owner).to eq(:unspecified) + end + end + describe "#add_dependency_graph" do it "should reverse simple chain" do c = SpecSetStruct.new(name: "c")