[llvm] 295d4e4 - [lit] Try to remove the flakeyness of `shtest-timeout.py` and `googletest-timeout.py`.

Dan Liew via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 8 10:46:28 PDT 2020


Author: Dan Liew
Date: 2020-10-08T10:46:18-07:00
New Revision: 295d4e420fd09c820e9ae2cc1a69276b1c21cfb7

URL: https://github.com/llvm/llvm-project/commit/295d4e420fd09c820e9ae2cc1a69276b1c21cfb7
DIFF: https://github.com/llvm/llvm-project/commit/295d4e420fd09c820e9ae2cc1a69276b1c21cfb7.diff

LOG: [lit] Try to remove the flakeyness of `shtest-timeout.py` and `googletest-timeout.py`.

The tests previously relied on the `short.py` and `FirstTest.subTestA`
script being executed on a machine within a short time window (1 or 2
seconds). While this "seems to work" it can fail on resource constrained
machines. We could bump the timeout a little bit (bumping it too
much would mean the test would take a long time to execute) but it wouldn't
really solve the problem of the test being prone to failures.

This patch tries to remove this flakeyness by separating testing into
two separate parts:

1. Testing if a test can hit a timeout.
2. Testing if a test can run to completion in the presence of a
timeout.

This way we can give (1.) a really short timeout (to make the test run
as fast as possible) and (2.) a really long timeout. This means for (2.)
we are no longer trying to rely on the "short" test executing within
some short time window. Instead the window is now 3600 seconds which
should be long enough even for a heavily resource constrained machine to
execute the "short" test.

Thanks to Julian Lettner for suggesting this approach. This superseeds
my original approach in https://reviews.llvm.org/D88807.

This patch also changes the command line override test to run the quick
test rather than the slow one to make the test run faster.

Differential Revision: https://reviews.llvm.org/D89020

Added: 
    

Modified: 
    llvm/utils/lit/tests/Inputs/googletest-timeout/DummySubDir/OneTest.py
    llvm/utils/lit/tests/googletest-timeout.py
    llvm/utils/lit/tests/shtest-timeout.py

Removed: 
    


################################################################################
diff  --git a/llvm/utils/lit/tests/Inputs/googletest-timeout/DummySubDir/OneTest.py b/llvm/utils/lit/tests/Inputs/googletest-timeout/DummySubDir/OneTest.py
index f3a90ff4cd67..acf13a466fa7 100644
--- a/llvm/utils/lit/tests/Inputs/googletest-timeout/DummySubDir/OneTest.py
+++ b/llvm/utils/lit/tests/Inputs/googletest-timeout/DummySubDir/OneTest.py
@@ -8,27 +8,21 @@
 
 if sys.argv[1] == "--gtest_list_tests":
     print("""\
-FirstTest.
-  subTestA
-  subTestB
-  subTestC
+T.
+  QuickSubTest
+  InfiniteLoopSubTest
 """)
     sys.exit(0)
 elif not sys.argv[1].startswith("--gtest_filter="):
     raise ValueError("unexpected argument: %r" % (sys.argv[1]))
 
 test_name = sys.argv[1].split('=',1)[1]
-if test_name == 'FirstTest.subTestA':
-    print('I am subTest A, I PASS')
+if test_name == 'T.QuickSubTest':
+    print('I am QuickSubTest, I PASS')
     print('[  PASSED  ] 1 test.')
     sys.exit(0)
-elif test_name == 'FirstTest.subTestB':
-    print('I am subTest B, I am slow')
-    time.sleep(6)
-    print('[  PASSED  ] 1 test.')
-    sys.exit(0)
-elif test_name == 'FirstTest.subTestC':
-    print('I am subTest C, I will hang')
+elif test_name == 'T.InfiniteLoopSubTest':
+    print('I am InfiniteLoopSubTest, I will hang')
     while True:
         pass
 else:

