Result model: Consider empty suite skipped, not passed.

pekkaklarck · pekkaklarck · commit fe1d8990392e · 2020-09-29T11:20:28.000+03:00
This makes suite status easier to understand and explain:

- If any fail -&gt; FAIL
- Else if any pass -&gt; PASS
- Else -&gt; SKIP
diff --git a/src/robot/result/model.py b/src/robot/result/model.py
@@ -226,7 +226,7 @@ def __init__(self, name='', doc='', metadata=None, source=None,
 
     @property
     def passed(self):
-        """``True`` if no test has failed, ``False`` otherwise."""
+        """``True`` if no test has failed but some have passed, ``False`` otherwise."""
         return self.status == 'PASS'
 
     @property
@@ -236,23 +236,22 @@ def failed(self):
 
     @property
     def skipped(self):
-        """``True`` if all tests have been skipped, ``False`` otherwise."""
+        """``True`` if there are no passed or failed tests, ``False`` otherwise."""
         return self.status == 'SKIP'
 
     @property
     def status(self):
         """'PASS', 'FAIL' or 'SKIP' depending on test statuses.
 
         - If any test has failed, status is 'FAIL'.
-        - If no test has failed but some has passed, status is 'PASS'.
-        - If all tests have been skipped, status is 'SKIP'.
+        - If no test has failed but at least some test has passed, status is 'PASS'.
+        - If there are no failed or passed tests, status is 'SKIP'. This covers both
+          the case when all tests have been skipped and when there are no tests.
         """
         stats = self.statistics.all    # Local variable avoids recreating stats.
         if stats.failed:
             return 'FAIL'
-        # TODO: Would 'SKIP' be better status if there are no executed tests?
-        # Update also docstring above accordingly.
-        if stats.total == 0 or stats.passed:
+        if stats.passed:
             return 'PASS'
         return 'SKIP'
 
diff --git a/utest/reporting/test_jsmodelbuilders.py b/utest/reporting/test_jsmodelbuilders.py
@@ -168,7 +168,7 @@ def _verify_status(self, model, status=0, start=None, elapsed=0):
         assert_equal(model, (status, start, elapsed))
 
     def _verify_suite(self, suite, name='', doc='', metadata=(), source='',
-                      relsource='', status=1, message='', start=None, elapsed=0,
+                      relsource='', status=3, message='', start=None, elapsed=0,
                       suites=(), tests=(), keywords=(), stats=(0, 0, 0, 0)):
         status = (status, start, elapsed, message) \
                 if message else (status, start, elapsed)
diff --git a/utest/result/test_resultmodel.py b/utest/result/test_resultmodel.py
@@ -45,8 +45,8 @@ def _create_suite_with_tests(self):
 
 class TestSuiteStatus(unittest.TestCase):
 
-    def test_suite_status_is_pass_by_default(self):
-        assert_equal(TestSuite().status, 'PASS')
+    def test_suite_status_is_skip_if_there_are_no_tests(self):
+        assert_equal(TestSuite().status, 'SKIP')
 
     def test_suite_status_is_fail_if_failed_test(self):
         suite = TestSuite()
@@ -67,7 +67,6 @@ def test_suite_status_is_pass_if_passed_and_skipped(self):
         suite = TestSuite()
         for i in range(5):
             suite.tests.create(status='PASS')
-        for i in range(5):
             suite.tests.create(status='SKIP')
         assert_equal(suite.status, 'PASS')
 
@@ -82,12 +81,14 @@ def test_suite_status_is_fail_if_failed_subsuite(self):
         suite = TestSuite()
         suite.suites.create().tests.create(status='FAIL')
         assert_equal(suite.status, 'FAIL')
+        suite.tests.create(status='PASS')
+        assert_equal(suite.status, 'FAIL')
 
     def test_passed_failed_skipped_propertys(self):
         suite = TestSuite()
-        assert_true(suite.passed)
+        assert_false(suite.passed)
         assert_false(suite.failed)
-        assert_false(suite.skipped)
+        assert_true(suite.skipped)
         suite.tests.create(status='SKIP')
         assert_false(suite.passed)
         assert_false(suite.failed)
