From f4db824682cd228889f0acebab751e0651b8e394 Mon Sep 17 00:00:00 2001
From: Iryna Shcherbina <ishcherb@redhat.com>
Date: Wed, 31 May 2017 22:03:23 +0200
Subject: [PATCH] Improve artifact report

* Better formatting for requires of each package
* Source rpm result (BuildRequires) on top

Also fixes an important bug, when rpm and srpm have the same NVR.
Previously it generated one result for both packages in such cases.
Now BuildRequires are displayed separately.
This was cought by the integration test.

Fixes #20
---
 python_versions_check.py              |  4 ++--
 taskotron_python_versions/common.py   |  4 ++++
 taskotron_python_versions/requires.py | 22 ++++++++++++----------
 test/integration/test_integration.py  | 12 +++++++-----
 4 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/python_versions_check.py b/python_versions_check.py
index b03882b..987c56a 100644
--- a/python_versions_check.py
+++ b/python_versions_check.py
@@ -37,7 +37,7 @@ def run(koji_build, workdir='.', artifactsdir='artifacts'):
             except PackageException as err:
                 log.error('{}: {}'.format(file_, err))
             else:
-                if file_.endswith('.src.rpm'):
+                if package.is_srpm:
                     srpm_packages.append(package)
                 else:
                     packages.append(package)
@@ -54,7 +54,7 @@ def run(koji_build, workdir='.', artifactsdir='artifacts'):
     details.append(task_two_three(packages, koji_build, artifact))
     details.append(task_naming_scheme(packages, koji_build, artifact))
     details.append(task_requires_naming_scheme(
-        packages + srpm_packages, koji_build, artifact))
+        srpm_packages + packages, koji_build, artifact))
 
     # finally, the main detail with overall results
     outcome = 'PASSED'
diff --git a/taskotron_python_versions/common.py b/taskotron_python_versions/common.py
index ad57be9..ff97bc6 100644
--- a/taskotron_python_versions/common.py
+++ b/taskotron_python_versions/common.py
@@ -56,6 +56,10 @@ def __init__(self, path):
             except rpm.error as err:
                 raise PackageException('{}: {}'.format(self.filename, err))
 
+    @property
+    def is_srpm(self):
+        return self.filename.endswith('.src.rpm')
+
     @property
     def name(self):
         """Package name as a string."""
diff --git a/taskotron_python_versions/requires.py b/taskotron_python_versions/requires.py
index 6fa175d..1b84a41 100644
--- a/taskotron_python_versions/requires.py
+++ b/taskotron_python_versions/requires.py
@@ -1,5 +1,3 @@
-import collections
-
 import dnf
 
 from .common import log, write_to_artifact
@@ -120,20 +118,24 @@ def task_requires_naming_scheme(packages, koji_build, artifact):
     repoquery = get_dnf_query(fedora_release)
 
     outcome = 'PASSED'
-    misnamed_requires = collections.defaultdict(set)
+
+    problem_rpms = set()
+    message_rpms = ''
 
     for package in packages:
         log.debug('Checking requires of {}'.format(package.filename))
 
         requires = check_requires_naming_scheme(package, repoquery)
         if requires:
-            misnamed_requires[package.nvr].update(requires)
             outcome = 'FAILED'
+            problem_rpms.add(package.nvr)
 
-    message_rpms = ''
-    for package_name, requires in misnamed_requires.items():
-        message_rpms += '{}\n * Requires: {}\n'.format(
-            package_name, ', '.join(sorted(requires)))
+            message = '\n{} {}Requires:\n * {}\n'.format(
+                package.nvr,
+                'Build' if package.is_srpm else '',
+                '\n * '.join(sorted(requires)))
+            if message not in message_rpms:
+                message_rpms += message
 
     detail = check.CheckDetail(
         checkname='python-versions.requires_naming_scheme',
@@ -141,10 +143,10 @@ def task_requires_naming_scheme(packages, koji_build, artifact):
         report_type=check.ReportType.KOJI_BUILD,
         outcome=outcome)
 
-    if misnamed_requires:
+    if problem_rpms:
         detail.artifact = artifact
         write_to_artifact(artifact, MESSAGE.format(message_rpms), INFO_URL)
-        problems = 'Problematic RPMs:\n' + ', '.join(misnamed_requires.keys())
+        problems = 'Problematic RPMs:\n' + ', '.join(problem_rpms)
     else:
         problems = 'No problems found.'
 
diff --git a/test/integration/test_integration.py b/test/integration/test_integration.py
index 3f4e452..0e11b50 100644
--- a/test/integration/test_integration.py
+++ b/test/integration/test_integration.py
@@ -201,14 +201,16 @@ def test_artifact_contains_requires_naming_scheme_and_looks_as_expected(
 
     print(artifact)
 
-    expected_requires = 'python-psutil (python2-psutil is available)'
-
     assert dedent("""
         These RPMs use `python-` prefix without Python version in *Requires:
-        {}
-         * Requires: {}
+
+        tracer-0.6.9-1.fc23 BuildRequires:
+         * python-psutil (python2-psutil is available)
+
+        tracer-0.6.9-1.fc23 Requires:
+         * python-psutil (python2-psutil is available)
 
         This is strongly discouraged and should be avoided. Please check
         the required packages, and use names with either `python2-` or
         `python3-` prefix.
-    """).strip().format(result.item, expected_requires) in artifact.strip()
+    """).strip() in artifact.strip()