[Progression] Move update_build_metadata to postprocess (#3463)

src/clusterfuzz/_internal/bot/tasks/utasks/fuzz_task.py

@@ -1787,8 +1787,7 @@ def run(self):
                                                       crash_revision)
     # TODO(https://github.com/google/clusterfuzz/issues/3008): Move this to
     # postprocess.
-    testcase_manager.update_build_metadata(self.job_type, crash_revision,
-                                           build_data)
+    testcase_manager.update_build_metadata(self.job_type, build_data)
     _track_build_run_result(self.job_type, crash_revision,
                             build_data.is_bad_build)
     if build_data.is_bad_build:

src/clusterfuzz/_internal/bot/tasks/utasks/progression_task.py

@@ -15,6 +15,7 @@
 
 import os
 import time
+from typing import List
 
 from clusterfuzz._internal.base import bisection
 from clusterfuzz._internal.base import tasks
@@ -51,6 +52,14 @@ def _maybe_clear_progression_last_min_max_metadata(
     testcase.put()
 
 
+def _update_build_metadata(job_type: str,
+                           build_data_list: List[uworker_msg_pb2.BuildData]):
+  """A helper method to update the build metadata corresponding to a
+  job_type."""
+  for build_data in build_data_list:
+    testcase_manager.update_build_metadata(job_type, build_data)
+
+
 def _save_current_fixed_range_indices(testcase, uworker_output):
   """Save current fixed range indices in case we die in middle of task."""
   task_output = uworker_output.progression_task_output
@@ -302,9 +311,7 @@ def _testcase_reproduces_in_revision(
         error_type=uworker_msg_pb2.ErrorType.PROGRESSION_BUILD_SETUP_ERROR)
 
   build_data = testcase_manager.check_for_bad_build(job_type, revision)
-  # TODO(https://github.com/google/clusterfuzz/issues/3008): Move this to
-  # postprocess.
-  testcase_manager.update_build_metadata(job_type, revision, build_data)
+  progression_task_output.build_data_list.append(build_data)
   if build_data.is_bad_build:
     # TODO(alhijazi): This is not logged for recoverable builds.
     error_message = f'Bad build at r{revision}. Skipping'
@@ -429,7 +436,7 @@ def find_fixed_range(uworker_input):
   min_revision = testcase.get_metadata('last_progression_min')
   max_revision = testcase.get_metadata('last_progression_max')
   progression_task_output = uworker_msg_pb2.ProgressionTaskOutput(
-      clear_min_max_metadata=False)
+      clear_min_max_metadata=False, build_data_list=[])
   if min_revision or max_revision:
     # Clear these to avoid using them in next run. If this run fails, then we
     # should try next run without them to see it succeeds. If this run succeeds,
@@ -607,6 +614,8 @@ def utask_postprocess(output: uworker_msg_pb2.Output):
   if output.HasField('progression_task_output'):
     task_output = output.progression_task_output
     _update_issue_metadata(testcase, task_output.issue_metadata)
+    _update_build_metadata(output.uworker_input.job_type,
+                           task_output.build_data_list)
 
   if output.error_type != uworker_msg_pb2.ErrorType.NO_ERROR:
     uworker_handle_errors.handle(output, HANDLED_ERRORS)

src/clusterfuzz/_internal/bot/tasks/utasks/regression_task.py

