-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[lldb-dap] Increase DAP default timeout #170890
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) ChangesDAP tests easily timeout when the computer is under-load. This is easily noticeable if you run the test in a loop a compile llvm. or running the test on slower CI/machines. This affects mostly the Would lead in the direction of enabling some of the test on windows. Increase the DEFAULT_TIMEOUT from 10 to 50 Full diff: https://github.com/llvm/llvm-project/pull/170890.diff 2 Files Affected:
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 7a9d5a82983d7..7c95cc3d87f56 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
@@ -15,6 +15,7 @@
import time
from typing import (
Any,
+ Final,
Optional,
Dict,
cast,
@@ -30,7 +31,7 @@
# 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: Final[float] = 50 * (10 if ("ASAN_OPTIONS" in os.environ) else 1)
# See lldbtest.Base.spawnSubprocess, which should help ensure any processes
# created by the DAP client are terminated correctly when the test ends.
@@ -348,7 +349,7 @@ def _recv_packet(
self,
*,
predicate: Optional[Callable[[ProtocolMessage], bool]] = None,
- timeout: Optional[float] = DEFAULT_TIMEOUT,
+ timeout: float = DEFAULT_TIMEOUT,
) -> Optional[ProtocolMessage]:
"""Processes received packets from the adapter.
Updates the DebugCommunication stateful properties based on the received
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 c7d302cc2dea2..fa8b1b7ab6426 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
@@ -1,6 +1,6 @@
import os
import time
-from typing import Optional, Callable, Any, List, Union
+from typing import Optional, Callable, Any, List, Union, Final
import uuid
import dap_server
@@ -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 = dap_server.DEFAULT_TIMEOUT
+ DEFAULT_TIMEOUT: Final[float] = dap_server.DEFAULT_TIMEOUT
NO_DEBUG_INFO_TESTCASE = True
def create_debug_adapter(
|
DrSergei
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure that we should increase the default timeout for all tests. Maybe we can do it only for mentioned tests. My intention here is that a big timeout can cause problems on CI when flaky tests are failed (e.g. waiting for events). As far as I know, there was big work done to unify timeout, so this solution is expected from this point of view.
| # 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: Final[float] = 50 * (10 if ("ASAN_OPTIONS" in os.environ) else 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe use 50.0 float literal
The goal is to make the test no longer flaky, IMO if there is a wait for event longer than 50 seconds there is something else wrong with the test than the timeout itself.
the unified timeouts was completed here |
ashgti
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with this if we think it will improve stability.
However, I think there may still be a few places in tests where we wait for a negative assertion to time out. We may need to make sure we find anymore of those and make them positive assertions so they can end early.
For me, if I run the tests with "-DLLVM_LIT_ARGS='-v --use-unique-output-file-name --time-tests'",
$ ninja check-lldb-api-tools-lldb-dap
Slowest Tests:
--------------------------------------------------------------------------
102.61s: lldb-api :: tools/lldb-dap/variables/TestDAP_variables.py
77.41s: lldb-api :: tools/lldb-dap/evaluate/TestDAP_evaluate.py
46.74s: lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py
32.29s: lldb-api :: tools/lldb-dap/module/TestDAP_module.py
32.08s: lldb-api :: tools/lldb-dap/completions/TestDAP_completions.py
28.63s: lldb-api :: tools/lldb-dap/disconnect/TestDAP_disconnect.py
27.49s: lldb-api :: tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py
26.55s: lldb-api :: tools/lldb-dap/server/TestDAP_server.py
26.47s: lldb-api :: tools/lldb-dap/restart/TestDAP_restart.py
24.46s: lldb-api :: tools/lldb-dap/threads/TestDAP_threads.py
20.19s: lldb-api :: tools/lldb-dap/exception/objc/TestDAP_exception_objc.py
19.72s: lldb-api :: tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py
15.53s: lldb-api :: tools/lldb-dap/console/TestDAP_console.py
15.08s: lldb-api :: tools/lldb-dap/breakpoint/TestDAP_logpoints.py
13.81s: lldb-api :: tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
13.81s: lldb-api :: tools/lldb-dap/optimized/TestDAP_optimized.py
13.68s: lldb-api :: tools/lldb-dap/module-event/TestDAP_module_event.py
11.66s: lldb-api :: tools/lldb-dap/memory/TestDAP_memory.py
10.55s: lldb-api :: tools/lldb-dap/terminated-event/TestDAP_terminatedEvent.py
10.48s: lldb-api :: tools/lldb-dap/attach-commands/TestDAP_attachCommands.py
I see a few tests that are pretty slow. The slowest ones are where I have been meaning to check for negative assertions on a timeout.
JDevlieghere
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DAP tests easily timeout when the computer is under-load.
This is easily noticeable if you run the test in a loop a compile llvm.
or running the test on slower CI/machines.
This affects mostly the `DAP_launch`, `DAP_attach`,
`DAP_restart_console` and occasionally `DAP_output` tests.
Would lead in the direction of enabling some of the test on windows.
```
File "/Volumes/workspace/Dev/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 1140, in request_launch
return self._send_recv(command_dict)
File "/Volumes/workspace/Dev/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 548, in _send_recv
raise ValueError(f"no response for {request!r}")
ValueError: no response for {'command': 'launch', 'type': 'request', 'arguments': {'program': '/Volumes/workspace/Dev/llvm-build/release/lldb-test-build.noindex/tools/lldb-dap/restart/TestDAP_restart_console.test_basic_functionality/a.out', 'initCommands': ['settings clear --all', 'settings set symbols.enable-external-lookup false', 'settings set target.inherit-tcc true', 'settings set target.disable-aslr false', 'settings set target.detach-on-error false', 'settings set target.auto-apply-fixits false', 'settings set plugin.process.gdb-remote.packet-timeout 60', 'settings set symbols.clang-modules-cache-path "/Volumes/workspace/Dev/llvm-build/release/lldb-test-build.noindex/module-cache-lldb"', 'settings set use-color false', 'settings set show-statusline false'], 'console': 'integratedTerminal', 'disableASLR': False, 'enableAutoVariableSummaries': False, 'enableSyntheticChildDebugging': False, 'displayExtendedBacktrace': False}, 'seq': 2}
Config=arm64-/Volumes/workspace/Dev/llvm-build/release/bin/clang
----------------------------------------------------------------------
Ran 2 tests in 28.579s
```
Increase the DEFAULT_TIMEOUT from 10 to 50
(cherry picked from commit 0c179aa)
DAP tests easily timeout when the computer is under-load.
This is easily noticeable if you run the test in a loop a compile llvm. or running the test on slower CI/machines.
This affects mostly the
DAP_launch,DAP_attach,DAP_restart_consoleand occasionallyDAP_outputtests.Would lead in the direction of enabling some of the test on windows.
Increase the DEFAULT_TIMEOUT from 10 to 50