llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) <details> <summary>Changes</summary> Various DAP tests are specifying their own timeouts, with values ranging from "1" to "20". Most of them seem arbitrary, but some come with a comment. The performance characters of running these tests in CI are unpredictable (they generally run much slower than developers expect) and really not something we can make assumptions about. I suspect these timeouts are a contributing factor to the flakiness of the DAP tests. This PR unifies the timeouts around a central value in the DAP server. Fixes #<!-- -->162523 --- Patch is 22.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/163292.diff 9 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+25-26) - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+30-49) - (modified) lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py (+2-4) - (modified) lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py (+2-2) - (modified) lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py (-2) - (modified) lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py (+4-8) - (modified) lldb/test/API/tools/lldb-dap/module/TestDAP_module.py (+2-2) - (modified) lldb/test/API/tools/lldb-dap/output/TestDAP_output.py (+1-1) - (modified) lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py (+1-1) ``````````diff diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index 8eb64b4ab8b2b..7ce90bdf0d362 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -27,6 +27,10 @@ Literal, ) +# set timeout based on whether ASAN was enabled or not. Increase +# timeout by a factor of 10 if ASAN is enabled. +DEFAULT_TIMEOUT = 10 * (10 if ("ASAN_OPTIONS" in os.environ) else 1) + ## DAP type references @@ -282,26 +286,24 @@ def get_output(self, category: str, clear=True) -> str: def collect_output( self, category: str, - timeout: float, pattern: Optional[str] = None, clear=True, ) -> str: """Collect output from 'output' events. Args: category: The category to collect. - timeout: The max duration for collecting output. pattern: Optional, if set, return once this pattern is detected in the collected output. Returns: The collected output. """ - deadline = time.monotonic() + timeout + deadline = time.monotonic() + DEFAULT_TIMEOUT output = self.get_output(category, clear) while deadline >= time.monotonic() and ( pattern is None or pattern not in output ): - event = self.wait_for_event(["output"], timeout=deadline - time.monotonic()) + event = self.wait_for_event(["output"]) if not event: # Timeout or EOF break output += self.get_output(category, clear=clear) @@ -555,25 +557,20 @@ def predicate(p: ProtocolMessage): return cast(Optional[Response], self._recv_packet(predicate=predicate)) - def wait_for_event( - self, filter: List[str] = [], timeout: Optional[float] = None - ) -> Optional[Event]: + def wait_for_event(self, filter: List[str] = []) -> Optional[Event]: """Wait for the first event that matches the filter.""" def predicate(p: ProtocolMessage): return p["type"] == "event" and p["event"] in filter return cast( - Optional[Event], self._recv_packet(predicate=predicate, timeout=timeout) + Optional[Event], + self._recv_packet(predicate=predicate, timeout=DEFAULT_TIMEOUT), ) - def wait_for_stopped( - self, timeout: Optional[float] = None - ) -> Optional[List[Event]]: + def wait_for_stopped(self) -> Optional[List[Event]]: stopped_events = [] - stopped_event = self.wait_for_event( - filter=["stopped", "exited"], timeout=timeout - ) + stopped_event = self.wait_for_event(filter=["stopped", "exited"]) while stopped_event: stopped_events.append(stopped_event) # If we exited, then we are done @@ -582,26 +579,28 @@ def wait_for_stopped( # Otherwise we stopped and there might be one or more 'stopped' # events for each thread that stopped with a reason, so keep # checking for more 'stopped' events and return all of them - stopped_event = self.wait_for_event( - filter=["stopped", "exited"], timeout=0.25 + # Use a shorter timeout for additional stopped events + def predicate(p: ProtocolMessage): + return p["type"] == "event" and p["event"] in ["stopped", "exited"] + + stopped_event = cast( + Optional[Event], self._recv_packet(predicate=predicate, timeout=0.25) ) return stopped_events - def wait_for_breakpoint_events(self, timeout: Optional[float] = None): + def wait_for_breakpoint_events(self): breakpoint_events: list[Event] = [] while True: - event = self.wait_for_event(["breakpoint"], timeout=timeout) + event = self.wait_for_event(["breakpoint"]) if not event: break breakpoint_events.append(event) return breakpoint_events - def wait_for_breakpoints_to_be_verified( - self, breakpoint_ids: list[str], timeout: Optional[float] = None - ): + def wait_for_breakpoints_to_be_verified(self, breakpoint_ids: list[str]): """Wait for all breakpoints to be verified. Return all unverified breakpoints.""" while any(id not in self.resolved_breakpoints for id in breakpoint_ids): - breakpoint_event = self.wait_for_event(["breakpoint"], timeout=timeout) + breakpoint_event = self.wait_for_event(["breakpoint"]) if breakpoint_event is None: break @@ -614,14 +613,14 @@ def wait_for_breakpoints_to_be_verified( ) ] - def wait_for_exited(self, timeout: Optional[float] = None): - event_dict = self.wait_for_event(["exited"], timeout=timeout) + def wait_for_exited(self): + event_dict = self.wait_for_event(["exited"]) if event_dict is None: raise ValueError("didn't get exited event") return event_dict - def wait_for_terminated(self, timeout: Optional[float] = None): - event_dict = self.wait_for_event(["terminated"], timeout) + def wait_for_terminated(self): + event_dict = self.wait_for_event(["terminated"]) if event_dict is None: raise ValueError("didn't get terminated event") return event_dict diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index f7b1ed80fceb5..29935bb8046ff 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -18,7 +18,7 @@ class DAPTestCaseBase(TestBase): # set timeout based on whether ASAN was enabled or not. Increase # timeout by a factor of 10 if ASAN is enabled. - DEFAULT_TIMEOUT = 10 * (10 if ("ASAN_OPTIONS" in os.environ) else 1) + DEFAULT_TIMEOUT = dap_server.DEFAULT_TIMEOUT NO_DEBUG_INFO_TESTCASE = True def create_debug_adapter( @@ -118,11 +118,9 @@ def set_function_breakpoints( self.wait_for_breakpoints_to_resolve(breakpoint_ids) return breakpoint_ids - def wait_for_breakpoints_to_resolve( - self, breakpoint_ids: list[str], timeout: Optional[float] = DEFAULT_TIMEOUT - ): + def wait_for_breakpoints_to_resolve(self, breakpoint_ids: list[str]): unresolved_breakpoints = self.dap_server.wait_for_breakpoints_to_be_verified( - breakpoint_ids, timeout + breakpoint_ids ) self.assertEqual( len(unresolved_breakpoints), @@ -134,11 +132,10 @@ def wait_until( self, predicate: Callable[[], bool], delay: float = 0.5, - timeout: float = DEFAULT_TIMEOUT, ) -> bool: """Repeatedly run the predicate until either the predicate returns True or a timeout has occurred.""" - deadline = time.monotonic() + timeout + deadline = time.monotonic() + self.DEFAULT_TIMEOUT while deadline > time.monotonic(): if predicate(): return True @@ -155,15 +152,13 @@ def assertCapabilityIsNotSet(self, key: str, msg: Optional[str] = None) -> None: if key in self.dap_server.capabilities: self.assertEqual(self.dap_server.capabilities[key], False, msg) - def verify_breakpoint_hit( - self, breakpoint_ids: List[Union[int, str]], timeout: float = DEFAULT_TIMEOUT - ): + def verify_breakpoint_hit(self, breakpoint_ids: List[Union[int, str]]): """Wait for the process we are debugging to stop, and verify we hit any breakpoint location in the "breakpoint_ids" array. "breakpoint_ids" should be a list of breakpoint ID strings (["1", "2"]). The return value from self.set_source_breakpoints() or self.set_function_breakpoints() can be passed to this function""" - stopped_events = self.dap_server.wait_for_stopped(timeout) + stopped_events = self.dap_server.wait_for_stopped() normalized_bp_ids = [str(b) for b in breakpoint_ids] for stopped_event in stopped_events: if "body" in stopped_event: @@ -186,11 +181,11 @@ def verify_breakpoint_hit( f"breakpoint not hit, wanted breakpoint_ids {breakpoint_ids} in stopped_events {stopped_events}", ) - def verify_all_breakpoints_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): + def verify_all_breakpoints_hit(self, breakpoint_ids): """Wait for the process we are debugging to stop, and verify we hit all of the breakpoint locations in the "breakpoint_ids" array. "breakpoint_ids" should be a list of int breakpoint IDs ([1, 2]).""" - stopped_events = self.dap_server.wait_for_stopped(timeout) + stopped_events = self.dap_server.wait_for_stopped() for stopped_event in stopped_events: if "body" in stopped_event: body = stopped_event["body"] @@ -208,12 +203,12 @@ def verify_all_breakpoints_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): return self.assertTrue(False, f"breakpoints not hit, stopped_events={stopped_events}") - def verify_stop_exception_info(self, expected_description, timeout=DEFAULT_TIMEOUT): + def verify_stop_exception_info(self, expected_description): """Wait for the process we are debugging to stop, and verify the stop reason is 'exception' and that the description matches 'expected_description' """ - stopped_events = self.dap_server.wait_for_stopped(timeout) + stopped_events = self.dap_server.wait_for_stopped() for stopped_event in stopped_events: if "body" in stopped_event: body = stopped_event["body"] @@ -338,26 +333,14 @@ def get_console(self): def get_important(self): return self.dap_server.get_output("important") - def collect_stdout( - self, timeout: float = DEFAULT_TIMEOUT, pattern: Optional[str] = None - ) -> str: - return self.dap_server.collect_output( - "stdout", timeout=timeout, pattern=pattern - ) + def collect_stdout(self, pattern: Optional[str] = None) -> str: + return self.dap_server.collect_output("stdout", pattern=pattern) - def collect_console( - self, timeout: float = DEFAULT_TIMEOUT, pattern: Optional[str] = None - ) -> str: - return self.dap_server.collect_output( - "console", timeout=timeout, pattern=pattern - ) + def collect_console(self, pattern: Optional[str] = None) -> str: + return self.dap_server.collect_output("console", pattern=pattern) - def collect_important( - self, timeout: float = DEFAULT_TIMEOUT, pattern: Optional[str] = None - ) -> str: - return self.dap_server.collect_output( - "important", timeout=timeout, pattern=pattern - ) + def collect_important(self, pattern: Optional[str] = None) -> str: + return self.dap_server.collect_output("important", pattern=pattern) def get_local_as_int(self, name, threadId=None): value = self.dap_server.get_local_variable_value(name, threadId=threadId) @@ -393,14 +376,13 @@ def stepIn( targetId=None, waitForStop=True, granularity="statement", - timeout=DEFAULT_TIMEOUT, ): response = self.dap_server.request_stepIn( threadId=threadId, targetId=targetId, granularity=granularity ) self.assertTrue(response["success"]) if waitForStop: - return self.dap_server.wait_for_stopped(timeout) + return self.dap_server.wait_for_stopped() return None def stepOver( @@ -408,7 +390,6 @@ def stepOver( threadId=None, waitForStop=True, granularity="statement", - timeout=DEFAULT_TIMEOUT, ): response = self.dap_server.request_next( threadId=threadId, granularity=granularity @@ -417,40 +398,40 @@ def stepOver( response["success"], f"next request failed: response {response}" ) if waitForStop: - return self.dap_server.wait_for_stopped(timeout) + return self.dap_server.wait_for_stopped() return None - def stepOut(self, threadId=None, waitForStop=True, timeout=DEFAULT_TIMEOUT): + def stepOut(self, threadId=None, waitForStop=True): self.dap_server.request_stepOut(threadId=threadId) if waitForStop: - return self.dap_server.wait_for_stopped(timeout) + return self.dap_server.wait_for_stopped() return None def do_continue(self): # `continue` is a keyword. resp = self.dap_server.request_continue() self.assertTrue(resp["success"], f"continue request failed: {resp}") - def continue_to_next_stop(self, timeout=DEFAULT_TIMEOUT): + def continue_to_next_stop(self): self.do_continue() - return self.dap_server.wait_for_stopped(timeout) + return self.dap_server.wait_for_stopped() - def continue_to_breakpoint(self, breakpoint_id: str, timeout=DEFAULT_TIMEOUT): - self.continue_to_breakpoints((breakpoint_id), timeout) + def continue_to_breakpoint(self, breakpoint_id: str): + self.continue_to_breakpoints((breakpoint_id)) - def continue_to_breakpoints(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT): + def continue_to_breakpoints(self, breakpoint_ids): self.do_continue() - self.verify_breakpoint_hit(breakpoint_ids, timeout) + self.verify_breakpoint_hit(breakpoint_ids) - def continue_to_exception_breakpoint(self, filter_label, timeout=DEFAULT_TIMEOUT): + def continue_to_exception_breakpoint(self, filter_label): self.do_continue() self.assertTrue( - self.verify_stop_exception_info(filter_label, timeout), + self.verify_stop_exception_info(filter_label), 'verify we got "%s"' % (filter_label), ) - def continue_to_exit(self, exitCode=0, timeout=DEFAULT_TIMEOUT): + def continue_to_exit(self, exitCode=0): self.do_continue() - stopped_events = self.dap_server.wait_for_stopped(timeout) + stopped_events = self.dap_server.wait_for_stopped() self.assertEqual( len(stopped_events), 1, "stopped_events = {}".format(stopped_events) ) diff --git a/lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py b/lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py index ed373f2c427a9..9e29f07db80f1 100644 --- a/lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py +++ b/lldb/test/API/tools/lldb-dap/attach-commands/TestDAP_attachCommands.py @@ -71,7 +71,7 @@ def test_commands(self): breakpoint_ids = self.set_function_breakpoints(functions) self.assertEqual(len(breakpoint_ids), len(functions), "expect one breakpoint") self.continue_to_breakpoints(breakpoint_ids) - output = self.collect_console(timeout=10, pattern=stopCommands[-1]) + output = self.collect_console(pattern=stopCommands[-1]) self.verify_commands("stopCommands", output, stopCommands) # Continue after launch and hit the "pause()" call and stop the target. @@ -81,7 +81,7 @@ def test_commands(self): time.sleep(0.5) self.dap_server.request_pause() self.dap_server.wait_for_stopped() - output = self.collect_console(timeout=10, pattern=stopCommands[-1]) + output = self.collect_console(pattern=stopCommands[-1]) self.verify_commands("stopCommands", output, stopCommands) # Continue until the program exits @@ -90,7 +90,6 @@ def test_commands(self): # "exitCommands" that were run after the second breakpoint was hit # and the "terminateCommands" due to the debugging session ending output = self.collect_console( - timeout=10.0, pattern=terminateCommands[0], ) self.verify_commands("exitCommands", output, exitCommands) @@ -141,7 +140,6 @@ def test_terminate_commands(self): # "terminateCommands" self.dap_server.request_disconnect(terminateDebuggee=True) output = self.collect_console( - timeout=1.0, pattern=terminateCommands[0], ) self.verify_commands("terminateCommands", output, terminateCommands) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py b/lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py index 151ad761a5044..beab4d6c1f5a6 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py @@ -82,14 +82,14 @@ def test_breakpoint_events(self): ) # Flush the breakpoint events. - self.dap_server.wait_for_breakpoint_events(timeout=5) + self.dap_server.wait_for_breakpoint_events() # Continue to the breakpoint self.continue_to_breakpoints(dap_breakpoint_ids) verified_breakpoint_ids = [] unverified_breakpoint_ids = [] - for breakpoint_event in self.dap_server.wait_for_breakpoint_events(timeout=5): + for breakpoint_event in self.dap_server.wait_for_breakpoint_events(): breakpoint = breakpoint_event["body"]["breakpoint"] id = breakpoint["id"] if breakpoint["verified"]: diff --git a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py index e61d2480ea4bb..f53813a8a48f6 100644 --- a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py +++ b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py @@ -23,7 +23,6 @@ def test_command_directive_quiet_on_success(self): exitCommands=["?" + command_quiet, command_not_quiet], ) full_output = self.collect_console( - timeout=1.0, pattern=command_not_quiet, ) self.assertNotIn(command_quiet, full_output) @@ -51,7 +50,6 @@ def do_test_abort_on_error( expectFailure=True, ) full_output = self.collect_console( - timeout=1.0, pattern=command_abort_on_error, ) self.assertNotIn(command_quiet, full_output) diff --git a/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py b/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py index bb835af12f5ef..1f4afabbd161e 100644 --- a/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py +++ b/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py @@ -23,15 +23,15 @@ def test_module_event(self): self.continue_to_breakpoints(breakpoint_ids) # We're now stopped at breakpoint 1 before the dlopen. Flush all the module events. - event = self.dap_server.wait_for_event(["module"], 0.25) + event = self.dap_server.wait_for_event(["module"]) while event is not None: - event = self.dap_server.wait_for_event(["module"], 0.25) + event = self.dap_server.wait_for_event(["module"]) # Continue to the second breakpoint, before the dlclose. self.continue_to_breakpoints(breakpoint_ids) # Make sure we got a module event for libother. - event = self.dap_server.wait_for_event(["module"], 5) + event = self.dap_server.wait_for_event(["module"]) self.assertIsNotNone(event, "didn't get a module event") module_name = event["body"]["module"]["name"] module_id = event["body"]["module"]["id"] @@ -42,7 +42,7 @@ def test_module_event(self): self.continue_to_breakpoints(breakpoint_ids) # Make sure we got a module event for libother. - event = self.dap_server.wait_for_event(["module"], 5) + event = self.dap_server.wait_for_event(["module"]) self.assertIsNotNone(event, "didn't get a module event") reason = event["body"]["reason"] self.assertEqual(r... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/163292 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
