From aa7858b1672605e47912853bbb6bef2ac7b55c29 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Mar 27 2018 12:30:46 +0000 Subject: [PATCH 1/2] Create a custom FedoraAtomicCi rule The issue is that bodhi always provides an NVR but if the package is not being considered by the Fedora Atomic CI pipeline, its corresponding git repository isn't being clone and its `original_spec_nvr` value not included in the `pipeline.package.ignore` fedmsg message. In other words, relying on `pipeline.package.ignore` messages when querying info about a particular NVR is never going to work. With this rule we are specifying a list of packages of interest (to be filled) and a test_case_name. Every package that is part of the list of interest must satisfy this test case, all the others are considered satisfying the rule. Signed-off-by: Pierre-Yves Chibon Signed-off-by: Ralph Bean --- diff --git a/greenwave/policies.py b/greenwave/policies.py index af8de2d..67f9159 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -199,6 +199,55 @@ class PassingTestCaseRule(Rule): } +class FedoraAtomicCi(Rule): + """ + This rule requires that the value of the specified field is part of a + specified list. + """ + yaml_tag = u'!FedoraAtomicCi' + yaml_loader = yaml.SafeLoader + + def __init__(self, test_case_name, repos): + self.test_case_name = test_case_name + self.repos = repos + + def check(self, item, results, waivers): + """ Check that the request satisfies the requirement of the Fedora + Atomic CI pipeline. + + If the request (item) corresponds to a request for Fedora Atomic CI + results request and if it is the case, check that the package for + which this request is, is in the allowed list. + If it is, then proceed as usual using the specified test_case_name. + If the package is not in the list, then consider this requirement + moot and satisfied. + + """ + + if 'original_spec_nvr' not in item: + return RuleSatisfied() + + nvr = item['original_spec_nvr'] + pkg_name = nvr.rsplit('-', 2)[0] + if pkg_name not in self.repos: + return RuleSatisfied() + + rule = PassingTestCaseRule() + rule.test_case_name = self.test_case_name + return rule.check(item, results, waivers) + + def __repr__(self): + return "%s(test_case_name=%s, repos=%r)" % ( + self.__class__.__name__, self.test_case_name, self.repos) + + def to_json(self): + return { + 'rule': self.__class__.__name__, + 'test_case_name': self.test_case_name, + 'repos': self.repos, + } + + class Policy(yaml.YAMLObject): yaml_tag = u'!Policy' yaml_loader = yaml.SafeLoader From 5d220953a7e6252084c9ae1dc962cdcabeea4135 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Mar 27 2018 12:30:46 +0000 Subject: [PATCH 2/2] Abstract out the PackageSpecificRule to be applicable to both Fedora Atomic CI and OSCI. --- diff --git a/greenwave/policies.py b/greenwave/policies.py index 67f9159..18ffa55 100644 --- a/greenwave/policies.py +++ b/greenwave/policies.py @@ -199,35 +199,34 @@ class PassingTestCaseRule(Rule): } -class FedoraAtomicCi(Rule): +class PackageSpecificRule(Rule): """ - This rule requires that the value of the specified field is part of a - specified list. + This rule only applies itself to results which are for its configured + list of packages (called "repos"). + + This intermediary class should be considered abstract, and not used directly. """ - yaml_tag = u'!FedoraAtomicCi' - yaml_loader = yaml.SafeLoader def __init__(self, test_case_name, repos): self.test_case_name = test_case_name self.repos = repos def check(self, item, results, waivers): - """ Check that the request satisfies the requirement of the Fedora - Atomic CI pipeline. + """ Check that the item passes testcase for the given results, but + only if the item is an instance of a package name configured for + this rule (specified by "repos"). - If the request (item) corresponds to a request for Fedora Atomic CI - results request and if it is the case, check that the package for - which this request is, is in the allowed list. - If it is, then proceed as usual using the specified test_case_name. - If the package is not in the list, then consider this requirement - moot and satisfied. + Items which do not bear the "nvr_key" for this rule are considered + satisfied (ignored). + Items whose package names (extracted from their NVR) do not appear + in the "repos" list of this rule are considered satisfied (ignored). """ - if 'original_spec_nvr' not in item: + if self.nvr_key not in item: return RuleSatisfied() - nvr = item['original_spec_nvr'] + nvr = item[self.nvr_key] pkg_name = nvr.rsplit('-', 2)[0] if pkg_name not in self.repos: return RuleSatisfied() @@ -245,9 +244,22 @@ class FedoraAtomicCi(Rule): 'rule': self.__class__.__name__, 'test_case_name': self.test_case_name, 'repos': self.repos, + 'nvr_key': self.nvr_key, } +class FedoraAtomicCi(PackageSpecificRule): + yaml_tag = u'!FedoraAtomicCi' + yaml_loader = yaml.SafeLoader + nvr_key = 'original_spec_nvr' + + +class PackageSpecificBuild(PackageSpecificRule): + yaml_tag = u'!PackageSpecificBuild' + yaml_loader = yaml.SafeLoader + nvr_key = 'item' + + class Policy(yaml.YAMLObject): yaml_tag = u'!Policy' yaml_loader = yaml.SafeLoader diff --git a/greenwave/tests/test_policies.py b/greenwave/tests/test_policies.py index cbe5493..500550f 100644 --- a/greenwave/tests/test_policies.py +++ b/greenwave/tests/test_policies.py @@ -57,6 +57,74 @@ rules: assert isinstance(decision[0], RuleSatisfied) +def test_package_specific_rule(tmpdir): + p = tmpdir.join('fedora.yaml') + p.write(""" +--- !Policy +id: "some_policy" +product_versions: + - rhel-9000 +decision_context: compose_gate +blacklist: [] +rules: + - !PackageSpecificBuild {test_case_name: sometest, repos: [nethack]} + """) + policies = load_policies(tmpdir.strpath) + policy = policies[0] + + # Ensure that we fail with no results + item = {'item': 'nethack-1.2.3-1.el9000', 'type': 'koji_build'} + results, waivers = [], [] + decision = policy.check(item, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], TestResultMissing) + + # That a matching, failing result can fail + results = [{ + 'id': 123, + 'item': 'nethack-1.2.3-1.el9000', + 'testcase': {'name': 'sometest'}, + 'outcome': 'FAILED', + }] + decision = policy.check(item, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], TestResultFailed) + + # That a matching, passing result can pass + results = [{ + 'id': 123, + 'item': 'nethack-1.2.3-1.el9000', + 'testcase': {'name': 'sometest'}, + 'outcome': 'PASSED', + }] + decision = policy.check(item, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], RuleSatisfied) + + # That a non-matching passing result is ignored. + item = {'item': 'foobar-1.2.3-1.el9000', 'type': 'koji_build'} + results = [{ + 'id': 123, + 'item': 'foobar-1.2.3-1.el9000', + 'testcase': {'name': 'sometest'}, + 'outcome': 'PASSED', + }] + decision = policy.check(item, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], RuleSatisfied) + + # That a non-matching failing result is ignored. + results = [{ + 'id': 123, + 'item': 'foobar-1.2.3-1.el9000', + 'testcase': {'name': 'sometest'}, + 'outcome': 'FAILED', + }] + decision = policy.check(item, results, waivers) + assert len(decision) == 1 + assert isinstance(decision[0], RuleSatisfied) # ooooh. + + def test_load_policies(): app = create_app('greenwave.config.TestingConfig') assert len(app.config['policies']) > 0