TIAF script fixes (#3044)

* Fix typo in MARS document key. Signed-off-by: John <jonawals@amazon.com> * Add diff support for commits on different branches. Signed-off-by: John <jonawals@amazon.com> * Restore archiving of historic data objects. Signed-off-by: John <jonawals@amazon.com> * Correctly handle commit diffs for PR builds. Signed-off-by: John <jonawals@amazon.com> * Comment out archiving of historic data objects. Signed-off-by: John <jonawals@amazon.com> * Fix python typo Signed-off-by: John <jonawals@amazon.com> * Remove comment pertaining to s3 bucket. Signed-off-by: John <jonawals@amazon.com> * Fix test selection efficiency for MARS. Signed-off-by: John <jonawals@amazon.com>
2021-08-12 10:21:33 +01:00
parent c410f1385c
commit f4e8ecd35e
4 changed files with 41 additions and 14 deletions
@@ -21,12 +21,14 @@ class Repo:
        branch = self._repo.active_branch
        return branch.name

-    def create_diff_file(self, src_commit_hash: str, dst_commit_hash: str, output_path: pathlib.Path):
+    def create_diff_file(self, src_commit_hash: str, dst_commit_hash: str, output_path: pathlib.Path, multi_branch: bool):
        """
        Attempts to create a diff from the src and dst commits and write to the specified output file.

        @param src_commit_hash: The hash for the source commit.
        @param dst_commit_hash: The hash for the destination commit.
+        @param multi_branch:    The two commits are on different branches so view the changes on the 
+                                branch containing and up to dst_commit, starting at a common ancestor of both.
        @param output_path:     The path to the file to write the diff to.
        """

@@ -39,8 +41,15 @@ class Repo:
        except EnvironmentError as e:
            raise RuntimeError(f"Could not create path for output file '{output_path}'")

+        args = ["git", "diff", "--name-status", f"--output={output_path}"]
+        if multi_branch:
+            args.append(f"{src_commit_hash}...{dst_commit_hash}")
+        else:
+            args.append(src_commit_hash)
+            args.append(dst_commit_hash)
+
        # git diff will only write to the output file if both commit hashes are valid
-        subprocess.run(["git", "diff", "--name-status", f"--output={output_path}", src_commit_hash, dst_commit_hash])
+        subprocess.run(args)
        if not output_path.is_file():
            raise RuntimeError(f"Source commit '{src_commit_hash}' and/or destination commit '{dst_commit_hash}' are invalid")

@@ -15,7 +15,7 @@ logger = get_logger(__file__)

 MARS_JOB_KEY = "job"
 SRC_COMMIT_KEY = "src_commit"
-DST_COMMIT_KEY = "src_commit"
+DST_COMMIT_KEY = "dst_commit"
 COMMIT_DISTANCE_KEY = "commit_distance"
 SRC_BRANCH_KEY = "src_branch"
 DST_BRANCH_KEY = "dst_branch"
@@ -318,7 +318,7 @@ def generate_mars_sequence(sequence_report: dict, mars_job: dict, change_list:di
    test_run_selection = {}
    test_run_selection[SELECTED_KEY] = generate_mars_test_run_selections(sequence_report[SELECTED_TEST_RUNS_KEY], sequence_report[SELECTED_TEST_RUN_REPORT_KEY], t0_timestamp)
    if sequence_report[SEQUENCE_TYPE_KEY] == IMPACT_ANALYSIS_SEQUENCE_TYPE_KEY or sequence_report[SEQUENCE_TYPE_KEY] == SAFE_IMPACT_ANALYSIS_SEQUENCE_TYPE_KEY:
-        total_test_runs = sequence_report[TOTAL_NUM_TEST_RUNS_KEY]
+        total_test_runs = sequence_report[TOTAL_NUM_TEST_RUNS_KEY] + len(sequence_report[DISCARDED_TEST_RUNS_KEY])
        if total_test_runs > 0:
            test_run_selection[SELECTED_KEY][EFFICIENCY_KEY] = (1.0 - (test_run_selection[SELECTED_KEY][TOTAL_NUM_TEST_RUNS_KEY] / total_test_runs)) * 100
        else:
@@ -50,7 +50,7 @@ class TestImpact:
                    logger.warning(f"Could not find TIAF binary at location {self._tiaf_bin}, TIAF will be turned off.")
                    self._use_test_impact_analysis = False
                else:
-                    logger.info(f"Runtime binary found at location {self._tiaf_bin}")
+                    logger.info(f"Runtime binary found at location '{self._tiaf_bin}'")

                # Workspaces
                self._active_workspace = self._config["workspace"]["active"]["root"]
@@ -71,21 +71,33 @@ class TestImpact:

        self._has_change_list = False
        self._change_list_path = None
+        self._src_commit = last_commit_hash

        # Check whether or not a previous commit hash exists (no hash is not a failure)
-        self._src_commit = last_commit_hash
        if self._src_commit:
-            if self._repo.is_descendent(self._src_commit, self._dst_commit) == False:
-                logger.info(f"Source commit '{self._src_commit}' and destination commit '{self._dst_commit}' are not related.")
-                return
-            self._commit_distance = self._repo.commit_distance(self._src_commit, self._dst_commit)
-            diff_path = pathlib.Path(pathlib.PurePath(self._temp_workspace).joinpath(f"changelist.{instance_id}.diff"))
+            if self._is_source_of_truth_branch:
+                # For branch builds, the dst commit must be descended from the src commit
+                if not self._repo.is_descendent(self._src_commit, self._dst_commit):
+                    logger.error(f"Source commit '{self._src_commit}' and destination commit '{self._dst_commit}' must be related for branch builds.")
+                    return
+
+                # Calculate the distance (in commits) between the src and dst commits
+                self._commit_distance = self._repo.commit_distance(self._src_commit, self._dst_commit)
+                logger.info(f"The distance between '{self._src_commit}' and '{self._dst_commit}' commits is '{self._commit_distance}' commits.")
+                multi_branch = False
+            else:
+                # For pull request builds, the src and dst commits are on different branches so we need to ensure a common ancestor is used for the diff
+                multi_branch = True
+
            try:
-                self._repo.create_diff_file(self._src_commit, self._dst_commit, diff_path)
+                # Attempt to generate a diff between the src and dst commits
+                logger.error(f"Source '{self._src_commit}' and destination '{self._dst_commit}' will be diff'd.")
+                diff_path = pathlib.Path(pathlib.PurePath(self._temp_workspace).joinpath(f"changelist.{instance_id}.diff"))
+                self._repo.create_diff_file(self._src_commit, self._dst_commit, diff_path, multi_branch)
            except RuntimeError as e:
                logger.error(e)
                return
-                
+
            # A diff was generated, attempt to parse the diff and construct the change list
            logger.info(f"Generated diff between commits '{self._src_commit}' and '{self._dst_commit}': '{diff_path}'.") 
            with open(diff_path, "r") as diff_data:
@@ -189,7 +201,7 @@ class TestImpact:
            self._is_source_of_truth_branch = True
            self._source_of_truth_branch = self._src_branch
        else:
-            # PR builds use their destination as the source of truth and never update the coverage data for the source of truth
+            # Pull request builds use their destination as the source of truth and never update the coverage data for the source of truth
            self._is_source_of_truth_branch = False
            self._source_of_truth_branch = self._dst_branch

@@ -48,6 +48,12 @@ class PersistentStorageS3(PersistentStorage):
            for object in self._bucket.objects.filter(Prefix=self._historic_data_key):
                logger.info(f"Historic data found for branch '{branch}'.")

+                # Archive the existing object with the name of the existing last commit hash
+                #archive_key = f"{self._dir}/archive/{self._last_commit_hash}.{object_extension}"
+                #logger.info(f"Archiving existing historic data to '{archive_key}' in bucket '{self._bucket.name}'...")
+                #self._bucket.copy({"Bucket": self._bucket.name, "Key": self._historic_data_key}, archive_key)
+                #logger.info(f"Archiving complete.")
+
                # Decode the historic data object into raw bytes
                logger.info(f"Attempting to decode historic data object...")
                response = object.get()