fix missing base_commit_sha in mark_as_processed during exceptional behavior (#13300)

Author: jakecoffman

updater/lib/dependabot/update_files_command.rb

@@ -39,7 +39,7 @@ def perform_job
           handle_parser_error(e)
           # If dependency file parsing has failed, there's nothing more we can do,
           # so let's mark the job as processed and stop.
-          return service.mark_job_as_processed(Environment.job_definition["base_commit_sha"])
+          return service.mark_job_as_processed(base_commit_sha)
         end
 
         # Update the service's metadata about this project
@@ -50,14 +50,10 @@ def perform_job
         # As above, we can remove the responsibility for handling fatal/job halting
         # errors from Dependabot::Updater entirely.
         begin
-          Dependabot::Updater.new(
-            service: service,
-            job: job,
-            dependency_snapshot: dependency_snapshot
-          ).run
+          Dependabot::Updater.new(service:, job:, dependency_snapshot:).run
         rescue Dependabot::DependencyFileNotParseable => e
           handle_dependency_file_not_parseable_error(e)
-          return service.mark_job_as_processed(Environment.job_definition["base_commit_sha"])
+          return service.mark_job_as_processed(base_commit_sha)
         end
 
         # Wait for all PRs to be created
@@ -66,7 +62,7 @@ def perform_job
         # Finally, mark the job as processed. The Dependabot::Updater may have
         # reported errors to the service, but we always consider the job as
         # successfully processed unless it actually raises.
-        service.mark_job_as_processed(dependency_snapshot.base_commit_sha)
+        service.mark_job_as_processed(base_commit_sha)
       end
     end
 
@@ -84,7 +80,7 @@ def job
 
     sig { override.returns(T.nilable(String)) }
     def base_commit_sha
-      Environment.job_definition["base_commit_sha"]
+      @fetched_files.base_commit_sha
     end
 
     private

updater/spec/dependabot/update_files_command_spec.rb

@@ -22,19 +22,20 @@
     )
   end
   let(:job_definition) do
-    JSON.parse(fixture("file_fetcher_output/output.json"))
+    JSON.parse(fixture("jobs/job_without_credentials.json"))
+  end
+  let(:manifest) do
+    Dependabot::DependencyFile.new(
+      name: "Gemfile",
+      content: fixture("bundler/original/Gemfile"),
+      directory: "/"
+    )
   end
   let(:fetched_files) do
     # We no longer write encoded files to disk, need to migrate the fixtures in this test
     Dependabot::FetchedFiles.new(
-      dependency_files: job_definition["base64_dependency_files"].map do |file|
-        Dependabot::DependencyFile.new(
-          name: file["name"],
-          content: Base64.decode64(file["content"]),
-          directory: file["directory"] || "/"
-        )
-      end,
-      base_commit_sha: job_definition["base_commit_sha"]
+      dependency_files: [manifest],
+      base_commit_sha: "1c6331732c41e4557a16dacb82534f1d1c831848"
     )
   end
   let(:job_id) { "123123" }
@@ -66,53 +67,11 @@
       expect(dummy_runner).to receive(:run)
       expect(service).to receive(:mark_job_as_processed)
         .with(base_commit_sha)
-
-      perform_job
-    end
-
-    it "sends dependency metadata to the service" do
       expect(service).to receive(:update_dependency_list)
         .with(dependency_snapshot: an_instance_of(Dependabot::DependencySnapshot))
 
       perform_job
     end
-
-    context "with vendoring_dependencies" do
-      let(:snapshot) do
-        instance_double(
-          Dependabot::DependencySnapshot,
-          base_commit_sha: "1c6331732c41e4557a16dacb82534f1d1c831848"
-        )
-      end
-      let(:repo_contents_path) { "repo/path" }
-
-      let(:job_definition) do
-        JSON.parse(fixture("file_fetcher_output/vendoring_output.json"))
-      end
-
-      before do
-        allow(Dependabot::Environment).to receive(:repo_contents_path).and_return(repo_contents_path)
-        allow(Dependabot::DependencySnapshot).to receive(:create_from_job_definition).and_return(snapshot)
-      end
-
-      it "delegates to Dependabot::Updater" do
-        dummy_runner = double(run: nil)
-        base_commit_sha = "1c6331732c41e4557a16dacb82534f1d1c831848"
-        expect(Dependabot::Updater)
-          .to receive(:new)
-          .with(
-            service: service,
-            job: an_object_having_attributes(id: job_id, repo_contents_path: repo_contents_path),
-            dependency_snapshot: snapshot
-          )
-          .and_return(dummy_runner)
-        expect(dummy_runner).to receive(:run)
-        expect(service).to receive(:mark_job_as_processed)
-          .with(base_commit_sha)
-
-        perform_job
-      end
-    end
   end
 
   describe "#perform_job when there is an error parsing the dependency files" do

updater/spec/dependabot/update_graph_command_spec.rb

@@ -22,19 +22,20 @@
     )
   end
   let(:job_definition) do
-    JSON.parse(fixture("file_fetcher_output/output-directories-only.json"))
+    JSON.parse(fixture("jobs/bundler_directories.json"))
+  end
+  let(:manifest) do
+    Dependabot::DependencyFile.new(
+      name: "Gemfile",
+      content: fixture("bundler/original/Gemfile"),
+      directory: "/"
+    )
   end
   let(:fetched_files) do
     # We no longer write encoded files to disk, need to migrate the fixtures in this test
     Dependabot::FetchedFiles.new(
-      dependency_files: job_definition["base64_dependency_files"].map do |file|
-        Dependabot::DependencyFile.new(
-          name: file["name"],
-          content: Base64.decode64(file["content"]),
-          directory: file["directory"] || "/"
-        )
-      end,
-      base_commit_sha: job_definition["base_commit_sha"]
+      dependency_files: [manifest],
+      base_commit_sha: "1c6331732c41e4557a16dacb82534f1d1c831848"
     )
   end
   let(:job_id) { "123123" }
@@ -67,8 +68,11 @@
   describe "#perform_job when the directory is empty or doesn't exist" do
     subject(:perform_job) { job.perform_job }
 
-    let(:job_definition) do
-      JSON.parse(fixture("file_fetcher_output/output-empty-directory.json"))
+    let(:fetched_files) do
+      Dependabot::FetchedFiles.new(
+        dependency_files: [],
+        base_commit_sha: "1c6331732c41e4557a16dacb82534f1d1c831848"
+      )
     end
 
     before do
@@ -78,7 +82,7 @@
 
     it "emits a create_dependency_submission call to the Dependabot service with an empty snapshot" do
       expect(service).to receive(:create_dependency_submission) do |args|
-        expected_manifest_file = Dependabot::DependencyFile.new(name: "", content: "", directory: "/foo")
+        expected_manifest_file = Dependabot::DependencyFile.new(name: "", content: "", directory: "/")
 
         expect(args[:dependency_submission]).to be_a(GithubApi::DependencySubmission)