Repository: packit-service/packit (python)

Issue:
validate-config improvements
Follow-up to #826

There are two design problems with the PR above:

1. We validate PackageConfig, not content of .packit.yaml - this is problematic b/c validation happens while parsing the config, so the present implementation is actually "late to the party":
    ```
    $ packit validate-config
    ERROR    Cannot parse package config. {'jobs': {0: {'actions': ["'dict' required, got <class 'list'>."]}, 1: {'actions': ["'dict' required, got <class 'list'>."]}, 2: {'actions': ["'dict' required, got <class 'list'>."]}, 3: {'actions': ["'dict' required, got <class 'list'>."]}}, 'actions': ["'dict' required, got <class 'list'>."]}.
    ERROR    Cannot parse package config: ValidationError({'jobs': {0: {'actions': ["'dict' required, got <class 'list'>."]}, 1: {'actions': ["'dict' required, got <class 'list'>."]}, 2: {'actions': ["'dict' required, got <class 'list'>."]}, 3: {'actions': ["'dict' required, got <class 'list'>."]}}, 'actions': ["'dict' required, got <class 'list'>."]}).
    ```

2. The config is parsed twice: once in the CLI code, second time in `PackageConfigValidation`

This is tracks the deficiency and would be nice if it was solved soon.


Pull request:
validate-config: check .packit.yaml content
multiple changes here:

* we parse PackageConfig (PC) only once

* we use PC's parsing mechanism as the form to validate

* validation code is moved to a dedicated module

* related tests are cleaned up and simplifed

Fixes #884

Patch:
diff --git a/packit/api.py b/packit/api.py
index 48e364ad..ed9e429f 100644
--- a/packit/api.py
+++ b/packit/api.py
@@ -31,16 +31,17 @@ import shutil
 import sys
 from datetime import datetime
 from pathlib import Path
-from typing import Sequence, Callable, List, Tuple, Dict, Iterable, Optional, Union
+from typing import Sequence, Callable, List, Tuple, Dict, Iterable, Optional
 
+from ogr.abstract import PullRequest
 from tabulate import tabulate
 
-from ogr.abstract import PullRequest
 from packit import utils
 from packit.actions import ActionName
 from packit.config import Config
 from packit.config.common_package_config import CommonPackageConfig
-from packit.config.package_config import PackageConfigValidation
+from packit.config.package_config import find_packit_yaml, load_packit_yaml
+from packit.config.package_config_validator import PackageConfigValidator
 from packit.constants import SYNCING_NOTE
 from packit.copr_helper import CoprHelper
 from packit.distgit import DistGit
@@ -65,7 +66,9 @@ class PackitAPI:
     def __init__(
         self,
         config: Config,
-        package_config: CommonPackageConfig,
+        package_config: Optional[
+            CommonPackageConfig
+        ],  # validate doesn't want PackageConfig
         upstream_local_project: LocalProject = None,
         downstream_local_project: LocalProject = None,
     ) -> None:
@@ -812,45 +815,10 @@ class PackitAPI:
         # command handlers have nothing to clean
         logger.debug("PackitAPI.cleanup (there are no objects to clean)")
 
