[llvm] 81c0d36 - [LIT] error if directly named test won't be run indirectly

Ben Dunbobbin via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 21 05:23:35 PDT 2020


Author: Ben Dunbobbin
Date: 2020-10-21T13:21:29+01:00
New Revision: 81c0d36a1836c9be7c34a6d8198310ad7ea9bb53

URL: https://github.com/llvm/llvm-project/commit/81c0d36a1836c9be7c34a6d8198310ad7ea9bb53
DIFF: https://github.com/llvm/llvm-project/commit/81c0d36a1836c9be7c34a6d8198310ad7ea9bb53.diff

LOG: [LIT] error if directly named test won't be run indirectly

Currently, a LIT test named directly (on the command line) will
be run even if the name of the test file does not meet the rules
to be considered a test in the LIT test configuration files for
its test suite. For example, if the test does not have a
recognised file extension.

This makes it relatively easy to write a LIT test that won't
actually be run. I did in: https://reviews.llvm.org/D82567

This patch adds an error to avoid users doing that. There is a
small performance overhead for this check. A command line option
has been added so that users can opt into the old behaviour.

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

Added: 
    llvm/utils/lit/tests/Inputs/discovery/test.not-txt

Modified: 
    llvm/utils/lit/lit/LitTestCase.py
    llvm/utils/lit/lit/cl_arguments.py
    llvm/utils/lit/lit/discovery.py
    llvm/utils/lit/lit/main.py
    llvm/utils/lit/tests/discovery.py
    llvm/utils/lit/tests/unit/TestRunner.py

Removed: 
    


################################################################################
diff  --git a/llvm/utils/lit/lit/LitTestCase.py b/llvm/utils/lit/lit/LitTestCase.py
index 951f7be958e2..81ed61ab0964 100644
--- a/llvm/utils/lit/lit/LitTestCase.py
+++ b/llvm/utils/lit/lit/LitTestCase.py
@@ -55,8 +55,8 @@ def load_test_suite(inputs):
         params={})
 
     # Perform test discovery.
-    tests = lit.discovery.find_tests_for_inputs(lit_config, inputs)
+    tests = lit.discovery.find_tests_for_inputs(lit_config, inputs, False)
     test_adaptors = [LitTestCase(t, lit_config) for t in tests]
 
     # Return a unittest test suite which just runs the tests in order.
-    return unittest.TestSuite(test_adaptors)
+    return unittest.TestSuite(test_adaptors)
\ No newline at end of file

diff  --git a/llvm/utils/lit/lit/cl_arguments.py b/llvm/utils/lit/lit/cl_arguments.py
index 69166e00aba8..591d4f9aaafb 100644
--- a/llvm/utils/lit/lit/cl_arguments.py
+++ b/llvm/utils/lit/lit/cl_arguments.py
@@ -123,6 +123,12 @@ def parse_args():
     execution_group.add_argument("--allow-empty-runs",
             help="Do not fail the run if all tests are filtered out",
             action="store_true")
+    execution_group.add_argument("--no-indirectly-run-check",
+            dest="indirectlyRunCheck",
+            help="Do not error if a test would not be run if the user had "
+                 "specified the containing directory instead of naming the "
+                 "test directly.",
+            action="store_false")
 
     selection_group = parser.add_argument_group("Test Selection")
     selection_group.add_argument("--max-tests",

diff  --git a/llvm/utils/lit/lit/discovery.py b/llvm/utils/lit/lit/discovery.py
index d8054543d018..d83e90977e44 100644
--- a/llvm/utils/lit/lit/discovery.py
+++ b/llvm/utils/lit/lit/discovery.py
@@ -125,7 +125,8 @@ def search(path_in_suite):
 
     return search(path_in_suite)
 
-def getTests(path, litConfig, testSuiteCache, localConfigCache):
+def getTests(path, litConfig, testSuiteCache,
+             localConfigCache, indirectlyRunCheck):
     # Find the test suite for this input and its relative path.
     ts,path_in_suite = getTestSuite(path, litConfig, testSuiteCache)
     if ts is None:
@@ -137,10 +138,10 @@ def getTests(path, litConfig, testSuiteCache, localConfigCache):
                                                         path_in_suite))
 
     return ts, getTestsInSuite(ts, path_in_suite, litConfig,
-                               testSuiteCache, localConfigCache)
+                               testSuiteCache, localConfigCache, indirectlyRunCheck)
 
 def getTestsInSuite(ts, path_in_suite, litConfig,
-                    testSuiteCache, localConfigCache):
+                    testSuiteCache, localConfigCache, indirectlyRunCheck):
     # Check that the source path exists (errors here are reported by the
     # caller).
     source_path = ts.getSourcePath(path_in_suite)