diff  --git a/llvm/utils/lit/tests/googletest-timeout.py b/llvm/utils/lit/tests/googletest-timeout.py
index 44fad0a923f7..fece3852af33 100644
--- a/llvm/utils/lit/tests/googletest-timeout.py
+++ b/llvm/utils/lit/tests/googletest-timeout.py
@@ -1,29 +1,47 @@
 # REQUIRES: lit-max-individual-test-time
 
+###############################################################################
+# Check tests can hit timeout when set
+###############################################################################
+
 # Check that the per test timeout is enforced when running GTest tests.
 #
-# RUN: not %{lit} -j 1 -v %{inputs}/googletest-timeout --timeout=1 > %t.cmd.out
-# RUN: FileCheck < %t.cmd.out %s
+# RUN: not %{lit} -j 1 -v %{inputs}/googletest-timeout \
+# RUN:   --filter=InfiniteLoopSubTest --timeout=1 > %t.cmd.out
+# RUN: FileCheck --check-prefix=CHECK-INF < %t.cmd.out %s
 
 # Check that the per test timeout is enforced when running GTest tests via
 # the configuration file
 #
 # RUN: not %{lit} -j 1 -v %{inputs}/googletest-timeout \
-# RUN: --param set_timeout=1 > %t.cfgset.out 2> %t.cfgset.err
-# RUN: FileCheck < %t.cfgset.out %s
+# RUN:  --filter=InfiniteLoopSubTest  --param set_timeout=1 \
+# RUN:  > %t.cfgset.out
+# RUN: FileCheck --check-prefix=CHECK-INF < %t.cfgset.out %s
 
-# CHECK: -- Testing:
-# CHECK: PASS: googletest-timeout :: {{[Dd]ummy[Ss]ub[Dd]ir}}/OneTest.py/FirstTest.subTestA
-# CHECK: TIMEOUT: googletest-timeout :: {{[Dd]ummy[Ss]ub[Dd]ir}}/OneTest.py/FirstTest.subTestB
-# CHECK: TIMEOUT: googletest-timeout :: {{[Dd]ummy[Ss]ub[Dd]ir}}/OneTest.py/FirstTest.subTestC
-# CHECK: Passed   : 1
-# CHECK: Timed Out: 2
+# CHECK-INF: -- Testing:
+# CHECK-INF: TIMEOUT: googletest-timeout :: {{[Dd]ummy[Ss]ub[Dd]ir}}/OneTest.py/T.InfiniteLoopSubTest
+# CHECK-INF: Timed Out: 1
+
+###############################################################################
+# Check tests can complete with a timeout set
+#
+# `QuickSubTest` should execute quickly so we shouldn't wait anywhere near the
+# 3600 second timeout.
+###############################################################################
+
+# RUN: %{lit} -j 1 -v %{inputs}/googletest-timeout \
+# RUN:   --filter=QuickSubTest --timeout=3600 > %t.cmd.out
+# RUN: FileCheck --check-prefix=CHECK-QUICK < %t.cmd.out %s
+
+# CHECK-QUICK: PASS: googletest-timeout :: {{[Dd]ummy[Ss]ub[Dd]ir}}/OneTest.py/T.QuickSubTest
+# CHECK-QUICK: Passed : 1
 
 # Test per test timeout via a config file and on the command line.
 # The value set on the command line should override the config file.
-# RUN: not %{lit} -j 1 -v %{inputs}/googletest-timeout \
-# RUN: --param set_timeout=1 --timeout=2 > %t.cmdover.out 2> %t.cmdover.err
-# RUN: FileCheck < %t.cmdover.out %s
+# RUN: %{lit} -j 1 -v %{inputs}/googletest-timeout --filter=QuickSubTest \
+# RUN:   --param set_timeout=1 --timeout=3600 \
+# RUN:   > %t.cmdover.out 2> %t.cmdover.err
+# RUN: FileCheck --check-prefix=CHECK-QUICK < %t.cmdover.out %s
 # RUN: FileCheck --check-prefix=CHECK-CMDLINE-OVERRIDE-ERR < %t.cmdover.err %s
 
