twister: Adjust status for quarantined instances

Skipped status fits quarantined items better than filtered.
Filtered tests are by default removed from reports, which
shouldn't be the case for quarantined tests.
Adjust tests unit and blackbox tests accordingly.

Signed-off-by: Maciej Perkowski <maciej.perkowski@nordicsemi.no>

Author: PerMac

scripts/pylib/twister/twisterlib/reports.py

@@ -734,7 +734,7 @@ def summary(self, results, duration):
                     f'.'
                 )
 
-        built_only = results.total - run - results.filtered_configs
+        built_only = results.total - run - results.filtered_configs - results.skipped
         logger.info(
             f"{Fore.GREEN}{run}{Fore.RESET} test configurations executed on platforms,"
             f" {TwisterStatus.get_color(TwisterStatus.NOTRUN)}{built_only}{Fore.RESET}"

scripts/pylib/twister/twisterlib/runner.py

@@ -1885,7 +1885,7 @@ def run(self):
                     self.results.done -= self.results.error
                     self.results.error = 0
             else:
-                self.results.done = self.results.filtered_static
+                self.results.done = self.results.filtered_static + self.results.skipped
 
             self.execute(pipeline, done_queue)
 
@@ -1923,6 +1923,10 @@ def update_counting_before_pipeline(self):
                 self.results.filtered_configs_increment()
                 self.results.filtered_cases_increment(len(instance.testsuite.testcases))
                 self.results.cases_increment(len(instance.testsuite.testcases))
+            elif instance.status == TwisterStatus.SKIP and "overflow" not in instance.reason:
+                self.results.skipped_increment()
+                self.results.skipped_cases_increment(len(instance.testsuite.testcases))
+                self.results.cases_increment(len(instance.testsuite.testcases))
             elif instance.status == TwisterStatus.ERROR:
                 self.results.error_increment()
 

scripts/pylib/twister/twisterlib/testplan.py

@@ -605,10 +605,12 @@ def handle_quarantined_tests(self, instance: TestInstance, plat: Platform):
                 sim_name
             )
             if matched_quarantine and not self.options.quarantine_verify:
-                instance.add_filter("Quarantine: " + matched_quarantine, Filters.QUARANTINE)
+                instance.status = TwisterStatus.SKIP
+                instance.reason = "Quarantine: " + matched_quarantine
                 return
             if not matched_quarantine and self.options.quarantine_verify:
-                instance.add_filter("Not under quarantine", Filters.QUARANTINE)
+                instance.status = TwisterStatus.SKIP
+                instance.reason = "Not under quarantine"
 
     def load_from_file(self, file, filter_platform=None):
         if filter_platform is None:

scripts/pylib/twister/twisterlib/twister_main.py

@@ -104,20 +104,26 @@ def twister(options: argparse.Namespace, default_options: argparse.Namespace):
         logger.error(f"{e}")
         return 1
 
-    if options.verbose > 1:
-        # if we are using command line platform filter, no need to list every
-        # other platform as excluded, we know that already.
-        # Show only the discards that apply to the selected platforms on the
-        # command line
+    # if we are using command line platform filter, no need to list every
+    # other platform as excluded, we know that already.
+    # Show only the discards that apply to the selected platforms on the
+    # command line
 
+    if options.verbose > 0:
         for i in tplan.instances.values():
-            if i.status == TwisterStatus.FILTER:
+            if i.status in [TwisterStatus.SKIP,TwisterStatus.FILTER]:
                 if options.platform and not tplan.check_platform(i.platform, options.platform):
                     continue
+                # Filtered tests should be visable only when verbosity > 1
+                if options.verbose < 2 and i.status == TwisterStatus.FILTER:
+                    continue
+                res = i.reason
+                if "Quarantine" in i.reason:
+                    res = "Quarantined"
                 logger.info(
                     f"{i.platform.name:<25} {i.testsuite.name:<50}"
-                    f" {Fore.YELLOW}FILTERED{Fore.RESET}: {i.reason}"
-                )
+                    f" {Fore.YELLOW}{i.status.upper()}{Fore.RESET}: {res}"
+                    )
 
     report = Reporting(tplan, env)
     plan_file = os.path.join(options.outdir, "testplan.json")

scripts/tests/twister/test_runner.py

@@ -2545,6 +2545,8 @@ def mock_client_from_environ(jobs):
     results_mock().iteration = 0
     results_mock().failed = 2
     results_mock().total = 9
+    results_mock().filtered_static = 0
+    results_mock().skipped = 0
 
     def iteration_increment(value=1, decrement=False):
         results_mock().iteration += value * (-1 if decrement else 1)
