Author: David Spickett Date: 2026-09-04T15:07:26+01:00 New Revision: 69056048fe7d50960e4e5a806a898a3146845641
URL: https://github.com/llvm/llvm-project/commit/69056048fe7d50960e4e5a806a898a3146845641 DIFF: https://github.com/llvm/llvm-project/commit/69056048fe7d50960e4e5a806a898a3146845641.diff LOG: [lldb][test] Fix flaky results from TestSymStore.py test_http_timeout (#221196) Fixes #217733. Observed only on Windows on Arm but I bet it can happen other places. Sometimes the test would fail because it did not find the error message in the log file. The problem is that the test is relying on the client to realise the timeout has been exceeded and close the connection, before the fake responder can send back status 200. So what sometimes happens is: * The client sends the request. * The server gets the request and waits N seconds. * The client is delayed doing other things, and doesn't realise the timeout has been reached. So the connection is still open. * The server tries to respond with 200, and that sends succesfully. * The client wakes up and sees the 200 response, so there's no error to report. To fix this, I've changed the responder from merely being slow to never responding. This means the client can be delayed as long as it likes. To prevent the responder hanging around, it waits on an event which the test will set once the test is done (whether pass or fail), then the responder will shut down normally. This change does mean that if the client was buggy, we could be waiting up to the global test timeout. However I think that's a reasonable trade to not have flaky test results. The alternative is to increase the responder delay, which will have to vary machine to machine and run to run. In the end, you'd end up with a delay very similar to the global test timeout anyway. This change also narrows the original intent, though not the original behaviour of the test. We would like to test that the timeout is used, is accurate, and is configurable. Doing so without flakes is more work than I'm willing to commit to right now. Added: Modified: lldb/test/API/symstore/TestSymStore.py Removed: ################################################################################ diff --git a/lldb/test/API/symstore/TestSymStore.py b/lldb/test/API/symstore/TestSymStore.py index 4c668134447d1..09cd5a71e715b 100644 --- a/lldb/test/API/symstore/TestSymStore.py +++ b/lldb/test/API/symstore/TestSymStore.py @@ -234,18 +234,13 @@ def do_GET(self): super().do_GET() -class SlowHTTPHandler(http.server.BaseHTTPRequestHandler): - """HTTP request handler that delays responses to simulate a slow server.""" +class NeverRespondsHTTPHandler(http.server.BaseHTTPRequestHandler): + """HTTP request handler that never responds.""" - delay = 2 # seconds to sleep before responding; set per test + can_finish = threading.Event() def do_GET(self): - try: - time.sleep(self.delay) - self.send_response(200) - self.end_headers() - except (BrokenPipeError, ConnectionResetError): - pass # client disconnected after timeout, as expected + self.can_finish.wait() def log_message(self, *args): pass # suppress server-side output @@ -451,18 +446,23 @@ def test_http_timeout(self): """ exe, sym = self.build_inferior() with MockedSymStore(self, exe, sym) as dir: - SlowHTTPHandler.delay = 5 # seconds; exceeds the 1s timeout below + NeverRespondsHTTPHandler.can_finish.clear() self.runCmd("settings set plugin.symbol-locator.symstore.timeout 1") - with HTTPServer(dir, SlowHTTPHandler) as url: - self.runCmd(f"settings set plugin.symbol-locator.symstore.urls {url}") - warnings = "" - with open(self.getBuildArtifact("stderr.txt"), "w+b") as err_file: - self.dbg.SetErrorFileHandle(err_file, False) - self.try_breakpoint(exe, should_have_loc=False) - self.dbg.SetErrorFileHandle(sys.stderr, False) - err_file.seek(0) - warnings = err_file.read().decode() - self.assertIn("failed to download", warnings) + + with HTTPServer(dir, NeverRespondsHTTPHandler) as url: + try: + self.runCmd( + f"settings set plugin.symbol-locator.symstore.urls {url}" + ) + with open(self.getBuildArtifact("stderr.txt"), "w+b") as err_file: + self.dbg.SetErrorFileHandle(err_file, False) + self.try_breakpoint(exe, should_have_loc=False) + self.dbg.SetErrorFileHandle(sys.stderr, False) + err_file.seek(0) + warnings = err_file.read().decode() + self.assertIn("failed to download", warnings) + finally: + NeverRespondsHTTPHandler.can_finish.set() @skipUnlessPackageAvailable("cryptography") def test_https(self): _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