@@ -149,8 +150,30 @@ def getTestsInSuite(ts, path_in_suite, litConfig,
 
     # Check if the user named a test directly.
     if not os.path.isdir(source_path):
-        lc = getLocalConfig(ts, path_in_suite[:-1], litConfig, localConfigCache)
-        yield Test.Test(ts, path_in_suite, lc)
+        test_dir_in_suite = path_in_suite[:-1]
+        lc = getLocalConfig(ts, test_dir_in_suite, litConfig, localConfigCache)
+        test = Test.Test(ts, path_in_suite, lc)
+
+        # Issue a error if the specified test would not be run if
+        # the user had specified the containing directory instead of
+        # of naming the test directly. This helps to avoid writing
+        # tests which are not executed. The check adds some performance
+        # overhead which might be important if a large number of tests
+        # are being run directly.
+        # --no-indirectly-run-check: skips this check.
+        if indirectlyRunCheck and lc.test_format is not None:
+            found = False
+            for res in lc.test_format.getTestsInDirectory(ts, test_dir_in_suite,
+                                                          litConfig, lc):
+                if test.getFullName() == res.getFullName():
+                    found = True
+                    break
+            if not found:
+                litConfig.error(
+                    '%r would not be run indirectly: change name or LIT config'
+                    % test.getFullName())
+
+        yield test
         return
 
     # Otherwise we have a directory to search for tests, start by getting the
@@ -196,10 +219,11 @@ def getTestsInSuite(ts, path_in_suite, litConfig,
         # Otherwise, load from the nested test suite, if present.
         if sub_ts is not None:
             subiter = getTestsInSuite(sub_ts, subpath_in_suite, litConfig,
-                                      testSuiteCache, localConfigCache)
+                                      testSuiteCache, localConfigCache,
+                                      indirectlyRunCheck)
         else:
             subiter = getTestsInSuite(ts, subpath, litConfig, testSuiteCache,
-                                      localConfigCache)
+                                      localConfigCache, indirectlyRunCheck)
 
         N = 0
         for res in subiter:
@@ -208,7 +232,7 @@ def getTestsInSuite(ts, path_in_suite, litConfig,
         if sub_ts and not N:
             litConfig.warning('test suite %r contained no tests' % sub_ts.name)
 
-def find_tests_for_inputs(lit_config, inputs):
+def find_tests_for_inputs(lit_config, inputs, indirectlyRunCheck):
     """
     find_tests_for_inputs(lit_config, inputs) -> [Test]
 
@@ -237,8 +261,8 @@ def find_tests_for_inputs(lit_config, inputs):
     local_config_cache = {}
     for input in actual_inputs:
         prev = len(tests)
-        tests.extend(getTests(input, lit_config,
-                              test_suite_cache, local_config_cache)[1])
+        tests.extend(getTests(input, lit_config, test_suite_cache,
+                              local_config_cache, indirectlyRunCheck)[1])
         if prev == len(tests):
             lit_config.warning('input %r contained no tests' % input)
 
@@ -247,4 +271,4 @@ def find_tests_for_inputs(lit_config, inputs):
         sys.stderr.write('%d errors, exiting.\n' % lit_config.numErrors)
         sys.exit(2)
 
-    return tests
+    return tests
\ No newline at end of file

diff  --git a/llvm/utils/lit/lit/main.py b/llvm/utils/lit/lit/main.py
index d94d7280809d..6c9885c4d4be 100755
--- a/llvm/utils/lit/lit/main.py
+++ b/llvm/utils/lit/lit/main.py
@@ -39,7 +39,8 @@ def main(builtin_params={}):
         config_prefix=opts.configPrefix,
         echo_all_commands=opts.echoAllCommands)
 
-    discovered_tests = lit.discovery.find_tests_for_inputs(lit_config, opts.test_paths)
+    discovered_tests = lit.discovery.find_tests_for_inputs(lit_config, opts.test_paths,
+                                                           opts.indirectlyRunCheck)
     if not discovered_tests:
         sys.stderr.write('error: did not discover any tests for provided path(s)\n')
         sys.exit(2)

diff  --git a/llvm/utils/lit/tests/Inputs/discovery/test.not-txt b/llvm/utils/lit/tests/Inputs/discovery/test.not-txt
new file mode 100644
index 000000000000..b80b60b7a279
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/discovery/test.not-txt
@@ -0,0 +1 @@
+# RUN: true

diff  --git a/llvm/utils/lit/tests/discovery.py b/llvm/utils/lit/tests/discovery.py
index 94b227f33fb3..84ee2221efe7 100644
--- a/llvm/utils/lit/tests/discovery.py
+++ b/llvm/utils/lit/tests/discovery.py
@@ -51,17 +51,17 @@
 # CHECK-CONFIG-MAP-ERR: resolved input '{{.*(/|\\\\)config-map-discovery(/|\\\\)main-config}}' to 'config-map'::()
 
 
-# Check discovery when exact test names are given.
+# Check discovery when tests are named directly.
 #
 # RUN: %{lit} \
 # RUN:     %{inputs}/discovery/subdir/test-three.py \
 # RUN:     %{inputs}/discovery/subsuite/test-one.txt \
 # RUN:   -j 1 --show-tests --show-suites -v > %t.out
-# RUN: FileCheck --check-prefix=CHECK-EXACT-TEST < %t.out %s
+# RUN: FileCheck --check-prefix=CHECK-DIRECT-TEST < %t.out %s
 #
-# CHECK-EXACT-TEST: -- Available Tests --
-# CHECK-EXACT-TEST: sub-suite :: test-one
-# CHECK-EXACT-TEST: top-level-suite :: subdir/test-three
+# CHECK-DIRECT-TEST: -- Available Tests --
+# CHECK-DIRECT-TEST: sub-suite :: test-one
+# CHECK-DIRECT-TEST: top-level-suite :: subdir/test-three
 
 # Check discovery when config files end in .py
 # RUN: %{lit} %{inputs}/py-config-discovery \
@@ -122,18 +122,31 @@
 # CHECK-ASEXEC-OUT: top-level-suite :: test-one
 # CHECK-ASEXEC-OUT: top-level-suite :: test-two
 
-# Check discovery when exact test names are given.
+# Check discovery when tests are named directly.
 #
 # FIXME: Note that using a path into a subsuite doesn't work correctly here.
 #
 # RUN: %{lit} \
 # RUN:     %{inputs}/exec-discovery/subdir/test-three.py \
 # RUN:   -j 1 --show-tests --show-suites -v > %t.out
-# RUN: FileCheck --check-prefix=CHECK-ASEXEC-EXACT-TEST < %t.out %s
+# RUN: FileCheck --check-prefix=CHECK-ASEXEC-DIRECT-TEST < %t.out %s
 #
-# CHECK-ASEXEC-EXACT-TEST: -- Available Tests --
-# CHECK-ASEXEC-EXACT-TEST: top-level-suite :: subdir/test-three
+# CHECK-ASEXEC-DIRECT-TEST: -- Available Tests --
+# CHECK-ASEXEC-DIRECT-TEST: top-level-suite :: subdir/test-three
 
+# Check an error is emitted when the directly named test would not be run
+# indirectly (e.g. when the directory containing the test is specified).
+#
+# RUN: not %{lit} \
+# RUN:     %{inputs}/discovery/test.not-txt -j 1 2>%t.err
+# RUN: FileCheck --check-prefix=CHECK-ERROR-INDIRECT-RUN-CHECK < %t.err %s
+#
+# CHECK-ERROR-INDIRECT-RUN-CHECK: error: 'top-level-suite :: test.not-txt' would not be run indirectly
+
+# Check that no error is emitted with --no-indirectly-run-check.
+#
+# RUN: %{lit} \
+# RUN:     %{inputs}/discovery/test.not-txt -j 1 --no-indirectly-run-check
 
 # Check that we don't recurse infinitely when loading an site specific test
 # suite located inside the test source root.
@@ -156,4 +169,4 @@
 # CHECK-ASEXEC-INTREE-NEXT:     Available Features:
 # CHECK-ASEXEC-INTREE-NEXT:     Available Substitutions:
 # CHECK-ASEXEC-INTREE-NEXT: -- Available Tests --
-# CHECK-ASEXEC-INTREE-NEXT: exec-discovery-in-tree-suite :: test-one
+# CHECK-ASEXEC-INTREE-NEXT: exec-discovery-in-tree-suite :: test-one
\ No newline at end of file

diff  --git a/llvm/utils/lit/tests/unit/TestRunner.py b/llvm/utils/lit/tests/unit/TestRunner.py
index 33e4ebc716f4..411146effb1c 100644
--- a/llvm/utils/lit/tests/unit/TestRunner.py
+++ b/llvm/utils/lit/tests/unit/TestRunner.py
@@ -40,7 +40,7 @@ def load_keyword_parser_lit_tests():
         test_path = os.path.dirname(os.path.dirname(__file__))
         inputs = [os.path.join(test_path, 'Inputs/testrunner-custom-parsers/')]
         assert os.path.isdir(inputs[0])
-        tests = lit.discovery.find_tests_for_inputs(lit_config, inputs)
+        tests = lit.discovery.find_tests_for_inputs(lit_config, inputs, False)
         assert len(tests) == 1 and "there should only be one test"
         TestIntegratedTestKeywordParser.inputTestCase = tests[0]
 
@@ -291,4 +291,4 @@ def test_recursive_substitution_invalid_value(self):
 
 if __name__ == '__main__':
     TestIntegratedTestKeywordParser.load_keyword_parser_lit_tests()
-    unittest.main(verbosity=2)
+    unittest.main(verbosity=2)
\ No newline at end of file


        


More information about the llvm-commits mailing list