@@ -112,7 +112,7 @@ def _testcase_reproduces_in_revision(testcase,
   build_data = testcase_manager.check_for_bad_build(job_type, revision)
   # TODO(https://github.com/google/clusterfuzz/issues/3008): Move this to
   # postprocess.
-  testcase_manager.update_build_metadata(job_type, revision, build_data)
+  testcase_manager.update_build_metadata(job_type, build_data)
   if build_data.is_bad_build:
     log_message = 'Bad build at r%d. Skipping' % revision
     testcase = data_handler.get_testcase_by_id(testcase.key.id())

src/clusterfuzz/_internal/bot/testcase_manager.py

@@ -1135,6 +1135,7 @@ def check_for_bad_build(job_type: str,
     # should_ignore_crash_result set to True because build metadata does not
     # need to be updated in this case.
     return uworker_msg_pb2.BuildData(
+        revision=crash_revision,
         is_bad_build=False,
         should_ignore_crash_result=True,
         build_run_console_output='')
@@ -1198,13 +1199,13 @@ def check_for_bad_build(job_type: str,
         output=build_run_console_output,
         snapshot=process_handler.get_runtime_snapshot())
   return uworker_msg_pb2.BuildData(
+      revision=crash_revision,
       is_bad_build=is_bad_build,
       should_ignore_crash_result=crash_result.should_ignore(),
       build_run_console_output=build_run_console_output)
 
 
-def update_build_metadata(job_type: str, crash_revision: int,
-                          build_data: uworker_msg_pb2.BuildData):
+def update_build_metadata(job_type: str, build_data: uworker_msg_pb2.BuildData):
   """
   Updates the corresponding build metadata.
 
@@ -1213,16 +1214,16 @@ def update_build_metadata(job_type: str, crash_revision: int,
 
   Arguments:
     job_type (str): The type of job we are executing on.
-    crash_revision (int): The revision at which the target was built.
     build_data (BuildData): the result of check_for_bad_build call.
   """
   if build_data.should_ignore_crash_result:
     return
 
-  build_state = data_handler.get_build_state(job_type, crash_revision)
+  build_state = data_handler.get_build_state(job_type,
+                                             build_data.crash_revision)
   # If none of the other bots have added information about this build,
   # then add it now.
   if build_state == data_types.BuildState.UNMARKED:
-    data_handler.add_build_metadata(job_type, crash_revision,
+    data_handler.add_build_metadata(job_type, build_data.crash_revision,
                                     build_data.is_bad_build,
                                     build_data.build_run_console_output)

src/clusterfuzz/_internal/protos/uworker_msg.proto

@@ -148,9 +148,10 @@ message VariantTaskOutput {
 }
 
 message BuildData{
-  optional bool is_bad_build = 1;
-  optional bool should_ignore_crash_result = 2;
-  optional string build_run_console_output = 3;
+  optional int64 revision = 1;
+  optional bool is_bad_build = 2;
+  optional bool should_ignore_crash_result = 3;
+  optional string build_run_console_output = 4;
 }
 
 message ProgressionTaskOutput{
@@ -168,6 +169,7 @@ message ProgressionTaskOutput{
   // and last_progression_max from the testcase on postprocess.
   optional bool clear_min_max_metadata = 9;
   map<string, string> issue_metadata = 10;
+  repeated BuildData build_data_list = 11;
 }
 
 enum ErrorType {

src/clusterfuzz/_internal/tests/core/bot/tasks/utasks/progression_task_test.py

@@ -86,31 +86,39 @@ def test_error_on_failed_setup(self):
   def test_bad_build_error(self):
     """Tests _testcase_reproduces_in_revision behaviour on bad builds."""
     self.mock.check_app_path.return_value = True
-    self.mock.check_for_bad_build.return_value = uworker_msg_pb2.BuildData(
+    build_data = uworker_msg_pb2.BuildData(
+        revision=1,
         is_bad_build=True,
         should_ignore_crash_result=False,
         build_run_console_output='')
     progression_task_output = uworker_msg_pb2.ProgressionTaskOutput()
+    self.mock.check_for_bad_build.return_value = build_data
     result, worker_output = progression_task._testcase_reproduces_in_revision(  # pylint: disable=protected-access
         None, '/tmp/blah', 'job_type', 1, progression_task_output)
     self.assertIsNone(result)
     self.assertEqual(worker_output.error_type,
                      uworker_msg_pb2.ErrorType.PROGRESSION_BAD_BUILD)
     self.assertEqual(worker_output.error_message, 'Bad build at r1. Skipping')
+    self.assertEqual(len(progression_task_output.build_data_list), 1)
+    self.assertEqual(progression_task_output.build_data_list[0], build_data)
 
   def test_no_crash(self):
     """Tests _testcase_reproduces_in_revision behaviour with no crash or error."""
     self.mock.check_app_path.return_value = True
-    self.mock.check_for_bad_build.return_value = uworker_msg_pb2.BuildData(
+    build_data = uworker_msg_pb2.BuildData(
+        revision=1,
         is_bad_build=False,
         should_ignore_crash_result=False,
         build_run_console_output='')
+    self.mock.check_for_bad_build.return_value = build_data
     testcase = data_types.Testcase()
     progression_task_output = uworker_msg_pb2.ProgressionTaskOutput()
     result, worker_output = progression_task._testcase_reproduces_in_revision(  # pylint: disable=protected-access
         testcase, '/tmp/blah', 'job_type', 1, progression_task_output)
     self.assertIsNone(worker_output)
     self.assertIsNotNone(result)
+    self.assertEqual(len(progression_task_output.build_data_list), 1)
+    self.assertEqual(progression_task_output.build_data_list[0], build_data)
 
 
 @test_utils.with_cloud_emulators('datastore')