Prepare to use pydantic config

Refactor most validation tests to be parameterized
Remove redundant tests

buildrunner/__init__.py

@@ -129,7 +129,7 @@ def __init__(
         push: bool,
         cleanup_images: bool,
         cleanup_cache: bool,
-        steps_to_run: List[str],
+        steps_to_run: Optional[List[str]],
         publish_ports: bool,
         log_generated_files: bool,
         docker_timeout: int,
@@ -409,7 +409,6 @@ def get_filename(caches_root, cache_name):
             cache_name = f"{project_name}-{cache_name}"
 
         caches_root = self.global_config.get("caches-root", DEFAULT_CACHES_ROOT)
-        local_cache_archive_file = None
         try:
             local_cache_archive_file = get_filename(caches_root, cache_name)
         except Exception as exc:  # pylint: disable=broad-except

buildrunner/config.py

@@ -37,7 +37,7 @@
     load_config,
 )
 
-from buildrunner.validation.config import validate_config
+from buildrunner.validation.config import generate_and_validate_config
 
 from . import fetch
 
@@ -461,7 +461,7 @@ def load_config(self, cfg_file, ctx=None, log_file=True, default_tag=None) -> di
             # Always add default tag if not set
             config = self._set_default_tag(config, default_tag)
 
-            errors = validate_config(**config)
+            _, errors = generate_and_validate_config(**config)
             if errors:
                 raise BuildRunnerConfigurationError(
                     f"Invalid configuration, {errors.count()} error(s) found:"

buildrunner/validation/config.py

@@ -7,7 +7,7 @@
 """
 
 import os
-from typing import Any, Dict, List, Optional, Set, Union
+from typing import Any, Dict, List, Optional, Set, Tuple, Union
 
 # pylint: disable=no-name-in-module
 from pydantic import BaseModel, Field, field_validator, ValidationError
@@ -327,7 +327,7 @@ def validate_multi_platform_build(mp_push_tags: Set[str]):
                         )
 
                     if step.run:
-                        raise ValueError(f"{RUN_MP_ERROR_MESSAGE} {step_name}")
+                        raise ValueError(f"{RUN_MP_ERROR_MESSAGE} step {step_name}")
 
                     # Check for valid push section, duplicate mp tags are not allowed
                     validate_push(step.push, mp_push_tags, step_name)
@@ -359,16 +359,15 @@ def validate_multi_platform_build(mp_push_tags: Set[str]):
         return vals
 
 
-def validate_config(**kwargs) -> Errors:
+def generate_and_validate_config(**kwargs) -> Tuple[Optional[Config], Optional[Errors]]:
     """
-    Check if the config file is valid
+    Check if the config file is valid and return the config instance or validation errors.
 
-    Raises:
-        ValueError | pydantic.ValidationError : If the config file is invalid
+    Returns:
+        errors.Errors : If the config file is invalid
+        Config : If the config file is valid
     """
-    errors = None
     try:
-        Config(**kwargs)
+        return Config(**kwargs), None
     except ValidationError as exc:
-        errors = get_validation_errors(exc)
-    return errors
+        return None, get_validation_errors(exc)

tests/test_config_validation/conftest.py

@@ -0,0 +1,32 @@
+import os
+from typing import List, Union
+
+import pytest
+import yaml
+
+from buildrunner.validation.config import generate_and_validate_config, Errors
+
+
+@pytest.fixture(autouse=True)
+def set_cwd():
+    # Some of the validation tests rely on loading the Dockerfiles in the correct directories, so set the path to the
+    # top-level project folder (i.e. the root of the repository)
+    os.chdir(os.path.realpath(os.path.join(os.path.dirname(__file__), "../..")))
+
+
+@pytest.fixture()
+def assert_generate_and_validate_config_errors():
+    def _func(config_data: Union[str, dict], error_matches: List[str]):
+        if isinstance(config_data, str):
+            config_data = yaml.load(config_data, Loader=yaml.Loader)
+        config, errors = generate_and_validate_config(**config_data)
+        if error_matches:
+            assert not config
+            assert isinstance(errors, Errors)
+            for index, error_match in enumerate(error_matches):
+                assert error_match in errors.errors[index].message
+        else:
+            assert config
+            assert not errors
+
+    return _func

tests/test_config_validation/test_global_config.py

@@ -1,11 +1,11 @@
-import yaml
+import pytest
 
-from buildrunner.validation.config import validate_config
-from buildrunner.validation.errors import Errors
 
-
-def test_global_config():
-    config_yaml = """
+@pytest.mark.parametrize(
+    "config_yaml, error_matches",
+    [
+        (
+            """
     # The 'env' global configuration may be used to set environment variables
     # available to all buildrunner runs that load this config file. Env vars do
     # not need to begin with a prefix to be included in this list (i.e.
@@ -70,24 +70,18 @@ def test_global_config():
     # Setting the TMP, TMPDIR, or TEMP env vars should do the same thing,
     # but on some systems it may be necessary to use this instead.
     temp-dir: /my/tmp/dir
-    """
-    config = yaml.load(config_yaml, Loader=yaml.Loader)
-    errors = validate_config(**config)
-    assert errors is None
-
-
-def test_global_config_ssh_key_file():
-    config_yaml = """
+        """,
+            [],
+        ),
+        (
+            """
     ssh-keys:
       - file: /path/to/ssh/private/key.pem
-    """
-    config = yaml.load(config_yaml, Loader=yaml.Loader)
-    errors = validate_config(**config)
-    assert errors is None
-
-
-def test_global_config_ssh_invalid():
-    config_yaml = """
+        """,
+            [],
+        ),
+        (
+            """
     ssh-keys:
       key: |
         -----INLINE KEY-----
@@ -98,9 +92,12 @@ def test_global_config_ssh_invalid():
       aliases:
         - 'my-github-key'
       bogus-attribute: 'bogus'
-    """
-    config = yaml.load(config_yaml, Loader=yaml.Loader)
-    errors = validate_config(**config)
-    print(errors)
-    assert isinstance(errors, Errors)
-    assert errors.count() > 0
+        """,
+            ["Extra inputs are not permitted", "Input should be a valid list"],
+        ),
+    ],
+)
+def test_config_data(
+    config_yaml, error_matches, assert_generate_and_validate_config_errors
+):
+    assert_generate_and_validate_config_errors(config_yaml, error_matches)

tests/test_config_validation/test_multi_platform.py

@@ -1,10 +1,16 @@
-import yaml
-from buildrunner.validation.config import validate_config, Errors, RUN_MP_ERROR_MESSAGE
+import pytest
 
+from buildrunner.validation.config import (
+    RUN_MP_ERROR_MESSAGE,
+)
 
-def test_no_run_with_multiplatform_build():
-    # Run in multi platform build is not supported
-    config_yaml = """
+
+@pytest.mark.parametrize(
+    "config_yaml, error_matches",
+    [
+        # Run in multiplatform build is not supported
+        (
+            """
     steps:
         build-container-multi-platform:
             build:
@@ -19,16 +25,14 @@ def test_no_run_with_multiplatform_build():
             run:
                 image: user1/buildrunner-test-multi-platform
                 cmd: echo "Hello World"
-    """
-    config = yaml.load(config_yaml, Loader=yaml.Loader)
-    errors = validate_config(**config)
-    assert isinstance(errors, Errors)
-    assert errors.count() == 1
-
-
-def test_no_run_with_single_build():
-    # Run in single platform build is supported
-    config_yaml = """
+    """,
+            [
+                "run is not allowed in the same step as a multi-platform build step build-container-multi-platform"
+            ],
+        ),
+        # Run in single platform build is supported
+        (
+            """
     steps:
         build-container:
             build:
@@ -40,15 +44,12 @@ def test_no_run_with_single_build():
             run:
                 image: user1/buildrunner-test-multi-platform
                 cmd: echo "Hello World"
-    """
-    config = yaml.load(config_yaml, Loader=yaml.Loader)
-    errors = validate_config(**config)
-    assert errors is None
-
-
-def test_invalid_post_build():
-    # Post build is not supported for multi platform builds
-    config_yaml = """
+    """,
+            [],
+        ),
+        # Post build is not supported for multiplatform builds
+        (
+            """
     steps:
         build-container-multi-platform:
             build:
@@ -59,25 +60,25 @@ def test_invalid_post_build():
                     - linux/arm64/v8
             run:
                 post-build: path/to/build/context
-    """
-    config = yaml.load(config_yaml, Loader=yaml.Loader)
-    errors = validate_config(**config)
-    assert isinstance(errors, Errors)
-    assert errors.count() == 1
-    assert RUN_MP_ERROR_MESSAGE in errors.errors[0].message
-
-
-def test_valid_post_build():
-    # Post build is supported for single platform builds
-    config_yaml = """
+    """,
+            [RUN_MP_ERROR_MESSAGE],
+        ),
+        # Post build is supported for single platform builds
+        (
+            """
     steps:
         build-container-single-platform:
             build:
                 dockerfile: |
                     FROM busybox:latest
             run:
                 post-build: path/to/build/context
-    """
-    config = yaml.load(config_yaml, Loader=yaml.Loader)
-    errors = validate_config(**config)
-    assert errors is None
+    """,
+            [],
+        ),
+    ],
+)
+def test_config_data(
+    config_yaml, error_matches, assert_generate_and_validate_config_errors
+):
+    assert_generate_and_validate_config_errors(config_yaml, error_matches)