From 81d642578f1e0af421b00dc05be08acacdca266f Mon Sep 17 00:00:00 2001
From: Lukas Holecek <hluk@email.cz>
Date: Nov 26 2019 11:18:45 +0000
Subject: Add error count to summary and add error_reason in requirements


Fixes #488

JIRA: FACTORY-5181
Signed-off-by: Lukas Holecek <hluk@email.cz>

---

diff --git a/greenwave/policies.py b/greenwave/policies.py
index d9a70c4..7ed88b0 100644
--- a/greenwave/policies.py
+++ b/greenwave/policies.py
@@ -151,7 +151,7 @@ class TestResultMissingWaived(RuleSatisfied):
 class TestResultFailed(RuleNotSatisfied):
     """
     A required test case did not pass (that is, its outcome in ResultsDB was
-    not ``PASSED`` or ``INFO``) and no corresponding waiver was found.
+    not ``PASSED`` or ``INFO``).
     """
 
     def __init__(self, subject, test_case_name, scenario, result_id):
@@ -176,6 +176,43 @@ class TestResultFailed(RuleNotSatisfied):
         return TestResultPassed(self.test_case_name, self.result_id)
 
 
+class TestResultErrored(RuleNotSatisfied):
+    """
+    A required test case failed to finish, i.e. the system failed during the
+    testing process and could not finish the testing.  (outcome in ResultsDB
+    was ``ERROR``).
+    """
+
+    def __init__(
+            self,
+            subject,
+            test_case_name,
+            scenario,
+            result_id,
+            error_reason):
+        self.subject = subject
+        self.test_case_name = test_case_name
+        self.scenario = scenario
+        self.result_id = result_id
+        self.error_reason = error_reason
+
+    def to_json(self):
+        return {
+            'type': 'test-result-errored',
+            'testcase': self.test_case_name,
+            'result_id': self.result_id,
+            'error_reason': self.error_reason,
+            # These are for backwards compatibility only
+            # (the values are already visible in the result data itself, the
+            # caller shouldn't need them repeated here):
+            'item': self.subject.to_dict(),
+            'scenario': self.scenario,
+        }
+
+    def to_waived(self):
+        return TestResultPassed(self.test_case_name, self.result_id)
+
+
 class InvalidGatingYaml(RuleNotSatisfied):
     """
     Remote policy parsing failed.
@@ -266,19 +303,7 @@ class ExcludedInPolicy(RuleSatisfied):
         }
 
 
-def summarize_answers(answers):
-    """
-    Produces a one-sentence human-readable summary of the result of evaluating a policy.
-
-    Args:
-        answers (list): List of :py:class:`Answers <Answer>` from evaluating a policy.
-
-    Returns:
-        str: Human-readable summary.
-    """
-    if not answers:
-        return 'no tests are required'
-
+def _summarize_answers_without_errored(answers):
     failure_count = len([answer for answer in answers if isinstance(answer, RuleNotSatisfied)])
     missing_count = len([answer for answer in answers if isinstance(answer, TestResultMissing)])
 
@@ -303,6 +328,28 @@ def summarize_answers(answers):
     return 'inexplicable result'
 
 
+def summarize_answers(answers):
+    """
+    Produces a one-sentence human-readable summary of the result of evaluating a policy.
+
+    Args:
+        answers (list): List of :py:class:`Answers <Answer>` from evaluating a policy.
+
+    Returns:
+        str: Human-readable summary.
+    """
+    if not answers:
+        return 'no tests are required'
+
+    summary = _summarize_answers_without_errored(answers)
+
+    errored_count = len([answer for answer in answers if isinstance(answer, TestResultErrored)])
+    if errored_count:
+        summary += f' ({errored_count} {"error" if errored_count == 1 else "errors"})'
+
+    return summary
+
+
 class Rule(SafeYAMLObject):
     """
     An individual rule within a policy. A policy consists of multiple rules.
@@ -538,6 +585,16 @@ class PassingTestCaseRule(Rule):
                       'testcase %s, because the outcome is %s', subject,
                       self.test_case_name, result['outcome'])
             return TestResultMissing(subject, self.test_case_name, self.scenario)
+
+        if result['outcome'] == 'ERROR':
+            error_reason = result.get('error_reason')
+            log.debug('Test result ERROR for the %s and '
+                      'testcase %s, error reason: %s', subject,
+                      self.test_case_name, error_reason)
+            return TestResultErrored(
+                subject, self.test_case_name, self.scenario, result['id'],
+                error_reason)
+
         log.debug('Test result failed for the %s and '
                   'testcase %s, because the outcome is %s and it didn\'t match any of the '
                   'previous cases', subject, self.test_case_name, result['outcome'])
diff --git a/greenwave/tests/test_api_v1.py b/greenwave/tests/test_api_v1.py
index 0dc45b7..89e51e6 100644
--- a/greenwave/tests/test_api_v1.py
+++ b/greenwave/tests/test_api_v1.py
@@ -89,6 +89,15 @@ def test_make_decision_retrieves_waivers_omitted_on_passed(mock_results, mock_wa
     mock_waivers.assert_not_called()
 
 
+def test_make_decision_retrieves_waivers_on_errored(mock_results, mock_waivers):
+    mock_results.return_value = [make_result(outcome='ERROR')]
+    mock_waivers.return_value = []
+    response = make_decision()
+    assert 200 == response.status_code
+    assert '1 of 1 required tests failed (1 error)' == response.json['summary']
+    mock_waivers.assert_called_once()
+
+
 def test_make_decision_retrieves_waivers_once_on_verbose_and_missing(mock_results, mock_waivers):
     mock_results.return_value = []
     mock_waivers.return_value = []
diff --git a/greenwave/tests/test_summary.py b/greenwave/tests/test_summary.py
index c297905..ccda425 100644
--- a/greenwave/tests/test_summary.py
+++ b/greenwave/tests/test_summary.py
@@ -2,6 +2,7 @@
 from greenwave.policies import (
     summarize_answers,
     RuleSatisfied,
+    TestResultErrored,
     TestResultFailed,
     TestResultMissing,
     TestResultMissingWaived,
@@ -14,6 +15,8 @@ from greenwave.subjects.subject_type import GenericSubjectType
 
 testSubject = Subject(GenericSubjectType('koji_build'), 'nethack-1.2.3-1.el9000')
 testResultPassed = RuleSatisfied()
+testResultErrored = TestResultErrored(
+    testSubject, 'test', None, 1, 'some error')
 testResultFailed = TestResultFailed(
     testSubject, 'test', None, 1)
 testResultMissing = TestResultMissing(
@@ -57,6 +60,13 @@ def test_summary_missing_waived():
     assert summarize_answers(answers) == 'All required tests passed'
 
 
+def test_summary_errored():
+    answers = [
+        testResultErrored,
+    ]
+    assert summarize_answers(answers) == '1 of 1 required tests failed (1 error)'
+
+
 def test_summary_one_passed_one_failed():
     answers = [
         testResultPassed,
@@ -98,6 +108,17 @@ def test_summary_one_passed_one_failed_one_missing():
     assert summarize_answers(answers) == '1 of 3 required tests failed, 1 result missing'
 
 
+def test_summary_one_passed_one_failed_one_missing_two_errored():
+    answers = [
+        testResultErrored,
+        testResultPassed,
+        testResultFailed,
+        testResultMissing,
+        testResultErrored,
+    ]
+    assert summarize_answers(answers) == '3 of 5 required tests failed, 1 result missing (2 errors)'
+
+
 def test_summary_invalid_gating_yaml():
     answers = [
         testInvalidGatingYaml,

