Skip to content

Commit

Permalink
Warn and skip empty groups, Close PRs for empty groups
Browse files Browse the repository at this point in the history
  • Loading branch information
brrygrdn committed Jul 11, 2023
1 parent 67d8150 commit 779657e
Show file tree
Hide file tree
Showing 11 changed files with 228 additions and 7 deletions.
8 changes: 8 additions & 0 deletions common/lib/dependabot/dependency_group.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require "wildcard_matcher"
require "yaml"

module Dependabot
class DependencyGroup
Expand All @@ -24,5 +25,12 @@ def contains?(dependency)
def to_h
{ "name" => name }
end

# Provides a debug utility to view the group as it appears in the config file.
def to_config_yaml
{
"groups" => { name => rules }
}.to_yaml.delete_prefix("---\n")
end
end
end
21 changes: 21 additions & 0 deletions common/spec/dependabot/dependency_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,25 @@
end
end
end

describe "#to_config_yaml" do
let(:rules) do
{
"patterns" => ["test-*", "nothing-matches-this"],
"exclude-patterns" => ["*-2"]
}
end

it "renders the group to match our configuration file" do
expect(dependency_group.to_config_yaml).to eql(<<~YAML)
groups:
test_group:
patterns:
- test-*
- nothing-matches-this
exclude-patterns:
- "*-2"
YAML
end
end
end
2 changes: 1 addition & 1 deletion updater/lib/dependabot/dependency_group_engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def warn_misconfigured_groups(groups)
This can happen if:
- the group's 'pattern' rules are mispelled
- your configuration's 'allow' rules do not permit any of the dependencies that match the group
- your configuration's 'ignore' rules exclude all of the dependencies that match the group
- the dependencies that match the group rules have been removed from your project
WARN
end
end
Expand Down
14 changes: 14 additions & 0 deletions updater/lib/dependabot/updater/group_update_creation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,20 @@ def peer_dependency_should_update_instead?(dependency_name, dependency_files, up
end
end

def warn_group_is_empty(group)
Dependabot.logger.warn(
"Skipping update group for '#{group.name}' as it does not match any allowed dependencies."
)

return unless Dependabot.logger.debug?

Dependabot.logger.debug(<<~DEBUG.chomp)
The configuration for this group is:
#{group.to_config_yaml}
DEBUG
end

def prepare_workspace
return unless job.clone? && job.repo_contents_path

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:, group:)
end

def perform
warn_group_is_empty(group) and return if group.dependencies.empty?

Dependabot.logger.info("Starting update group for '#{group.name}'")

dependency_change = compile_all_dependency_changes_for(group)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,29 @@ def perform
)

service.capture_exception(
error: DependabotError.new("Attempted to update a missing group."),
error: DependabotError.new("Attempted to refresh a missing group."),
job: job
)
return
end

Dependabot.logger.info("Starting PR update job for #{job.source.repo}")
Dependabot.logger.info("Updating the '#{dependency_snapshot.job_group.name}' group")

dependency_change = compile_all_dependency_changes_for(dependency_snapshot.job_group)
if dependency_snapshot.job_group.dependencies.empty?
# If the group is empty that means any Dependencies that did match this group
# have been removed from the project or are no longer allowed by the config.
#
# Let's warn that the group is empty and then signal the PR should be closed
# so users are informed this group is no longer actionable by Dependabot.
warn_group_is_empty(dependency_snapshot.job_group)
close_pull_request(reason: :dependency_group_empty)
else
Dependabot.logger.info("Updating the '#{dependency_snapshot.job_group.name}' group")

dependency_change = compile_all_dependency_changes_for(dependency_snapshot.job_group)

upsert_pull_request_with_error_handling(dependency_change)
upsert_pull_request_with_error_handling(dependency_change)
end
end

private
Expand Down
4 changes: 2 additions & 2 deletions updater/spec/dependabot/dependency_group_engine_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@
This can happen if:
- the group's 'pattern' rules are mispelled
- your configuration's 'allow' rules do not permit any of the dependencies that match the group
- your configuration's 'ignore' rules exclude all of the dependencies that match the group
- the dependencies that match the group rules have been removed from your project
WARN
)

Expand All @@ -180,7 +180,7 @@
This can happen if:
- the group's 'pattern' rules are mispelled
- your configuration's 'allow' rules do not permit any of the dependencies that match the group
- your configuration's 'ignore' rules exclude all of the dependencies that match the group
- the dependencies that match the group rules have been removed from your project
WARN
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,61 @@
end
end

