Add parallel tests and change source image removal failure to be a warning

Author: shanejbrown

.github/workflows/build.yaml

@@ -50,7 +50,10 @@ jobs:
         chmod 700 ~/.ssh
         chmod 600 ~/.ssh/buildrunner-deploy-*
     - name: Test with pytest
-      run: pytest -v --junitxml=test-reports/test-results.xml
+      run: |
+        pytest -v -m "not serial" --numprocesses=auto --junitxml=test-reports/serial-test-results.xml
+        pytest -v -m "serial" --junitxml=test-reports/non-serial-test-results.xml
+        python scripts/combine_xml.py test-reports/serial-test-results.xml test-reports/non-serial-test-results.xml > test-reports/test-result.xml
     - name: Publish test results
       uses: EnricoMi/publish-unit-test-result-action/linux@v2
       if: always()

buildrunner/__init__.py

@@ -25,6 +25,7 @@
 
 from retry import retry
 from vcsinfo import detect_vcs, VCSUnsupported, VCSMissingRevision
+from docker.errors import ImageNotFound
 
 from buildrunner import docker, loggers
 from buildrunner.config import (
@@ -516,11 +517,16 @@ def run(self):  # pylint: disable=too-many-statements,too-many-branches,too-many
             # cleanup the source image
             if self._source_image:
                 self.log.write(f"Destroying source image {self._source_image}\n")
-                _docker_client.remove_image(
-                    self._source_image,
-                    noprune=False,
-                    force=True,
-                )
+                try:
+                    _docker_client.remove_image(
+                        self._source_image,
+                        noprune=False,
+                        force=True,
+                    )
+                except ImageNotFound:
+                    self.log.warning(
+                        f"Failed to remove source image {self._source_image}\n"
+                    )
 
             if self.cleanup_images:
                 self.log.write("Removing local copy of generated images\n")

pytest.ini

@@ -0,0 +1,4 @@
+[pytest]
+addopts = --strict-markers
+markers =
+    serial: Mark tests which should be done serially

scripts/combine_xml.py

@@ -0,0 +1,26 @@
+"""
+Copyright 2024 Adobe
+All Rights Reserved.
+
+NOTICE: Adobe permits you to use, modify, and distribute this file in accordance
+with the terms of the Adobe license agreement accompanying it.
+"""
+
+import sys
+from xml.etree import ElementTree
+
+
+def run(files):
+    first = None
+    for filename in files:
+        data = ElementTree.parse(filename).getroot()
+        if first is None:
+            first = data
+        else:
+            first.extend(data)
+    if first is not None:
+        print(ElementTree.tostring(first, encoding="unicode"))
+
+
+if __name__ == "__main__":
+    run(sys.argv[1:])

tests/test_buildrunner_files.py

@@ -10,6 +10,13 @@
 TEST_DIR = os.path.dirname(__file__)
 top_dir_path = os.path.realpath(os.path.dirname(test_dir_path))
 
+serial_test_files = [
+    "test-general-buildx.yaml",
+    "test-general.yaml",
+    "test-push-artifact-buildx.yaml",
+    "test-security-scan.yaml",
+]
+
 
 def _get_test_args(file_name: str) -> Optional[List[str]]:
     if file_name == "test-timeout.yaml":
@@ -52,14 +59,22 @@ def _get_exit_code(file_name: str) -> int:
     return os.EX_OK
 
 
-def _get_test_runs(test_dir: str) -> List[Tuple[str, str, Optional[List[str]], int]]:
-    file_names = sorted(
-        [
-            file_name
-            for file_name in os.listdir(test_dir)
-            if file_name.startswith("test-") and file_name.endswith(".yaml")
-        ]
-    )
+def _get_test_runs(
+    test_dir: str, serial_tests: bool
+) -> List[Tuple[str, str, Optional[List[str]], int]]:
+    file_names = []
+    for file_name in os.listdir(test_dir):
+        if serial_tests:
+            if file_name in serial_test_files:
+                file_names.append(file_name)
+        else:
+            if (
+                file_name.startswith("test-")
+                and file_name.endswith(".yaml")
+                and file_name not in serial_test_files
+            ):
+                file_names.append(file_name)
+
     return [
         (test_dir, file_name, _get_test_args(file_name), _get_exit_code(file_name))
         for file_name in file_names
@@ -107,19 +122,42 @@ def fixture_set_env():
 
 
 @pytest.mark.parametrize(
-    "test_dir, file_name, args, exit_code", _get_test_runs(f"{TEST_DIR}/test-files")
+    "test_dir, file_name, args, exit_code",
+    _get_test_runs(test_dir=f"{TEST_DIR}/test-files", serial_tests=False),
 )
 def test_buildrunner_dir(test_dir: str, file_name, args, exit_code):
     _test_buildrunner_file(test_dir, file_name, args, exit_code)
 
 
+@pytest.mark.serial
+@pytest.mark.parametrize(
+    "test_dir, file_name, args, exit_code",
+    _get_test_runs(test_dir=f"{TEST_DIR}/test-files", serial_tests=True),
+)
+def test_serial_buildrunner_dir(test_dir: str, file_name, args, exit_code):
+    _test_buildrunner_file(test_dir, file_name, args, exit_code)
+
+
 @pytest.mark.skipif(
     "arm64" not in platform.uname().machine,
     reason="This test should only be run on arm64 architecture",
 )
 @pytest.mark.parametrize(
     "test_dir, file_name, args, exit_code",
-    _get_test_runs(f"{TEST_DIR}/test-files/arm-arch"),
+    _get_test_runs(test_dir=f"{TEST_DIR}/test-files/arm-arch", serial_tests=False),
 )
 def test_buildrunner_arm_dir(test_dir: str, file_name, args, exit_code):
     _test_buildrunner_file(test_dir, file_name, args, exit_code)
+
+
+@pytest.mark.serial
+@pytest.mark.skipif(
+    "arm64" not in platform.uname().machine,
+    reason="This test should only be run on arm64 architecture",
+)
+@pytest.mark.parametrize(
+    "test_dir, file_name, args, exit_code",
+    _get_test_runs(test_dir=f"{TEST_DIR}/test-files/arm-arch", serial_tests=True),
+)
+def test_serial_buildrunner_arm_dir(test_dir: str, file_name, args, exit_code):
+    _test_buildrunner_file(test_dir, file_name, args, exit_code)

tests/test_caching.py

@@ -119,6 +119,7 @@ def setup_cache_test_files(
     return test_files
 
 
+@pytest.mark.serial
 def test_restore_cache_basic(runner, tmp_dir_name, mock_logger, log_output):
     """
     Tests basic restore cache functionality
@@ -149,6 +150,7 @@ def test_restore_cache_basic(runner, tmp_dir_name, mock_logger, log_output):
         assert f"{file}\n" in output
 
 
+@pytest.mark.serial
 def test_restore_cache_no_cache(runner, mock_logger, log_output):
     """
     Tests restore cache when a match is not found
@@ -179,6 +181,7 @@ def test_restore_cache_no_cache(runner, mock_logger, log_output):
             assert f"{file}\n" not in output
 
 
+@pytest.mark.serial
 def test_restore_cache_prefix_matching(runner, tmp_dir_name, mock_logger, log_output):
     """
     Tests restore cache when there is prefix matching
@@ -218,6 +221,7 @@ def test_restore_cache_prefix_matching(runner, tmp_dir_name, mock_logger, log_ou
         assert f"{file}\n" in output
 
 
+@pytest.mark.serial
 def test_restore_cache_prefix_timestamps(runner, tmp_dir_name, mock_logger, log_output):
     """
     Tests that when the cache prefix matches it chooses the most recent archive file
@@ -263,6 +267,7 @@ def test_restore_cache_prefix_timestamps(runner, tmp_dir_name, mock_logger, log_
         assert f"{file}\n" in output
 
 
+@pytest.mark.serial
 def test_save_cache_basic(runner, tmp_dir_name, mock_logger):
     """
     Test basic save cache functionality
@@ -297,6 +302,7 @@ def test_save_cache_basic(runner, tmp_dir_name, mock_logger):
             assert file in extracted_files
 
 
+@pytest.mark.serial
 def test_save_cache_multiple_cache_keys(runner, tmp_dir_name, mock_logger):
     """
     Test save cache functionality when there are multiple cache keys.
@@ -382,6 +388,7 @@ def test_save_cache_multiple_cache_keys(runner, tmp_dir_name, mock_logger):
             assert file in extracted_files
 
 
+@pytest.mark.serial
 def test_save_cache_multiple_caches(runner, tmp_dir_name, mock_logger):
     venv_cache_name = "venv"
     venv_docker_path = "/root/venv_cache"

tests/test_multiplatform.py

@@ -260,6 +260,7 @@ def test_tag_native_platform_keep_images(name, platforms, expected_image_tags):
         docker.image.remove(name, force=True)
 
 
+@pytest.mark.serial
 def test_push():
     try:
         with MultiplatformImageBuilder() as remote_mp:
@@ -302,6 +303,7 @@ def test_push():
         docker.image.remove(build_name, force=True)
 
 
+@pytest.mark.serial
 def test_push_with_dest_names():
     dest_names = None
     try:
@@ -349,6 +351,7 @@ def test_push_with_dest_names():
             docker.image.remove(dest_name, force=True)
 
 
+@pytest.mark.serial
 @pytest.mark.parametrize(
     "name, platforms, expected_image_tags",
     [
@@ -402,6 +405,7 @@ def test_build(
         ), f"Failed to find {missing_images} in {[image.repo for image in built_image.built_images]}"
 
 
+@pytest.mark.serial
 @patch("buildrunner.docker.multiplatform_image_builder.docker.image.remove")
 @patch("buildrunner.docker.multiplatform_image_builder.docker.push")
 @patch(
@@ -544,6 +548,7 @@ def test_build_multiple_builds(
     ]
 
 
+@pytest.mark.serial
 @pytest.mark.parametrize(
     "builder, cache_builders, return_cache_options",
     [
@@ -591,6 +596,7 @@ def test_use_build_registry():
         registry_mpib._stop_local_registry()
 
 
+@pytest.mark.serial
 @pytest.mark.parametrize(
     "side_effect, expected_call_count",
     [
@@ -664,6 +670,7 @@ def test_push_retries(mock_docker, mock_config, side_effect, expected_call_count
     assert mock_docker.call_count == expected_call_count
 
 
+@pytest.mark.serial
 @pytest.mark.parametrize(
     "tagged_images, expected_call_count",
     [

tests/test_push_artifact.py

@@ -123,7 +123,7 @@ def test_artifacts_with_legacy_builder(test_name, artifacts_in_file):
     )
 
 
-# Test legacy builder
+# Test buildx builder
 @pytest.mark.parametrize(
     "test_name, artifacts_in_file",
     [