-# CHECK-CMDLINE-OVERRIDE-ERR: Forcing timeout to be 2 seconds
+# CHECK-CMDLINE-OVERRIDE-ERR: Forcing timeout to be 3600 seconds

diff  --git a/llvm/utils/lit/tests/shtest-timeout.py b/llvm/utils/lit/tests/shtest-timeout.py
index a0b792d9b915..558aca5f05bf 100644
--- a/llvm/utils/lit/tests/shtest-timeout.py
+++ b/llvm/utils/lit/tests/shtest-timeout.py
@@ -3,16 +3,14 @@
 # llvm.org/PR33944
 # UNSUPPORTED: system-windows
 
-# FIXME: This test is fragile because it relies on time which can
-# be affected by system performance. In particular we are currently
-# assuming that `short.py` can be successfully executed within 2
-# seconds of wallclock time.
+###############################################################################
+# Check tests can hit timeout when set
+###############################################################################
 
 # Test per test timeout using external shell
 # RUN: not %{lit} \
 # RUN: %{inputs}/shtest-timeout/infinite_loop.py \
-# RUN: %{inputs}/shtest-timeout/short.py \
-# RUN: -j 1 -v --debug --timeout 2 --param external=1 > %t.extsh.out 2> %t.extsh.err
+# RUN: -j 1 -v --debug --timeout 1 --param external=1 > %t.extsh.out 2> %t.extsh.err
 # RUN: FileCheck --check-prefix=CHECK-OUT-COMMON < %t.extsh.out %s
 # RUN: FileCheck --check-prefix=CHECK-EXTSH-ERR < %t.extsh.err %s
 #
@@ -21,8 +19,7 @@
 # Test per test timeout using internal shell
 # RUN: not %{lit} \
 # RUN: %{inputs}/shtest-timeout/infinite_loop.py \
-# RUN: %{inputs}/shtest-timeout/short.py \
-# RUN: -j 1 -v --debug --timeout 2 --param external=0 > %t.intsh.out 2> %t.intsh.err
+# RUN: -j 1 -v --debug --timeout 1 --param external=0 > %t.intsh.out 2> %t.intsh.err
 # RUN: FileCheck  --check-prefix=CHECK-OUT-COMMON < %t.intsh.out %s
 # RUN: FileCheck --check-prefix=CHECK-INTSH-OUT < %t.intsh.out %s
 # RUN: FileCheck --check-prefix=CHECK-INTSH-ERR < %t.intsh.err %s
@@ -36,40 +33,50 @@
 # Test per test timeout set via a config file rather than on the command line
 # RUN: not %{lit} \
 # RUN: %{inputs}/shtest-timeout/infinite_loop.py \
-# RUN: %{inputs}/shtest-timeout/short.py \
 # RUN: -j 1 -v --debug --param external=0 \
-# RUN: --param set_timeout=2 > %t.cfgset.out 2> %t.cfgset.err
+# RUN: --param set_timeout=1 > %t.cfgset.out 2> %t.cfgset.err
 # RUN: FileCheck --check-prefix=CHECK-OUT-COMMON  < %t.cfgset.out %s
 # RUN: FileCheck --check-prefix=CHECK-CFGSET-ERR < %t.cfgset.err %s
 #
 # CHECK-CFGSET-ERR: Using internal shell
 
 # CHECK-OUT-COMMON: TIMEOUT: per_test_timeout :: infinite_loop.py
-# CHECK-OUT-COMMON: Timeout: Reached timeout of 2 seconds
+# CHECK-OUT-COMMON: Timeout: Reached timeout of 1 seconds
 # CHECK-OUT-COMMON: Command {{([0-9]+ )?}}Output
+# CHECK-OUT-COMMON: Timed Out: 1
 