-    def validate(self) -> str:
-        """ Create output for PackageConfig validation."""
-        schema_errors = PackageConfigValidation(self.package_config).validate()
-
-        if not schema_errors:
-            return (
-                f"{self.package_config.config_file_path} is valid and ready to be used"
-            )
-
-        output = f"{self.package_config.config_file_path} does not pass validation:\n"
-        for field_name, errors in schema_errors.items():
-            output += self.validate_get_field_output(errors, field_name)
-        return output
-
-    def validate_get_field_output(
-        self,
-        errors: Union[list, dict],
-        field_name: str,
-        field_category: str = "* field",
-        level: int = 1,
-    ) -> str:
-        if isinstance(errors, list):
-            field_output = f"{field_category} {field_name}: {errors[0]}\n"
-            return field_output
-
-        field_output = self.validate_get_field_item_output(errors, field_name, level)
-        return field_output
-
-    def validate_get_field_item_output(
-        self, errors: dict, field_name: str, level: int
-    ) -> str:
-        index_output = f"{level * '*'} field {field_name} has incorrect values:\n"
-        level += 1
-        for index, item_errors in errors.items():
-            index_type_error = self.validate_get_field_output(
-                item_errors,
-                index,
-                field_category=f"{level * '*'} value at index",
-                level=level,
-            )
-            index_output += index_type_error
-        return index_output
+    @staticmethod
+    def validate_package_config(working_dir: Path) -> str:
+        """ validate .packit.yaml on the provided path and return human readable report """
+        config_path = find_packit_yaml(working_dir, try_local_dir_last=True,)
+        config_content = load_packit_yaml(config_path)
+        v = PackageConfigValidator(config_path, config_content)
+        return v.validate()
diff --git a/packit/cli/validate_config.py b/packit/cli/validate_config.py
index 1c79813c..b0049891 100644
--- a/packit/cli/validate_config.py
+++ b/packit/cli/validate_config.py
@@ -29,19 +29,18 @@ import os
 
 import click
 
+from packit.api import PackitAPI
 from packit.cli.types import LocalProjectParameter
 from packit.cli.utils import cover_packit_exception
-from packit.cli.utils import get_packit_api
-from packit.config import pass_config, get_context_settings
+from packit.config import get_context_settings
 
 logger = logging.getLogger(__name__)
 
 
 @click.command("validate-config", context_settings=get_context_settings())
 @click.argument("path_or_url", type=LocalProjectParameter(), default=os.path.curdir)
-@pass_config
 @cover_packit_exception
-def validate_config(config, path_or_url):
+def validate_config(path_or_url):
     """
     Validate PackageConfig validation.
 
@@ -51,6 +50,7 @@ def validate_config(config, path_or_url):
 
     PATH_OR_URL argument is a local path or a URL to a git repository with packit configuration file
     """
-    api = get_packit_api(config=config, local_project=path_or_url)
-    output = api.validate()
+    # we use PackageConfig.load_from_dict for the validation, hence we don't parse it here
+    output = PackitAPI.validate_package_config(path_or_url)
     logger.info(output)
+    # TODO: print more if config.debug
diff --git a/packit/config/common_package_config.py b/packit/config/common_package_config.py
index 7734005b..0d428277 100644
--- a/packit/config/common_package_config.py
+++ b/packit/config/common_package_config.py
@@ -155,6 +155,7 @@ class CommonPackageConfig:
         if self.config_file_path and self.config_file_path not in (
             item.src for item in files
         ):
+            # this relative because of glob: "Non-relative patterns are unsupported"
             files.append(
                 SyncFilesItem(src=self.config_file_path, dest=self.config_file_path)
             )
diff --git a/packit/config/package_config.py b/packit/config/package_config.py
index 5377350a..e6599fcb 100644
--- a/packit/config/package_config.py
+++ b/packit/config/package_config.py
@@ -25,7 +25,6 @@ import logging
 from pathlib import Path
 from typing import Optional, List, Dict, Union
 
-from marshmallow import ValidationError
 from ogr.abstract import GitProject
 from yaml import safe_load
 
@@ -197,39 +196,17 @@ class PackageConfig(CommonPackageConfig):
         )
 
 
-class PackageConfigValidation:
+def find_packit_yaml(
+    *directory, try_local_dir_first=False, try_local_dir_last=False,
+) -> Path:
     """
-    Package config validation
-    Does not validate fields: jobs`
-    """
-
-    def __init__(self, package_config: CommonPackageConfig):
-        self.package_config: CommonPackageConfig = package_config
-
-    def validate(self) -> dict:
-        from packit.schema import PackageConfigSchema
-
-        config_file_name_full = self.package_config.config_file_path
-        try:
-            loaded_config = safe_load(open(config_file_name_full))
-        except Exception as ex:
-            logger.error(f"Cannot load package config {config_file_name_full!r}.")
-            raise PackitConfigException(f"Cannot load package config: {ex!r}.")
-
-        try:
-            return PackageConfigSchema().validate(loaded_config)
-        except ValidationError as e:
-            return e.messages
+    find packit.yaml in provided directories: if a file matches, it's picked
+    if no config is found, raise PackitConfigException
 
