Skip to content

[Updater] Ensure we no longer test the legacy code path #7136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
brrygrdn merged 6 commits into from
Apr 24, 2023
Merged

[Updater] Ensure we no longer test the legacy code path #7136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
db0d843
Break any tests hitting legacy code
brrygrdn Apr 20, 2023
5f01e7f
Add a debug trace to Operation::class_for
brrygrdn Apr 20, 2023
dd37d93
Fix some ignore condition tests with improper job config
brrygrdn Apr 20, 2023
4f93444
Shuffle tests with specified deps into real contexts
brrygrdn Apr 21, 2023
a91bf01
Permit the one legacy-only spec to run
brrygrdn Apr 21, 2023
7d10554
Prevent any new specs hitting legacy code by default
brrygrdn Apr 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions updater/lib/dependabot/environment.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ def self.job_definition
@job_definition ||= JSON.parse(File.read(job_path))
end

# TODO: Remove
#
# This is purely to provide a stubbable method for tests to start disabling
# this codeline and it can be removed before merging this branch
def self.legacy_run_enabled?
true
end

private_class_method def self.environment_variable(variable_name, default = :_undefined)
return ENV.fetch(variable_name, default) unless default == :_undefined

Expand Down
2 changes: 2 additions & 0 deletions updater/lib/dependabot/updater.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ def run
# This is the original logic within run, we currently fail over to this if
# no Operation class exists for the given job.
def legacy_run
raise Dependabot::NotImplemented unless Environment.legacy_run_enabled?
Copy link
Copy Markdown
Member

@jakecoffman jakecoffman Apr 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other day I was wondering why we had this exception. Nice use of it!


service.increment_metric("updater.started", tags: { operation: "Legacy" })
if job.updating_a_pull_request?
Dependabot.logger.info("Starting PR update job for #{job.source.repo}")
Expand Down
11 changes: 11 additions & 0 deletions updater/lib/dependabot/updater/operations.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,17 @@ module Operations
]

def self.class_for(job:)
# Let's not bother generating the string if debug is disabled
if Dependabot.logger.debug?
update_type = job.security_updates_only? ? "security" : "version"
update_verb = job.updating_a_pull_request? ? "refresh" : "create"
update_deps = job.dependencies&.any? ? job.dependencies.count : "all"

Dependabot.logger.debug(
"Finding operation for a #{update_type} to #{update_verb} a Pull Request for #{update_deps} dependencies"
)
end

raise ArgumentError, "Expected Dependabot::Job, got #{job.class}" unless job.is_a?(Dependabot::Job)

OPERATIONS.find { |op| op.applies_to?(job: job) }
Expand Down
Loading