Skip to content

Commit

Permalink
Create feature flag for Grouped security updates (#8529)
Browse files Browse the repository at this point in the history
  • Loading branch information
ryanbrandenburg authored Dec 5, 2023
1 parent 8a7277c commit 1ecdf6d
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class CreateGroupSecurityUpdatePullRequest
include GroupUpdateCreation

def self.applies_to?(job:)
return false if Dependabot::Experiments.enabled?(:grouped_security_updates_disabled)
return false if job.updating_a_pull_request?
# If we haven't been given data for the vulnerable dependency,
# this strategy cannot act.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class RefreshGroupSecurityUpdatePullRequest
include GroupUpdateRefreshing

def self.applies_to?(job:)
return false if Dependabot::Experiments.enabled?(:grouped_security_updates_disabled)
# If we haven't been given data for the vulnerable dependency,
# this strategy cannot act.
return false unless job.dependencies&.any?
Expand Down
7 changes: 5 additions & 2 deletions updater/spec/dependabot/file_fetcher_command_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,11 @@
allow_any_instance_of(Dependabot::Bundler::FileFetcher)
.to receive(:commit)
.and_raise(StandardError, "my_branch")
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(true)
Dependabot::Experiments.register(:record_update_job_unknown_error, true)
end

after do
Dependabot::Experiments.reset!
end

it "tells the backend about the error via update job error api (and doesn't re-raise it)" do
Expand Down Expand Up @@ -176,7 +180,6 @@
allow_any_instance_of(Dependabot::Bundler::FileFetcher)
.to receive(:commit)
.and_raise(StandardError, "my_branch")
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(false)
end

it "tells the backend about the error via update job error api (and doesn't re-raise it)" do
Expand Down
10 changes: 6 additions & 4 deletions updater/spec/dependabot/integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,12 @@
context "when there is an exception that blocks PR creation (cloud)" do
before do
allow(api_client).to receive(:create_pull_request).and_raise(StandardError, "oh no!")
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_ecosystem_versions).and_return(true)
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(true)
Dependabot::Experiments.register(:record_ecosystem_versions, true)
Dependabot::Experiments.register(:record_update_job_unknown_error, true)
end

after do
Dependabot::Experiments.reset!
end

it "notifies Dependabot API of the problem" do
Expand Down Expand Up @@ -258,8 +262,6 @@

context "when there is an exception that blocks PR creation (ghes)" do
before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_ecosystem_versions).and_return(false)
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(false)
allow(api_client).to receive(:create_pull_request).and_raise(StandardError, "oh no!")
end

Expand Down
10 changes: 5 additions & 5 deletions updater/spec/dependabot/update_files_command_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,11 @@
context "with an update files error (cloud)" do
let(:error) { StandardError.new("hell") }
before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(true)
Dependabot::Experiments.register(:record_update_job_unknown_error, true)
end

after do
Dependabot::Experiments.reset!
end

it_behaves_like "a fast-failed job"
Expand Down Expand Up @@ -160,10 +164,6 @@
context "with an update files error (ghes)" do
let(:error) { StandardError.new("hell") }

before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(false)
end

it_behaves_like "a fast-failed job"

it "it captures the exception and records to a update job error api" do
Expand Down
30 changes: 15 additions & 15 deletions updater/spec/dependabot/updater/error_handler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,11 @@
end

before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(true)
Dependabot::Experiments.register(:record_update_job_unknown_error, true)
end

after do
Dependabot::Experiments.reset!
end

it "records the error with both update job error api services, logs the backtrace and captures the exception" do
Expand Down Expand Up @@ -115,10 +119,6 @@
end
end

before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(false)
end

it "records error with only update job error api service, logs the backtrace and captures the exception" do
expect(mock_service).to_not receive(:record_update_job_unknown_error)

Expand Down Expand Up @@ -162,7 +162,11 @@
end

before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(true)
Dependabot::Experiments.register(:record_update_job_unknown_error, true)
end

after do
Dependabot::Experiments.reset!
end

it "records the error with the service and logs the backtrace" do
Expand Down Expand Up @@ -232,10 +236,6 @@
end
end

before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(false)
end

it "records the error with the service and logs the backtrace" do
expect(mock_service).to receive(:record_update_job_error).with(
error_type: "unknown_error",
Expand Down Expand Up @@ -310,7 +310,11 @@
end

before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(true)
Dependabot::Experiments.register(:record_update_job_unknown_error, true)
end

after do
Dependabot::Experiments.reset!
end

it "records the error with the update job error services, logs the backtrace and captures the exception" do
Expand Down Expand Up @@ -360,10 +364,6 @@
end
end

before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(false)
end

it "records the error with the update job error services, logs the backtrace and captures the exception" do
expect(mock_service).to receive(:record_update_job_error).with(
error_type: "unknown_error",
Expand Down
38 changes: 38 additions & 0 deletions updater/spec/dependabot/updater/operations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,44 @@
.to be(Dependabot::Updater::Operations::CreateSecurityUpdatePullRequest)
end

it "returns the CreateGroupSecurityUpdatePullRequest class when Experiment flag is not provided" do
job = instance_double(Dependabot::Job,
security_updates_only?: true,
updating_a_pull_request?: false,
dependencies: [anything, anything],
dependency_groups: [anything],
is_a?: true)

expect(described_class.class_for(job: job))
.to be(Dependabot::Updater::Operations::CreateGroupSecurityUpdatePullRequest)
end

it "returns the CreateGroupSecurityUpdatePullRequest class when Experiment flag is off" do
Dependabot::Experiments.register(:grouped_security_updates_disabled, false)
job = instance_double(Dependabot::Job,
security_updates_only?: true,
updating_a_pull_request?: false,
dependencies: [anything, anything],
dependency_groups: [anything],
is_a?: true)

expect(described_class.class_for(job: job))
.to be(Dependabot::Updater::Operations::CreateGroupSecurityUpdatePullRequest)
end

it "returns the CreateSecurityUpdatePullRequest class when Experiment flag is true" do
Dependabot::Experiments.register(:grouped_security_updates_disabled, true)
job = instance_double(Dependabot::Job,
security_updates_only?: true,
updating_a_pull_request?: false,
dependencies: [anything, anything],
dependency_groups: [anything],
is_a?: true)

expect(described_class.class_for(job: job))
.to be(Dependabot::Updater::Operations::CreateSecurityUpdatePullRequest)
end

it "returns the RefreshGroupSecurityUpdatePullRequest class when the Job is for an existing security update for" \
" multiple dependencies" do
job = instance_double(Dependabot::Job,
Expand Down
12 changes: 10 additions & 2 deletions updater/spec/dependabot/updater_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1600,7 +1600,11 @@ def expect_update_checker_with_ignored_versions(versions, dependency_matcher: an

context "when an unknown error is raised while updating dependencies (cloud) " do
before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(true)
Dependabot::Experiments.register(:record_update_job_unknown_error, true)
end

after do
Dependabot::Experiments.reset!
end

it "tells Sentry" do
Expand Down Expand Up @@ -1983,7 +1987,11 @@ def expect_update_checker_with_ignored_versions(versions, dependency_matcher: an

context "when an unknown error is raised while updating dependencies (ghes)" do
before do
allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(false)
Dependabot::Experiments.register(:record_update_job_unknown_error, false)
end

after do
Dependabot::Experiments.reset!
end

it "tells Sentry" do
Expand Down

0 comments on commit 1ecdf6d

Please sign in to comment.