-# CHECK-OUT-COMMON: PASS: per_test_timeout :: short.py
 
-# CHECK-OUT-COMMON: Passed   : 1
-# CHECK-OUT-COMMON: Timed Out: 1
+###############################################################################
+# Check tests can complete in with a timeout set
+#
+# `short.py` should execute quickly so we shouldn't wait anywhere near the
+# 3600 second timeout.
+###############################################################################
 
-# Test per test timeout via a config file and on the command line.
-# The value set on the command line should override the config file.
-# RUN: not %{lit} \
-# RUN: %{inputs}/shtest-timeout/infinite_loop.py \
+# Test per test timeout using external shell
+# RUN: %{lit} \
 # RUN: %{inputs}/shtest-timeout/short.py \
-# RUN: -j 1 -v --debug --param external=0 \
-# RUN: --param set_timeout=1 --timeout=2 > %t.cmdover.out 2> %t.cmdover.err
-# RUN: FileCheck --check-prefix=CHECK-CMDLINE-OVERRIDE-OUT  < %t.cmdover.out %s
-# RUN: FileCheck --check-prefix=CHECK-CMDLINE-OVERRIDE-ERR < %t.cmdover.err %s
+# RUN: -j 1 -v --debug --timeout 3600 --param external=1 > %t.pass.extsh.out 2> %t.pass.extsh.err
+# RUN: FileCheck --check-prefix=CHECK-OUT-COMMON-SHORT < %t.pass.extsh.out %s
+# RUN: FileCheck --check-prefix=CHECK-EXTSH-ERR < %t.pass.extsh.err %s
 
-# CHECK-CMDLINE-OVERRIDE-ERR: Forcing timeout to be 2 seconds
+# Test per test timeout using internal shell
+# RUN: %{lit} \
+# RUN: %{inputs}/shtest-timeout/short.py \
+# RUN: -j 1 -v --debug --timeout 3600 --param external=0 > %t.pass.intsh.out 2> %t.pass.intsh.err
+# RUN: FileCheck  --check-prefix=CHECK-OUT-COMMON-SHORT < %t.pass.intsh.out %s
+# RUN: FileCheck --check-prefix=CHECK-INTSH-ERR < %t.pass.intsh.err %s
 
-# CHECK-CMDLINE-OVERRIDE-OUT: TIMEOUT: per_test_timeout :: infinite_loop.py
-# CHECK-CMDLINE-OVERRIDE-OUT: Timeout: Reached timeout of 2 seconds
-# CHECK-CMDLINE-OVERRIDE-OUT: Command {{([0-9]+ )?}}Output
+# CHECK-OUT-COMMON-SHORT: PASS: per_test_timeout :: short.py
+# CHECK-OUT-COMMON-SHORT: Passed: 1
 
-# CHECK-CMDLINE-OVERRIDE-OUT: PASS: per_test_timeout :: short.py
+# Test per test timeout via a config file and on the command line.
+# The value set on the command line should override the config file.
+# RUN: %{lit} \
+# RUN:   %{inputs}/shtest-timeout/short.py \
+# RUN:   -j 1 -v --debug --param external=0 \
+# RUN: --param set_timeout=1 --timeout=3600 > %t.pass.cmdover.out 2> %t.pass.cmdover.err
+# RUN: FileCheck --check-prefix=CHECK-OUT-COMMON-SHORT  < %t.pass.cmdover.out %s
+# RUN: FileCheck --check-prefix=CHECK-CMDLINE-OVERRIDE-ERR < %t.pass.cmdover.err %s
 
-# CHECK-CMDLINE-OVERRIDE-OUT: Passed   : 1
-# CHECK-CMDLINE-OVERRIDE-OUT: Timed Out: 1
+# CHECK-CMDLINE-OVERRIDE-ERR: Forcing timeout to be 3600 seconds


        


More information about the llvm-commits mailing list