context "when the snapshot has a group that does not match any dependencies" do
let(:job_definition) do
job_definition_fixture("bundler/version_updates/group_update_all_empty_group")
end

let(:dependency_files) do
original_bundler_files
end

before do
stub_rubygems_calls
end

it "warns the group is empty" do
allow(Dependabot.logger).to receive(:warn) # permit any other warning calls that happen
expect(Dependabot.logger).to receive(:warn).with(
"Skipping update group for 'everything-everywhere-all-at-once' as it does not match any allowed dependencies."
)

# Expect us to handle the dependencies individually instead
expect(Dependabot::Updater::Operations::UpdateAllVersions).to receive(:new).and_return(
instance_double(Dependabot::Updater::Operations::UpdateAllVersions, perform: nil)
)

group_update_all.perform
end

context "when debug is enabled" do
before do
allow(Dependabot.logger).to receive(:debug)
allow(Dependabot.logger).to receive(:debug?).and_return(true)
end

it "renders the group configuration for us" do
expect(Dependabot.logger).to receive(:debug).with(
<<~DEBUG
The configuration for this group is:
groups:
everything-everywhere-all-at-once:
patterns:
- "*bagel"
DEBUG
)

# Expect us to handle the dependencies individually instead
expect(Dependabot::Updater::Operations::UpdateAllVersions).to receive(:new).and_return(
instance_double(Dependabot::Updater::Operations::UpdateAllVersions, perform: nil)
)

group_update_all.perform
end
end
end

context "when there are two overlapping groups" do
let(:job_definition) do
job_definition_fixture("bundler/version_updates/group_update_all_overlapping_groups")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,31 @@
group_update_all.perform
end
end

context "when the target dependency group no longer matches any dependencies in the project" do
let(:job_definition) do
job_definition_fixture("bundler/version_updates/group_update_refresh_empty_group")
end

let(:dependency_files) do
original_bundler_files
end

before do
stub_rubygems_calls
end

it "logs a warning and tells the service to close the Pull Request" do
# Our mocks will fail due to unexpected messages if any errors or PRs are dispatched

allow(Dependabot.logger).to receive(:warn)
expect(Dependabot.logger).to receive(:warn).with(
"Skipping update group for 'everything-everywhere-all-at-once' as it does not match any allowed dependencies."
)

expect(mock_service).to receive(:close_pull_request).with(["dummy-pkg-b"], :dependency_group_empty)

group_update_all.perform
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
job:
package-manager: bundler
source:
provider: github
repo: dependabot/smoke-tests
directory: "/bundler"
branch:
api-endpoint: https://api.github.com/
hostname: github.com
dependencies:
existing-pull-requests: []
updating-a-pull-request: false
lockfile-only: false
update-subdependencies: false
ignore-conditions: []
requirements-update-strategy:
allowed-updates:
- dependency-type: direct
update-type: all
credentials-metadata:
- type: git_source
host: github.com
security-advisories: []
max-updater-run-time: 2700
vendor-dependencies: false
experiments:
grouped-updates-prototype: true
reject-external-code: false
commit-message-options:
prefix:
prefix-development:
include-scope:
security-updates-only: false
dependency-groups:
- name: everything-everywhere-all-at-once
rules:
patterns:
- "*bagel"
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
job:
package-manager: bundler
source:
provider: github
repo: dependabot/smoke-tests
directory: "/bundler"
branch:
api-endpoint: https://api.github.com/
hostname: github.com
dependencies:
- dummy-pkg-b
existing-pull-requests: []
existing-group-pull-requests:
- dependency-group-name: everything-everywhere-all-at-once
dependencies:
- dependency-name: dummy-pkg-b
dependency-version: 1.2.0
updating-a-pull-request: true
lockfile-only: false
update-subdependencies: false
ignore-conditions: []
requirements-update-strategy:
allowed-updates:
- dependency-type: direct
update-type: all
credentials-metadata:
- type: git_source
host: github.com
security-advisories: []
max-updater-run-time: 2700
vendor-dependencies: false
experiments:
grouped-updates-prototype: true
reject-external-code: false
commit-message-options:
prefix:
prefix-development:
include-scope:
security-updates-only: false
dependency-groups:
- name: everything-everywhere-all-at-once
rules:
patterns:
- "*bagel"
dependency-group-to-refresh: everything-everywhere-all-at-once

0 comments on commit 779657e

Please sign in to comment.