From 2502b0171874bab0321fc2c1cd815efbbbdbc4d3 Mon Sep 17 00:00:00 2001 From: Son Luong Ngoc Date: Mon, 29 Apr 2024 11:35:20 +0200 Subject: [PATCH] bzltestutil: restore timeout signal handler In https://github.com/bazelbuild/rules_go/pull/3920, a new mechanism to handle test timeout was introduced. However this broke existing tests that use SIGTERM inside. Restore the original behavior. --- go/tools/builders/generate_test_main.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/go/tools/builders/generate_test_main.go b/go/tools/builders/generate_test_main.go index f5b5fcce3..e892d8668 100644 --- a/go/tools/builders/generate_test_main.go +++ b/go/tools/builders/generate_test_main.go @@ -245,9 +245,23 @@ func main() { // we set -test.timeout according to the TEST_TIMEOUT, we need to ignore the signal so the test has // time to properly produce the output (e.g. stack trace). It will be killed by Bazel after the grace // period (15s) expires. + // // If TEST_TIMEOUT is not set (e.g., when the test binary is run by Delve for debugging), we don't - // ignore SIGTERM so it can be properly terminated. - signal.Ignore(syscall.SIGTERM) + // ignore SIGTERM so it can be properly terminated. (1) + // We do not panic (like native go test does) because users may legitimately want to use SIGTERM + // in tests. + // + // signal.Notify is used to ensure that there is a no-op signal handler registered. + // Avoid using signal.Ignore here: despite the name, it's only used to unregistered handler(s) that + // were previously registered by signal.Notify. See (2) for more information. + // + // (1): https://github.com/golang/go/blob/e816eb50140841c524fd07ecb4eaa078954eb47c/src/testing/testing.go#L2351 + // (2): https://github.com/bazelbuild/rules_go/pull/3929 + c := make(chan os.Signal, 1) + signal.Notify(c, syscall.SIGTERM) + go func() { + <-c + }() } {{if not .TestMain}}