Skip to content

Commit

Permalink
i#6558, i#6127: Set CTest timeouts for all tests (#6563)
Browse files Browse the repository at this point in the history
Before, we relied on drrun -s for all test suite timeouts except for
runcmp tests where we set a CTest timeout. This resulted in the default
10 minute CTest timeout for all tests, which was the only timeout for
runall tests and caused long suite times on the AArch64 machine which
accidentally had no ptrace privileges (#5740, #6558,

Here, we set the CTest time for runall in addition to runcmp, and for
all other tests with no timeout specified (which are presumably relying
on drrun -s) we set a timeout of the drrun timeout plus 30 seconds.

Tested on the attach test:

Before:
```
123: Test timeout computed to be: 1500
```
Now:
```
$ echo 1 | sudo tee /proc/sys/kernel/yama/ptrace_scope; /usr/bin/time ctest -V -R client.attach_test; echo 0 | sudo tee /proc/sys/kernel/yama/ptrace_scope
...
    Start 121: code_api|client.attach_test
...
121: Test timeout computed to be: 90
1/1 Test #121: code_api|client.attach_test ......***Timeout  90.11 sec
...
The following tests FAILED:
	121 - code_api|client.attach_test (Timeout)
...
Command exited with non-zero status 8
1.13user 0.80system 1:30.14elapsed 2%CPU (0avgtext+0avgdata 13196maxresident)k
```

The property being set on more tests was confirmed on debug x86-64:
Before:
```
$ grep -c TIMEOUT suite/tests/CTestTestfile.cmake
94
```
After:
```
$ grep -c TIMEOUT suite/tests/CTestTestfile.cmake
463
```
There seem to still be a few missing the property: the ones that don't
go through suite/. There were other efforts to avoid hangs on those such
as PR #6137.

Issue: #6127, #6558, #5740
  • Loading branch information
derekbruening committed Jan 16, 2024
1 parent 715f897 commit 8ea7016
Showing 1 changed file with 8 additions and 3 deletions.
11 changes: 8 additions & 3 deletions suite/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# **********************************************************
# Copyright (c) 2010-2023 Google, Inc. All rights reserved.
# Copyright (c) 2010-2024 Google, Inc. All rights reserved.
# Copyright (c) 2009-2010 VMware, Inc. All rights reserved.
# Copyright (c) 2016-2023 ARM Limited. All rights reserved.
# **********************************************************
Expand Down Expand Up @@ -1631,9 +1631,14 @@ function(torun test key source native standalone_dr dr_ops exe_ops added_out pas
# Though we mostly use drrun's -s we set this too in case the requested
# value is higher than ctest's default.
set_tests_properties(${test} PROPERTIES TIMEOUT ${${key}_timeout})
elseif (is_runcmp)
# Runcmp generally doesn't use drrun or runstats so we need a ctest timeout.
elseif (is_runcmp OR is_runall)
# Runcmp/runall generally don't use drrun or runstats so we need a ctest timeout.
set_tests_properties(${test} PROPERTIES TIMEOUT ${TEST_SECONDS})
else ()
# Even though we expect drrun -s to enforce a timeout, set one in ctest just
# in case, but give time for drrun first.
math(EXPR timeout "${TEST_SECONDS}+30")
set_tests_properties(${test} PROPERTIES TIMEOUT ${timeout})
endif ()

# Though we use drrun and runstats -s timeout parameters, we have
Expand Down

0 comments on commit 8ea7016

Please sign in to comment.