remove redundant _prepare_regexp (#1171)

change is_regex implementation logmatcher.match to read til EOF if there are more immediate lines regardless of timeout Co-authored-by: Pyifan <[email protected]>
morganstanley · Jan 9, 2025 · 922c584 · 922c584
1 parent 6b3d418
commit 922c584
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 27 deletions.
diff --git a/testplan/common/utils/comparison.py b/testplan/common/utils/comparison.py
@@ -14,7 +14,9 @@ def is_regex(obj):
     """
     Cannot do type check against SRE_Pattern, so we use duck typing.
     """
-    return hasattr(obj, "match") and hasattr(obj, "pattern")
+    import re
+
+    return isinstance(obj, re.Pattern)
 
 
 def basic_compare(first, second, strict=False):

diff --git a/testplan/common/utils/match.py b/testplan/common/utils/match.py
@@ -196,6 +196,8 @@ def _prepare_regexp(self, regexp: Regex) -> Pattern[AnyStr]:
 
         if isinstance(regexp, (str, bytes)):
             regexp = re.compile(regexp)
+        elif isinstance(regexp, re.Pattern):
+            pass
         else:
             try:
                 import rpyc
@@ -278,7 +280,6 @@ def _match(
         match = None
         start_time = time.time()
         end_time = start_time + timeout
-        regex = self._prepare_regexp(regex)
 
         with closing(self.log_stream) as log:
             log.seek(self.position)
@@ -301,13 +302,12 @@ def _match(
                     if match:
                         break
                 elif timeout > 0:
+                    if time.time() > end_time:
+                        break
                     time.sleep(LOG_MATCHER_INTERVAL)
                 else:
                     break
 
-                if timeout > 0 and time.time() > end_time:
-                    break
-
             self.position = self.log_stream.position
             if self._debug_info_e is None:
                 self._debug_info_e = (

diff --git a/tests/functional/testplan/runners/fixtures/assertions_passing/report.py b/tests/functional/testplan/runners/fixtures/assertions_passing/report.py
@@ -990,7 +990,7 @@
                                     "type": "LogfileMatch",
                                     "description": None,
                                     "passed": True,
-                                    "timeout": 1,
+                                    "timeout": 0.1,
                                     "results": [
                                         {
                                             "matched": "lime juice",
@@ -1007,7 +1007,7 @@
                                     "type": "LogfileMatch",
                                     "description": None,
                                     "passed": True,
-                                    "timeout": 1,
+                                    "timeout": 0.1,
                                     "results": [
                                         {
                                             "matched": "ginger beer",

diff --git a/tests/functional/testplan/runners/fixtures/assertions_passing/suites.py b/tests/functional/testplan/runners/fixtures/assertions_passing/suites.py
@@ -532,9 +532,9 @@ def test_logfile(self, env, result):
             f.write("vodka\n")
             f.write("lime juice\n")
             f.flush()
-            result.logfile.match(lm, r"lime juice", timeout=1)
+            result.logfile.match(lm, r"lime juice", timeout=0.1)
             result.logfile.seek_eof(lm)
-            with result.logfile.expect(lm, r"ginger beer", timeout=1):
+            with result.logfile.expect(lm, r"ginger beer", timeout=0.1):
                 f.write("ginger beer\n")
                 f.flush()
         finally:

diff --git a/tests/unit/testplan/common/utils/test_match.py b/tests/unit/testplan/common/utils/test_match.py
@@ -289,24 +289,12 @@ def construct_expected(slice):
 
     def test_match_large_file(self, large_logfile):
         """
-        Test matching the last entry in a large logfile, as a more realistic
-        test. The LogMatcher should quickly iterate through lines in the
-        logfile and return the match without timing out.
+        Test matching the last entry in a large logfile, the LogMatcher
+        shall iterate through lines in the logfile regardless of too small a timeout.
+        This avoids false alert when log file reading is slow due to machine load.
         """
         matcher = LogMatcher(log_path=large_logfile)
 
-        # Check that the LogMatcher can find the last 'Match me!' line in a
-        # reasonable length of time. 10s is a very generous timeout, most
-        # of the time it should complete in <1s.
-        match = matcher.match(
-            regex=r"^Match me!$", timeout=10, raise_on_timeout=False
-        )
-
-        assert match is not None
-        assert match.group(0) == "Match me!"
-
-        matcher.seek()
-
         # Check that the LogMatcher can find the last 'Match me!' line with
         # a whole-file scan.
         match = matcher.match(
@@ -318,13 +306,13 @@ def test_match_large_file(self, large_logfile):
 
         matcher.seek()
 
-        # Check that the LogMatcher will exit when timeout reaches while EOF
-        # not being met yet.
+        # Check that the LogMatcher will reach EOF regardless of timeout
         match = matcher.match(
             regex=r"^Match me!$", timeout=0.01, raise_on_timeout=False
         )
 
-        assert match is None
+        assert match is not None
+        assert match.group(0) == "Match me!"
 
     def test_scoped_match(self, rotating_logger, test_rotation):
         """unit test for expect api"""