Repository: packit-service/packit
Pull request #886 (2020-07-03 07:21:20)

package_config: set defaults correctly to all jobs
Fixes https://github.com/packit-service/packit-service/issues/714

configs are easy, right o_O  

So... Before this change, we first created PackageConfig (and copied all
attribs to all jobs) and then set all the defaults globally to that
PackageConfig instance -- obviously that doesn't work for the jobs, so
they didn't inherit the defaults.

With this change, we set the defaults in the raw_dict before creating
PackageConfig, hence defaults get processed correctly and set to all
jobs.

🍺


packit-as-a-service[bot]: Congratulations! One of the builds has completed. :champagne:

You can install the built RPMs by following these steps:

* `sudo yum install -y dnf-plugins-core` on RHEL 8
* `sudo dnf install -y dnf-plugins-core` on Fedora
* `dnf copr enable packit/packit-service-packit-886`
* And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.
softwarefactory-project-zuul[bot]: Build failed.

- [pre-commit ](https://softwarefactory-project.io/zuul/t/packit-service/build/ac0bc5636587458382fc1d3644918b7f) : FAILURE in 1m 07s
- [packit-tests-rpm ](https://softwarefactory-project.io/zuul/t/packit-service/build/0494befeeb754bf79728ff339c95c0fd) : SUCCESS in 7m 25s
- [packit-tests-pip-deps ](https://softwarefactory-project.io/zuul/t/packit-service/build/33daa957a59d449ba3a4451b1e7ed883) : SUCCESS in 7m 15s
- [packit-tests-git-master ](https://softwarefactory-project.io/zuul/t/packit-service/build/79bc2f67ce564bfea11e16557b940ceb) : SUCCESS in 7m 45s
- [packit-tests-rpm-sess-rec ](https://softwarefactory-project.io/zuul/t/packit-service/build/eabd1037dfb9476c9812b3b8daddf232) : SUCCESS in 2m 33s
- [packit-tests-pip-deps-sess-rec ](https://softwarefactory-project.io/zuul/t/packit-service/build/051ae924687c4e8cbdadfa87ba70a519) : SUCCESS in 2m 42s
- [packit-tests-git-master-sess-rec ](https://softwarefactory-project.io/zuul/t/packit-service/build/5cace413aadc4b3b8ae586d8973e0358) : SUCCESS in 3m 12s
- [reverse-dep-packit-service-tests ](https://softwarefactory-project.io/zuul/t/packit-service/build/75b9e295ba0947cbaca2470eba74fb51) : SUCCESS in 4m 45s

softwarefactory-project-zuul[bot]: Build failed.

- [pre-commit ](https://softwarefactory-project.io/zuul/t/packit-service/build/4e113d1e2c6c4fee98cd9d5694681731) : SUCCESS in 2m 39s
- [packit-tests-rpm ](https://softwarefactory-project.io/zuul/t/packit-service/build/3eacefb5c80c41feb89f417b5703bb70) : FAILURE in 7m 06s
- [packit-tests-pip-deps ](https://softwarefactory-project.io/zuul/t/packit-service/build/0c33dc280669439d8731e3a4fe46b08d) : FAILURE in 7m 25s
- [packit-tests-git-master ](https://softwarefactory-project.io/zuul/t/packit-service/build/a374d302f6d544ef800806929e6f6fe1) : FAILURE in 8m 43s
- [packit-tests-rpm-sess-rec ](https://softwarefactory-project.io/zuul/t/packit-service/build/6681555c79f141088925070558c5fbac) : SUCCESS in 2m 31s
- [packit-tests-pip-deps-sess-rec ](https://softwarefactory-project.io/zuul/t/packit-service/build/f387a3b87ee74de2ba89ea85a7fb0b7a) : SUCCESS in 3m 04s
- [packit-tests-git-master-sess-rec ](https://softwarefactory-project.io/zuul/t/packit-service/build/12752cb86eb84e02967b38e9e81fe796) : SUCCESS in 3m 02s
- [reverse-dep-packit-service-tests ](https://softwarefactory-project.io/zuul/t/packit-service/build/94b8f15611bc4f828aca7d779b87bc19) : FAILURE in 4m 56s


Patch:
diff --git a/packit/config/package_config.py b/packit/config/package_config.py
index e6599fcb..798a0774 100644
--- a/packit/config/package_config.py
+++ b/packit/config/package_config.py
@@ -129,23 +129,25 @@ class PackageConfig(CommonPackageConfig):
         if config_file_path and not raw_dict.get("config_file_path", None):
             raw_dict.update(config_file_path=config_file_path)
 
+        # we need to process defaults first so they get propagated to JobConfigs
+
         if "jobs" not in raw_dict:
             # we want default jobs to go through the proper parsing process
             raw_dict["jobs"] = get_default_jobs()
 
-        package_config = PackageConfigSchema().load_config(raw_dict)
-
-        if not getattr(package_config, "specfile_path", None):
+        if not raw_dict.get("specfile_path", None):
             if spec_file_path:
-                package_config.specfile_path = spec_file_path
+                raw_dict["specfile_path"] = spec_file_path
             else:
                 raise PackitConfigException("Spec file was not found!")
 
-        if not getattr(package_config, "upstream_package_name", None) and repo_name:
-            package_config.upstream_package_name = repo_name
+        if not raw_dict.get("upstream_package_name", None) and repo_name:
+            raw_dict["upstream_package_name"] = repo_name
 
-        if not getattr(package_config, "downstream_package_name", None) and repo_name:
-            package_config.downstream_package_name = repo_name
+        if not raw_dict.get("downstream_package_name", None) and repo_name:
+            raw_dict["downstream_package_name"] = repo_name
+
+        package_config = PackageConfigSchema().load_config(raw_dict)
 
         logger.debug(package_config)
         return package_config
diff --git a/packit/config/package_config_validator.py b/packit/config/package_config_validator.py
index ac97abcc..5e37047a 100644
--- a/packit/config/package_config_validator.py
+++ b/packit/config/package_config_validator.py
@@ -24,7 +24,7 @@ from typing import Dict, Union
 
 from marshmallow import ValidationError
 
-from packit.config.package_config import PackageConfig
+from packit.config.package_config import PackageConfig, get_local_specfile_path
 from packit.exceptions import PackitConfigException
 
 
@@ -40,7 +40,11 @@ class PackageConfigValidator:
         schema_errors = None
         try:
             PackageConfig.get_from_dict(
-                self.content, config_file_path=str(self.config_file_path)
+                self.content,
+                config_file_path=str(self.config_file_path),
+                spec_file_path=str(
+                    get_local_specfile_path(self.config_file_path.parent)
+                ),
             )
         except ValidationError as e:
             schema_errors = e.messages


Test patch:
diff --git a/tests/integration/test_validate_config.py b/tests/integration/test_validate_config.py
index 9614a195..d7a0df3e 100644
--- a/tests/integration/test_validate_config.py
+++ b/tests/integration/test_validate_config.py
@@ -23,7 +23,6 @@ from pathlib import Path
 import pytest
 
 from packit.api import PackitAPI
-from packit.config.config import Config
 from packit.utils import cwd
 
 
@@ -72,7 +71,7 @@ from packit.utils import cwd
             "** field pull_request has an incorrect value:\n"
             "*** value at index successful_build: Not a valid boolean.",
         ),
-        ("{}", "Spec file was not found!"),
+        ("{}", "packit.json is valid and ready to be used"),
         (
             """
             {
@@ -255,9 +254,8 @@ from packit.utils import cwd
 )
 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)
+        Path("packit.json").write_text(raw_package_config)
+        Path("packit.spec").write_text("hello")
 
-        api = PackitAPI(Config(), None)
-        output = api.validate_package_config(Path("."))
+        output = PackitAPI.validate_package_config(Path("."))
         assert expected_output in output
diff --git a/tests/unit/test_package_config.py b/tests/unit/test_package_config.py
index 9de55bf2..df249f0d 100644
--- a/tests/unit/test_package_config.py
+++ b/tests/unit/test_package_config.py
@@ -901,17 +901,30 @@ def test_get_package_config_from_repo(
     assert config.create_pr
 
 
-@pytest.mark.parametrize("content", ["{}"])
+@pytest.mark.parametrize(
+    "content",
+    [
+        "{}",
+        "{jobs: [{job: build, trigger: commit}]}",
+        "{downstream_package_name: horkyze, jobs: [{job: build, trigger: commit}]}",
+        "{upstream_package_name: slize, jobs: [{job: build, trigger: commit}]}",
+    ],
+)
 def test_get_package_config_from_repo_spec_file_not_defined(content):
+    specfile_path = "packit.spec"
     gp = flexmock(GitProject)
     gp.should_receive("full_repo_name").and_return("a/b")
     gp.should_receive("get_file_content").and_return(content)
-    gp.should_receive("get_files").and_return(["packit.spec"])
+    gp.should_receive("get_files").and_return([specfile_path])
     git_project = GitProject(repo="", service=GitService(), namespace="")
     config = get_package_config_from_repo(project=git_project, ref="")
     assert isinstance(config, PackageConfig)
-    assert Path(config.specfile_path).name == "packit.spec"
+    assert Path(config.specfile_path).name == specfile_path
     assert config.create_pr
+    for j in config.jobs:
+        assert j.specfile_path == specfile_path
+        assert j.downstream_package_name == config.downstream_package_name
+        assert j.upstream_package_name == config.upstream_package_name
 
 
 @pytest.mark.parametrize(