@@ -2608,7 +2610,7 @@ def test_twisterrunner_update_counting_before_pipeline():
         ),
         'dummy5': mock.Mock(
             status=TwisterStatus.SKIP,
-            reason=None,
+            reason="Quarantine",
             testsuite=mock.Mock(
                 testcases=[mock.Mock()]
             )
@@ -2629,6 +2631,7 @@ def test_twisterrunner_update_counting_before_pipeline():
         error = 0,
         cases = 0,
         filtered_cases = 0,
+        skipped = 0,
         skipped_cases = 0,
         failed_cases = 0,
         error_cases = 0,
@@ -2652,14 +2655,22 @@ def cases_increment(value=1, decrement=False):
     def filtered_cases_increment(value=1, decrement=False):
         tr.results.filtered_cases += value * (-1 if decrement else 1)
     tr.results.filtered_cases_increment = filtered_cases_increment
+    def skipped_increment(value=1, decrement=False):
+        tr.results.skipped += value * (-1 if decrement else 1)
+    tr.results.skipped_increment = skipped_increment
+    def skipped_cases_increment(value=1, decrement=False):
+        tr.results.skipped_cases += value * (-1 if decrement else 1)
+    tr.results.skipped_cases_increment = skipped_cases_increment
 
     tr.update_counting_before_pipeline()
 
     assert tr.results.filtered_static == 1
     assert tr.results.filtered_configs == 1
     assert tr.results.filtered_cases == 4
-    assert tr.results.cases == 4
+    assert tr.results.cases == 5
     assert tr.results.error == 1
+    assert tr.results.skipped == 1
+    assert tr.results.skipped_cases == 1
 
 
 def test_twisterrunner_show_brief(caplog):

scripts/tests/twister/test_testplan.py

@@ -342,11 +342,11 @@ def test_quarantine_short(class_testplan, platforms_list, test_data,
             if testname in expected_val:
                 assert instance.status == TwisterStatus.NONE
             else:
-                assert instance.status == TwisterStatus.FILTER
+                assert instance.status == TwisterStatus.SKIP
                 assert instance.reason == "Not under quarantine"
         else:
             if testname in expected_val:
-                assert instance.status == TwisterStatus.FILTER
+                assert instance.status == TwisterStatus.SKIP
                 assert instance.reason == "Quarantine: " + expected_val[testname]
             else:
                 assert instance.status == TwisterStatus.NONE

scripts/tests/twister_blackbox/test_quarantine.py

@@ -15,6 +15,7 @@
 import json
 
 # pylint: disable=duplicate-code
+# pylint: disable=no-name-in-module
 from conftest import ZEPHYR_BASE, TEST_DATA, testsuite_filename_mock
 from twisterlib.testplan import TestPlan
 
@@ -49,10 +50,15 @@ def test_quarantine_verify(self, out_path):
 
         with open(os.path.join(out_path, 'testplan.json')) as f:
             j = json.load(f)
+
+        # Quarantine-verify "swaps" statuses. The ones that are in quarantine list
+        # should no longer be quarantined, and the ones that are not in the list
+        # should be quarantined. Remove "quarantined" tests from "verify" testplan
+        # to count what should be verified.
         filtered_j = [
-           (ts['platform'], ts['name'], tc['identifier']) \
+           (ts['platform'], ts['name']) \
                for ts in j['testsuites'] \
-               for tc in ts['testcases'] if 'reason' not in tc
+               if ts['status'] != "skipped"
         ]
 
         assert str(sys_exit.value) == '0'
@@ -89,26 +95,26 @@ def test_quarantine_list(self, capfd, out_path, test_path, test_platforms, quara
         sys.stdout.write(out)
         sys.stderr.write(err)
 
-        board1_match1 = re.search('agnostic/group2/dummy.agnostic.group2 FILTERED: Quarantine: test '
+        board1_match1 = re.search('agnostic/group2/dummy.agnostic.group2 SKIPPED: Quarantine: test '
                                'intel_adl_crb', err)
         board1_match2 = re.search(
-            'agnostic/group1/subgroup2/dummy.agnostic.group1.subgroup2 FILTERED: Quarantine: test '
+            'agnostic/group1/subgroup2/dummy.agnostic.group1.subgroup2 SKIPPED: Quarantine: test '
             'intel_adl_crb',
             err)
         qemu_64_match = re.search(
-            'agnostic/group1/subgroup2/dummy.agnostic.group1.subgroup2 FILTERED: Quarantine: test '
+            'agnostic/group1/subgroup2/dummy.agnostic.group1.subgroup2 SKIPPED: Quarantine: test '
             'qemu_x86_64',
             err)
         all_platforms_match = re.search(
-            'agnostic/group1/subgroup1/dummy.agnostic.group1.subgroup1 FILTERED: Quarantine: test '
+            'agnostic/group1/subgroup1/dummy.agnostic.group1.subgroup1 SKIPPED: Quarantine: test '
             'all platforms',
             err)
         all_platforms_match2 = re.search(
-            'agnostic/group1/subgroup1/dummy.agnostic.group1.subgroup1 FILTERED: Quarantine: test '
+            'agnostic/group1/subgroup1/dummy.agnostic.group1.subgroup1 SKIPPED: Quarantine: test '
             'all platforms',
             err)
         all_platforms_match3 = re.search(
-            'agnostic/group1/subgroup1/dummy.agnostic.group1.subgroup1 FILTERED: Quarantine: test '
+            'agnostic/group1/subgroup1/dummy.agnostic.group1.subgroup1 SKIPPED: Quarantine: test '
             'all platforms',
             err)