Revert "Fix #330 - Preserve load-list order"

Several issues have been reported about the run order and other odd
behavior in running of tests since the 4.0.0 release including a high
degree of random failures in stestr CI. #331 is the only likely
candidate as to the root cause. In the interest of fixing these issues
for people this commit reverts #331.

This revert was already released as part of 4.0.1. Since the main branch
adopted `black` code formatting recently it was easier to do the
straight revert on the stable branch first as there was conflicts.

This reverts commit ff25c99.

stestr/commands/run.py

@@ -367,20 +367,6 @@ def gather_errors(test_dict):
     return ids
 
 
-def _load_list_and_filter(list_filename, filter_ids):
-    """Load list of IDs and optionally filter them by a list of ids."""
-    list_ids = []
-    set_ids = set()
-    # Should perhaps be text.. currently does its own decode.
-    with open(list_filename, "rb") as list_file:
-        # Remove duplicates and filter out non failing tests when required
-        for test_id in parse_list(list_file.read()):
-            if test_id not in set_ids and (filter_ids is None or test_id in filter_ids):
-                list_ids.append(test_id)
-                set_ids.add(test_id)
-    return list_ids
-
-
 def run_command(
     config=".stestr.conf",
     repo_url=None,
@@ -646,7 +632,17 @@ def run_tests():
     else:
         ids = None
     if load_list:
-        ids = _load_list_and_filter(load_list, ids)
+        list_ids = set()
+        # Should perhaps be text.. currently does its own decode.
+        with open(load_list, "rb") as list_file:
+            list_ids = set(parse_list(list_file.read()))
+        if ids is None:
+            # Use the supplied list verbatim
+            ids = list_ids
+        else:
+            # We have some already limited set of ids, just reduce to ids
+            # that are both failing and listed.
+            ids = list_ids.intersection(ids)
 
     if config and os.path.isfile(config):
         conf = config_file.TestrConf(config)

stestr/selection.py

@@ -11,7 +11,6 @@
 # under the License.
 
 import contextlib
-import random
 import re
 import sys
 
@@ -93,12 +92,7 @@ def _get_regex_from_include_list(file_path):
 
 
 def construct_list(
-    test_ids,
-    regexes=None,
-    exclude_list=None,
-    include_list=None,
-    exclude_regex=None,
-    randomize=False,
+    test_ids, regexes=None, exclude_list=None, include_list=None, exclude_regex=None
 ):
     """Filters the discovered test cases
 
@@ -110,11 +104,10 @@ def construct_list(
     :param str exclude_list: The path to an exclusion_list file
     :param str include_list: The path to an inclusion_list file
     :param str exclude_regex: regex pattern to exclude tests
-    :param str randomize: Randomize the result
 
     :return: iterable of strings. The strings are full
         test_ids
-    :rtype: list
+    :rtype: set
     """
 
     if not regexes:
@@ -150,15 +143,20 @@ def construct_list(
             exclude_data = [record]
 
     list_of_test_cases = filter_tests(regexes, test_ids)
-
-    if exclude_data:
-        # NOTE(afazekas): We might use a faster logic when the
-        # print option is not requested
-        for (rex, msg, s_list) in exclude_data:
-            # NOTE(mtreinish): In the case of overlapping regex the test case
-            # might have already been removed from the set of tests
-            list_of_test_cases = [tc for tc in list_of_test_cases if not rex.search(tc)]
-    if randomize:
-        random.shuffle(list_of_test_cases)
-
-    return list_of_test_cases
+    set_of_test_cases = set(list_of_test_cases)
+
+    if not exclude_data:
+        return set_of_test_cases
+
+    # NOTE(afazekas): We might use a faster logic when the
+    # print option is not requested
+    for (rex, msg, s_list) in exclude_data:
+        for test_case in list_of_test_cases:
+            if rex.search(test_case):
+                # NOTE(mtreinish): In the case of overlapping regex the test
+                # case might have already been removed from the set of tests
+                if test_case in set_of_test_cases:
+                    set_of_test_cases.remove(test_case)
+                    s_list.append(test_case)
+
+    return set_of_test_cases

stestr/subunit_runner/program.py

@@ -22,7 +22,7 @@
 
 
 def filter_by_ids(suite_or_case, test_ids):
-    """Return a test suite with the intersection of suite_or_case and test_ids.
+    """Remove tests from suite_or_case where their id is not in test_ids.
 
     :param suite_or_case: A test suite or test case.
     :param test_ids: Something that supports the __contains__ protocol.
@@ -58,32 +58,23 @@ def filter_by_ids(suite_or_case, test_ids):
     thus the backwards-compatible code paths attempt to mutate in place rather
     than guessing how to reconstruct a new suite.
     """
-
-    def _filter_tests(to_filter, id_map):
-        # Compatible objects
-        if extras.safe_hasattr(to_filter, "filter_by_ids"):
-            res = to_filter.filter_by_ids(test_ids)
-            _filter_tests(res, id_map)
-        # TestCase objects.
-        elif extras.safe_hasattr(to_filter, "id"):
-            test_id = to_filter.id()
-            if test_id in test_ids:
-                id_map[test_id] = to_filter
-        # Standard TestSuites or derived classes.
-        elif isinstance(to_filter, unittest.TestSuite):
-            for item in to_filter:
-                _filter_tests(item, id_map)
-
-    id_map = {}
-    _filter_tests(suite_or_case, id_map)
-
-    result = unittest.TestSuite()
-    for test_id in test_ids:
-        if test_id in id_map:
-            # Use pop to avoid duplicates
-            result.addTest(id_map.pop(test_id))
-
-    return result
+    # Compatible objects
+    if extras.safe_hasattr(suite_or_case, "filter_by_ids"):
+        return suite_or_case.filter_by_ids(test_ids)
+    # TestCase objects.
+    if extras.safe_hasattr(suite_or_case, "id"):
+        if suite_or_case.id() in test_ids:
+            return suite_or_case
+        else:
+            return unittest.TestSuite()
+    # Standard TestSuites or derived classes [assumed to be mutable].
+    if isinstance(suite_or_case, unittest.TestSuite):
+        filtered = []
+        for item in suite_or_case:
+            filtered.append(filter_by_ids(item, test_ids))
+        suite_or_case._tests[:] = filtered
+    # Everything else:
+    return suite_or_case
 
 
 def iterate_tests(test_suite_or_case):
@@ -198,9 +189,8 @@ def __init__(
                 lines = source.readlines()
             finally:
                 source.close()
-            test_ids = [line.strip().decode("utf-8") for line in lines]
+            test_ids = {line.strip().decode("utf-8") for line in lines}
             self.test = filter_by_ids(self.test, test_ids)
-
         # XXX: Local edit (see http://bugs.python.org/issue22860)
         if not self.listtests:
             self.runTests()

stestr/test_processor.py

@@ -108,6 +108,7 @@ def __init__(
         self._listpath = listpath
         self.test_filters = test_filters
         self._group_callback = group_callback
+        self.worker_path = None
         self.worker_path = worker_path
         self.concurrency_value = concurrency
         self.exclude_list = exclude_list
@@ -158,15 +159,12 @@ def list_subst(match):
             name = ""
             idlist = ""
         else:
-            # Randomize now if it's not going to be partitioned
-            randomize = self.randomize and self.concurrency == 1
             self.test_ids = selection.construct_list(
                 self.test_ids,
                 exclude_list=self.exclude_list,
                 include_list=self.include_list,
                 regexes=self.test_filters,
                 exclude_regex=self.exclude_regex,
-                randomize=randomize,
             )
             name = self.make_listfile()
             variables["IDFILE"] = name

stestr/tests/test_load.py

@@ -13,14 +13,11 @@
 import io
 
 from stestr.commands import load
-from stestr import subunit_trace
 from stestr.tests import base
 
 
 class TestLoadCommand(base.TestCase):
     def test_empty_with_pretty_out(self):
-        # Clear results that may be there
-        subunit_trace.RESULTS.clear()
         stream = io.BytesIO()
         output = io.BytesIO()
         res = load.load(