From 5c2b4113097e63627818c5f586cbc053a058bd68 Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Dec 18 2018 08:09:16 +0000
Subject: List package name in failed state reason


Client tool like module-build-watch and `module-build --watch' could
output failed reason. However, original message "Some components failed
to buld" is not informative enough. Instead, list package names of
failed builds would be helpful.

Signed-off-by: Chenxiong Qi <cqi@redhat.com>

---

diff --git a/module_build_service/scheduler/handlers/components.py b/module_build_service/scheduler/handlers/components.py
index 291ff1f..4f7c8a7 100644
--- a/module_build_service/scheduler/handlers/components.py
+++ b/module_build_service/scheduler/handlers/components.py
@@ -100,8 +100,11 @@ def _finalize(config, session, msg, state):
         if failed_components_in_batch:
             log.info("Batch done, but not tagging because of failed component builds. Will "
                      "transition the module to \"failed\"")
-            parent.transition(config, state=models.BUILD_STATES['failed'],
-                              state_reason="Some components failed to build.")
+            state_reason = 'Component(s) {} failed to build.'.format(
+                ', '.join(c.package for c in failed_components_in_batch))
+            parent.transition(config,
+                              state=models.BUILD_STATES['failed'],
+                              state_reason=state_reason)
             session.commit()
             return []
         elif not built_components_in_batch:
diff --git a/module_build_service/scheduler/handlers/repos.py b/module_build_service/scheduler/handlers/repos.py
index b29cfa2..d04a3c9 100644
--- a/module_build_service/scheduler/handlers/repos.py
+++ b/module_build_service/scheduler/handlers/repos.py
@@ -87,14 +87,18 @@ def done(config, session, msg):
     # Assemble the list of all successful components in the batch.
     good = [c for c in current_batch if c.state == koji.BUILD_STATES['COMPLETE']]
 
+    failed_states = (koji.BUILD_STATES['FAILED'],
+                     koji.BUILD_STATES['CANCELED'])
+
     # If *none* of the components completed for this batch, then obviously the
     # module fails.  However!  We shouldn't reach this scenario.  There is
     # logic over in the component handler which should fail the module build
     # first before we ever get here.  This is here as a race condition safety
     # valve.
     if module_build.component_builds and not good:
-        module_build.transition(config, models.BUILD_STATES['failed'],
-                                "Some components failed to build.")
+        state_reason = 'Component(s) {} failed to build.'.format(
+            ', '.join(c.package for c in current_batch if c.state in failed_states))
+        module_build.transition(config, models.BUILD_STATES['failed'], state_reason)
         session.commit()
         log.warning("Odd!  All components in batch failed for %r." % module_build)
         return
@@ -121,8 +125,7 @@ def done(config, session, msg):
     for c in module_build.component_builds:
         if c.state in [None, koji.BUILD_STATES['BUILDING']]:
             has_unbuilt_components = True
-        elif (c.state in [koji.BUILD_STATES['FAILED'],
-                          koji.BUILD_STATES['CANCELED']]):
+        elif (c.state in failed_states):
             has_failed_components = True
 
     further_work = []
@@ -141,8 +144,13 @@ def done(config, session, msg):
 
     else:
         if has_failed_components:
-            module_build.transition(config, state=models.BUILD_STATES['failed'],
-                                    state_reason="Some components failed to build.")
+            state_reason = 'Component(s) {} failed to build.'.format(
+                ', '.join(c.package for c in module_build.component_builds
+                          if c.state in failed_states)
+            )
+            module_build.transition(config,
+                                    state=models.BUILD_STATES['failed'],
+                                    state_reason=state_reason)
         else:
             module_build.transition(config, state=models.BUILD_STATES['done'])
         session.commit()
diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py
index 93408e9..d8bd0dc 100644
--- a/tests/test_build/test_build.py
+++ b/tests/test_build/test_build.py
@@ -697,13 +697,13 @@ class TestBuild:
             # we had a failing component in batch 2.
             elif c.package == "tangerine":
                 assert c.state == koji.BUILD_STATES['FAILED']
-                assert c.state_reason == "Some components failed to build."
+                assert c.state_reason == "Component(s) perl-Tangerine failed to build."
             else:
                 assert c.state == koji.BUILD_STATES['COMPLETE']
 
             # Whole module should be failed.
             assert c.module_build.state == models.BUILD_STATES['failed']
-            assert c.module_build.state_reason == "Some components failed to build."
+            assert c.module_build.state_reason == "Component(s) perl-Tangerine failed to build."
 
             # We should end up with batch 2 and never start batch 3, because
             # there were failed components in batch 2.
@@ -750,7 +750,8 @@ class TestBuild:
 
             # Whole module should be failed.
             assert c.module_build.state == models.BUILD_STATES['failed']
-            assert c.module_build.state_reason == "Some components failed to build."
+            assert c.module_build.state_reason == \
+                "Component(s) perl-Tangerine, perl-List-Compare failed to build."
 
             # We should end up with batch 2 and never start batch 3, because
             # there were failed components in batch 2.