[lldb-dap] Unify the timeouts for the DAP tests #163292
Draft
+71
−99
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 llvm#162523
|
Draft because this breaks |
ashgti
reviewed
Oct 14, 2025
| printf("pid = %i\n", getpid()); | ||
| #ifdef _WIN32 | ||
| Sleep(10 * 1000); | ||
| Sleep(1 * 1000); |
There was a problem hiding this comment.
This sleep and sleep on line 27 are now two different durations (1s vs 10s).
|
|
||
| # Flush the breakpoint events. | ||
| self.dap_server.wait_for_breakpoint_events(timeout=5) | ||
| self.dap_server.wait_for_breakpoint_events() |
There was a problem hiding this comment.
I think this waits for the full 10s for this to resolve.
Should we look into refactoring this test to not require a 10s wait? E.g. if we know the id of the breakpoints were interested in, then we can wait for those specifically and return once they've resolved.
|
|
||
| # Make sure we do not send another event | ||
| event = self.dap_server.wait_for_event(["module"], 3) | ||
| event = self.dap_server.wait_for_event(["module"]) |
There was a problem hiding this comment.
For a negative assertion, we're going to wait for the full 10s.
Maybe we need to refactor this test to move away from a negative assertion...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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