-
-def get_local_package_config(
-    *directory,
-    repo_name: str = None,
-    try_local_dir_first=False,
-    try_local_dir_last=False,
-) -> PackageConfig:
-    """
-    :return: local PackageConfig if present
+    :param directory: a list of dirs where we should find
+    :param try_local_dir_first: try in cwd first
+    :param try_local_dir_last: try cwd last
+    :return: Path to the config
     """
     directories = [Path(config_dir) for config_dir in directory]
     cwd = Path.cwd()
@@ -254,24 +231,52 @@ def get_local_package_config(
             config_file_name_full = config_dir / config_file_name
             if config_file_name_full.is_file():
                 logger.debug(f"Local package config found: {config_file_name_full}")
-                try:
-                    loaded_config = safe_load(open(config_file_name_full))
-                except Exception as ex:
-                    logger.error(
-                        f"Cannot load package config {config_file_name_full!r}."
-                    )
-                    raise PackitConfigException(f"Cannot load package config: {ex!r}.")
-                return parse_loaded_config(
-                    loaded_config=loaded_config,
-                    config_file_path=str(config_file_name),
-                    repo_name=repo_name,
-                    spec_file_path=str(get_local_specfile_path(config_dir)),
-                )
-
-            logger.debug(f"The local config file {config_file_name_full!r} not found.")
+                return config_file_name_full
     raise PackitConfigException("No packit config found.")
 
 
+def load_packit_yaml(config_file_path: Path) -> Dict:
+    """
+    load provided packit.yaml, raise PackitConfigException if something is wrong
+
+    :return: Dict with the file content
+    """
+    try:
+        return safe_load(config_file_path.read_text())
+    except Exception as ex:
+        logger.error(f"Cannot load package config {config_file_path}.")
+        raise PackitConfigException(f"Cannot load package config: {ex!r}.")
+
+
+def get_local_package_config(
+    *directory,
+    repo_name: str = None,
+    try_local_dir_first=False,
+    try_local_dir_last=False,
+) -> PackageConfig:
+    """
+    find packit.yaml in provided dirs, load it and return PackageConfig
+
+    :param directory: a list of dirs where we should find
+    :param repo_name: name of the git repository (default for project name)
+    :param try_local_dir_first: try in cwd first
+    :param try_local_dir_last: try cwd last
+    :return: local PackageConfig
+    """
+    config_file_name = find_packit_yaml(
+        *directory,
+        try_local_dir_first=try_local_dir_first,
+        try_local_dir_last=try_local_dir_last,
+    )
+    loaded_config = load_packit_yaml(config_file_name)
+    return parse_loaded_config(
+        loaded_config=loaded_config,
+        config_file_path=config_file_name.name,
+        repo_name=repo_name,
+        spec_file_path=str(get_local_specfile_path(config_file_name.parent)),
+    )
+
+
 def get_package_config_from_repo(
     project: GitProject, ref: str, spec_file_path: Optional[str] = None
 ) -> Optional[PackageConfig]:
diff --git a/packit/config/package_config_validator.py b/packit/config/package_config_validator.py
new file mode 100644
index 00000000..ac97abcc
--- /dev/null
+++ b/packit/config/package_config_validator.py
@@ -0,0 +1,85 @@
+# MIT License
+#
+# Copyright (c) 2018-2019 Red Hat, Inc.
+
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in all
+# copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+# SOFTWARE.
+from pathlib import Path
+from typing import Dict, Union
+
+from marshmallow import ValidationError
+
+from packit.config.package_config import PackageConfig
+from packit.exceptions import PackitConfigException
+
+
+class PackageConfigValidator:
+    """ validate content of .packit.yaml """
+
+    def __init__(self, config_file_path: Path, config_content: Dict):
+        self.config_file_path = config_file_path
+        self.content = config_content
+
+    def validate(self) -> str:
+        """ Create output for PackageConfig validation."""
+        schema_errors = None
+        try:
+            PackageConfig.get_from_dict(
+                self.content, config_file_path=str(self.config_file_path)
+            )
+        except ValidationError as e:
+            schema_errors = e.messages
+        except PackitConfigException as e:
+            return str(e)
+
+        if not schema_errors:
+            return f"{self.config_file_path.name} is valid and ready to be used"
+
+        output = f"{self.config_file_path.name} does not pass validation:\n"
+        for field_name, errors in schema_errors.items():
+            output += self.validate_get_field_output(errors, field_name)
+        return output
+
+    def validate_get_field_output(
+        self,
+        errors: Union[list, dict],
+        field_name: str,
+        field_category: str = "* field",
+        level: int = 1,
+    ) -> str:
+        if isinstance(errors, list):
+            field_output = f"{field_category} {field_name}: {errors[0]}\n"
+            return field_output
+
+        field_output = self.validate_get_field_item_output(errors, field_name, level)
+        return field_output
+
+    def validate_get_field_item_output(
+        self, errors: dict, field_name: str, level: int
+    ) -> str:
+        index_output = f"{level * '*'} field {field_name} has an incorrect value:\n"
+        level += 1
+        for index, item_errors in errors.items():
+            index_type_error = self.validate_get_field_output(
+                item_errors,
+                index,
+                field_category=f"{level * '*'} value at index",
+                level=level,
+            )
+            index_output += index_type_error
+        return index_output
diff --git a/packit/schema.py b/packit/schema.py
index c2cc3bc2..6b12b21a 100644
--- a/packit/schema.py
+++ b/packit/schema.py
@@ -346,6 +346,8 @@ class PackageConfigSchema(CommonConfigSchema):
         """
         add all the fields (except for jobs) to every job so we can process only jobconfig in p-s
         """
+        if not data:  # data is None when .packit.yaml is empty
+            return data
         for job in data.get("jobs", []):
             for k, v in data.items():
                 if k == "jobs":
@@ -361,6 +363,8 @@ class PackageConfigSchema(CommonConfigSchema):
         :param data: conf dictionary to process
         :return: processed dictionary
         """
+        if not data:  # data is None when .packit.yaml is empty
+            return data
 
         for new_key_name, old_key_name in self.deprecated_keys:
             old_key_value = data.get(old_key_name, None)
@@ -384,6 +388,8 @@ class PackageConfigSchema(CommonConfigSchema):
         :param data: conf dictionary to process
         :return: processed dictionary
         """
+        if not data:  # data is None when .packit.yaml is empty
+            return data
 
         specfile_path = data.get("specfile_path", None)
         if not specfile_path:


Test patch:
diff --git a/tests/data/validate_config/packit.json b/tests/data/validate_config/packit.json
deleted file mode 100644
index 0967ef42..00000000
--- a/tests/data/validate_config/packit.json
+++ /dev/null
@@ -1,1 +0,0 @@
-{}
diff --git a/tests/integration/test_validate_config.py b/tests/integration/test_validate_config.py
index 611b5076..9614a195 100644
--- a/tests/integration/test_validate_config.py
+++ b/tests/integration/test_validate_config.py
@@ -18,52 +18,65 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
+from pathlib import Path
+
 import pytest
 
-from packit.config import PackageConfig
-from packit.exceptions import PackitConfigException
+from packit.api import PackitAPI
+from packit.config.config import Config
 from packit.utils import cwd
-from tests.spellbook import DATA_DIR
 
 
 @pytest.mark.parametrize(
-    "package_config,raw_package_config,contains",
+    "raw_package_config,expected_output",
     [
         (
-            PackageConfig(config_file_path="packit.json",),
             """
             {
+                "config_file_path": "packit.json",
                 "dist_git_base_url": "https://packit.dev/",
                 "downstream_package_name": "packit",
                 "upstream_ref": "last_commit",
                 "upstream_package_name": "packit_upstream",
                 "create_tarball_command": ["commands"],
                 "allowed_gpg_keys": ["gpg"],
-                "dist_git_namespace": "awesome"
+                "dist_git_namespace": "awesome",
+                "notifications": {
+                    "pull_request": {
+                        "successful_build": True
+                    }
+                }
             }
             """,
             "packit.json is valid and ready to be used",
         ),
         (
-            PackageConfig(config_file_path="packit.json",),
             """
             {
+                "config_file_path": "packit.json",
                 "dist_git_base_url": "https://packit.dev/",
-                "downstream_package_name": 23,
+                "downstream_package_name": "packit",
                 "upstream_ref": "last_commit",
                 "upstream_package_name": "packit_upstream",
                 "create_tarball_command": ["commands"],
                 "allowed_gpg_keys": ["gpg"],
-                "dist_git_namespace": "awesome"
+                "dist_git_namespace": "awesome",
+                "notifications": {
+                    "pull_request": {
+                        "successful_build": 55
+                    }
+                }
             }
             """,
-            "packit.json does not pass validation:\n"
-            "* field downstream_package_name: Not a valid string.",
+            "* field notifications has an incorrect value:\n"
+            "** field pull_request has an incorrect value:\n"
+            "*** value at index successful_build: Not a valid boolean.",
         ),
+        ("{}", "Spec file was not found!"),
         (
-            PackageConfig(config_file_path="packit.json",),
             """
             {
+                "config_file_path": "packit.json",
                 "dist_git_base_url": "https://packit.dev/",
                 "downstream_package_name": "packit",
                 "upstream_ref": "last_commit",
@@ -71,271 +84,180 @@ from tests.spellbook import DATA_DIR
                 "create_tarball_command": ["commands"],
                 "allowed_gpg_keys": ["gpg"],
                 "dist_git_namespace": "awesome",
-                create_pr: ""
+                "synced_files": {
+                    "files_to_sync": ["a.md", "b.md", "c.txt"]
+                }
             }
             """,
-            "packit.json does not pass validation:\n* field create_pr: Not a valid boolean.",
+            "packit.json is valid and ready to be used",
         ),
-    ],
-)
-def test_schema_validation_primitive_types(
-    cwd_upstream, api_instance, package_config, raw_package_config, contains
-):
-    with cwd(DATA_DIR / "validate_config"):
-        u, d, api = api_instance
-        api.package_config = package_config
-
-        with open("packit.json", "w") as package_config_file:
-            package_config_file.writelines(raw_package_config)
-
-        output = api.validate()
-        assert contains in output
-
-
-@pytest.mark.parametrize(
-    "package_config,raw_package_config,contains",
-    [
         (
-            PackageConfig(config_file_path="packit.json",),
             """
-            {
-                "config_file_path": "packit.json",
-                "dist_git_base_url": "https: //packit.dev/",
-                "downstream_package_name": "packit",
-                "upstream_ref": "last_commit",
-                "upstream_package_name": "packit_upstream",
-                "create_tarball_command": ["commands"],
-                "allowed_gpg_keys": ["gpg"],
-                "dist_git_namespace": "awesome"
-            }
-            """,
-            "packit.json is valid and ready to be used",
+                {
+                    "config_file_path": "packit.json",
+                    "dist_git_base_url": "https://packit.dev/",
+                    "downstream_package_name": "packit",
+                    "upstream_ref": "last_commit",
+                    "upstream_package_name": "packit_upstream",
+                    "create_tarball_command": ["commands"],
+                    "allowed_gpg_keys": ["gpg"],
+                    "dist_git_namespace": "awesome",
+                    "synced_files": {
+                        "files_to_sync": [{ "src": 55, "dest": "a.md" }, "b.md", "c.txt"]
+                    }
+                }
+                """,
+            "* field synced_files has an incorrect value:\n"
+            "** field files_to_sync has an incorrect value:\n"
+            "*** value at index 0: Field `src` should have type str.",
         ),
         (
-            PackageConfig(config_file_path="packit.json",),
             """
-            {
-                "config_file_path": "packit.json",
-                "dist_git_base_url": "https: //packit.dev/",
-                "downstream_package_name": "packit",
-                "upstream_ref": "last_commit",
-                "upstream_package_name": "packit_upstream",
-                "create_tarball_command": ["commands"],
-                "allowed_gpg_keys": "gpg",
-                "dist_git_namespace": "awesome"
-            }
-            """,
-            "packit.json does not pass validation:\n"
-            "* field allowed_gpg_keys: Not a valid list.",
+                {
+                    "config_file_path": "packit.json",
+                    "dist_git_base_url": "https://packit.dev/",
+                    "downstream_package_name": "packit",
+                    "upstream_ref": "last_commit",
+                    "upstream_package_name": "packit_upstream",
+                    "create_tarball_command": ["commands"],
+                    "allowed_gpg_keys": ["gpg"],
+                    "dist_git_namespace": "awesome",
+                    "synced_files": {
+                        "files_to_sync": ["a.md", "b.md", { "src": "c.txt", "dest": True }]
+                    }
+                }
+                """,
+            "* field synced_files has an incorrect value:\n"
+            "** field files_to_sync has an incorrect value:\n"
+            "*** value at index 2: Field `dest` should have type str.",
         ),
         (
-            PackageConfig(config_file_path="packit.json",),
             """
-            {
-                "config_file_path":"packit.json",
-                "dist_git_base_url":"https://packit.dev/",
-                "downstream_package_name":"packit",
-                "upstream_ref":"last_commit",
-                "upstream_package_name":"packit_upstream",
-                "create_tarball_command":[25],
-                "allowed_gpg_keys":["gpg"],
-                "dist_git_namespace":"awesome"
-            }
-            """,
-            "packit.json does not pass validation:\n"
-            "* field create_tarball_command has incorrect values:\n"
-            "** value at index 0: Not a valid string.",
+                {
+                    "dist_git_base_url": "https://packit.dev/",
+                    "downstream_package_name": "packit",
+                    "upstream_ref": "last_commit",
+                    "upstream_package_name": "packit_upstream",
+                    "create_tarball_command": ["commands"],
+                    "allowed_gpg_keys": ["gpg"],
+                    "dist_git_namespace": "awesome"
+                }
+                """,
+            "packit.json is valid and ready to be used",
         ),
         (
-            PackageConfig(config_file_path="packit.json",),
             """
-            {
-                "config_file_path":"packit.json",
-                "dist_git_base_url":"https://packit.dev/",
-                "downstream_package_name":"packit",
-                "upstream_ref":"last_commit",
-                "upstream_package_name":"packit_upstream",
-                "create_tarball_command":["commands", True],
-                "allowed_gpg_keys":["gpg"],
-                "dist_git_namespace":"awesome"
-            }
-            """,
-            "packit.json does not pass validation:\n"
-            "* field create_tarball_command has incorrect values:\n"
-            "** value at index 1: Not a valid string.",
+                {
+                    "dist_git_base_url": "https://packit.dev/",
+                    "downstream_package_name": 23,
+                    "upstream_ref": "last_commit",
+                    "upstream_package_name": "packit_upstream",
+                    "create_tarball_command": ["commands"],
+                    "allowed_gpg_keys": ["gpg"],
+                    "dist_git_namespace": "awesome"
+                }
+                """,
+            "* field downstream_package_name: Not a valid string.",
         ),
-    ],
-)
-def test_schema_validation_list_types(
-    cwd_upstream, api_instance, package_config, raw_package_config, contains
-):
-    with cwd(DATA_DIR / "validate_config"):
-        u, d, api = api_instance
-        api.package_config = package_config
-
-        with open("packit.json", "w") as package_config_file:
-            package_config_file.writelines(raw_package_config)
-
-        output = api.validate()
-        assert contains in output
-
-
-@pytest.mark.parametrize(
-    "package_config,raw_package_config,contains",
-    [
         (
-            PackageConfig(config_file_path="packit.json",),
             """
-            {
-                "config_file_path": "packit.json",
-                "dist_git_base_url": "https://packit.dev/",
-                "downstream_package_name": "packit",
-                "upstream_ref": "last_commit",
-                "upstream_package_name": "packit_upstream",
-                "create_tarball_command": ["commands"],
-                "allowed_gpg_keys": ["gpg"],
-                "dist_git_namespace": "awesome",
-                "synced_files": {
-                    "files_to_sync": ["a.md", "b.md", "c.txt"]
+                {
+                    "dist_git_base_url": "https://packit.dev/",
+                    "downstream_package_name": "packit",
+                    "upstream_ref": "last_commit",
+                    "upstream_package_name": "packit_upstream",
+                    "create_tarball_command": ["commands"],
+                    "allowed_gpg_keys": ["gpg"],
+                    "dist_git_namespace": "awesome",
+                    create_pr: ""
                 }
-            }
-            """,
-            "packit.json is valid and ready to be used",
+                """,
+            "* field create_pr: Not a valid boolean.",
         ),
         (
-            PackageConfig(config_file_path="packit.json",),
             """
-            {
-                "config_file_path": "packit.json",
-                "dist_git_base_url": "https://packit.dev/",
-                "downstream_package_name": "packit",
-                "upstream_ref": "last_commit",
-                "upstream_package_name": "packit_upstream",
-                "create_tarball_command": ["commands"],
-                "allowed_gpg_keys": ["gpg"],
-                "dist_git_namespace": "awesome",
-                "synced_files": {
-                    "files_to_sync": [{ "src": 55, "dest": "a.md" }, "b.md", "c.txt"]
+                {
+                    "config_file_path": "packit.json",
+                    "dist_git_base_url": "https: //packit.dev/",
+                    "downstream_package_name": "packit",
+                    "upstream_ref": "last_commit",
+                    "upstream_package_name": "packit_upstream",
+                    "create_tarball_command": ["commands"],
+                    "allowed_gpg_keys": ["gpg"],
+                    "dist_git_namespace": "awesome"
                 }
-            }
-            """,
-            "packit.json does not pass validation:\n"
-            "* field synced_files has incorrect values:\n"
-            "** field files_to_sync has incorrect values:\n"
-            "*** value at index 0: Field `src` should have type str.",
+                """,
+            "packit.json is valid and ready to be used",
         ),
         (
-            PackageConfig(config_file_path="packit.json",),
             """
-            {
-                "config_file_path": "packit.json",
-                "dist_git_base_url": "https://packit.dev/",
-                "downstream_package_name": "packit",
-                "upstream_ref": "last_commit",
-                "upstream_package_name": "packit_upstream",
-                "create_tarball_command": ["commands"],
-                "allowed_gpg_keys": ["gpg"],
-                "dist_git_namespace": "awesome",
-                "synced_files": {
-                    "files_to_sync": ["a.md", "b.md", { "src": "c.txt", "dest": True }]
+                {
+                    "config_file_path": "packit.json",
+                    "dist_git_base_url": "https: //packit.dev/",
+                    "downstream_package_name": "packit",
+                    "upstream_ref": "last_commit",
+                    "upstream_package_name": "packit_upstream",
+                    "create_tarball_command": ["commands"],
+                    "allowed_gpg_keys": "gpg",
+                    "dist_git_namespace": "awesome"
                 }
-            }
-            """,
-            "packit.json does not pass validation:\n"
-            "* field synced_files has incorrect values:\n"
-            "** field files_to_sync has incorrect values:\n"
-            "*** value at index 2: Field `dest` should have type str.",
+                """,
+            "* field allowed_gpg_keys: Not a valid list.",
         ),
-    ],
-)
-def test_schema_validation_synced_files(
-    cwd_upstream, api_instance, package_config, raw_package_config, contains
-):
-    with cwd(DATA_DIR / "validate_config"):
-        u, d, api = api_instance
-        api.package_config = package_config
-
-        with open("packit.json", "w") as package_config_file:
-            package_config_file.writelines(raw_package_config)
-
-        output = api.validate()
-        assert contains in output
-
-
-@pytest.mark.parametrize(
-    "package_config,raw_package_config,contains",
-    [
         (
-            PackageConfig(config_file_path="packit.json",),
             """
-            {
-                "config_file_path": "packit.json",
-                "dist_git_base_url": "https://packit.dev/",
-                "downstream_package_name": "packit",
-                "upstream_ref": "last_commit",
-                "upstream_package_name": "packit_upstream",
-                "create_tarball_command": ["commands"],
-                "allowed_gpg_keys": ["gpg"],
-                "dist_git_namespace": "awesome",
-                "notifications": {
-                    "pull_request": {
-                        "successful_build": True
-                    }
+                {
+                    "config_file_path":"packit.json",
+                    "dist_git_base_url":"https://packit.dev/",
+                    "downstream_package_name":"packit",
+                    "upstream_ref":"last_commit",
+                    "upstream_package_name":"packit_upstream",
+                    "create_tarball_command":[25],
+                    "allowed_gpg_keys":["gpg"],
+                    "dist_git_namespace":"awesome"
                 }
-            }
-            """,
-            "packit.json is valid and ready to be used",
+                """,
+            "* field create_tarball_command has an incorrect value:\n"
+            "** value at index 0: Not a valid string.",
         ),
         (
-            PackageConfig(config_file_path="packit.json",),
             """
-            {
-                "config_file_path": "packit.json",
-                "dist_git_base_url": "https://packit.dev/",
-                "downstream_package_name": "packit",
-                "upstream_ref": "last_commit",
-                "upstream_package_name": "packit_upstream",
-                "create_tarball_command": ["commands"],
-                "allowed_gpg_keys": ["gpg"],
-                "dist_git_namespace": "awesome",
-                "notifications": {
-                    "pull_request": {
-                        "successful_build": 55
-                    }
+                {
+                    "config_file_path":"packit.json",
+                    "dist_git_base_url":"https://packit.dev/",
+                    "downstream_package_name":"packit",
+                    "upstream_ref":"last_commit",
+                    "upstream_package_name":"packit_upstream",
+                    "create_tarball_command":["commands", True],
+                    "allowed_gpg_keys":["gpg"],
+                    "dist_git_namespace":"awesome"
                 }
-            }
-            """,
-            "packit.json does not pass validation:\n"
-            "* field notifications has incorrect values:\n"
-            "** field pull_request has incorrect values:\n"
-            "*** value at index successful_build: Not a valid boolean.",
+                """,
+            "* field create_tarball_command has an incorrect value:\n"
+            "** value at index 1: Not a valid string.",
         ),
     ],
+    ids=[
+        "valid_1",
+        "notif_succ_build",
+        "empty",
+        "valid_2",
+        "synced_files_src",
+        "synced_files_dest",
+        "valid_3",
+        "downstream_name",
+        "create_pr",
+        "valid_4",
+        "allowed_gpg",
+        "create_tarball_1",
+        "create_tarball_2",
+    ],
 )
-def test_schema_validation_notifications(
-    cwd_upstream, api_instance, package_config, raw_package_config, contains
-):
-    with cwd(DATA_DIR / "validate_config"):
-        u, d, api = api_instance
-        api.package_config = package_config
-
+def test_schema_validation(tmpdir, raw_package_config, expected_output):
+    with cwd(tmpdir):
         with open("packit.json", "w") as package_config_file:
             package_config_file.writelines(raw_package_config)
 
-        output = api.validate()
-        assert contains in output
-
-
-def test_schema_validation_config_name_missing(cwd_upstream, api_instance):
-    with cwd(DATA_DIR / "validate_config"):
-        u, d, api = api_instance
-        api.package_config = PackageConfig()
-
-        with open("packit.json", "w") as package_config_file:
-            package_config_file.writelines("{}")
-            try:
-                api.validate()
-            except PackitConfigException:
-                assert True
-            else:
-                assert False
+        api = PackitAPI(Config(), None)
+        output = api.validate_package_config(Path("."))
+        assert expected_output in output