Skip to content

Commit

Permalink
[lit] Remove the --no-indirectly-run-check option
Browse files Browse the repository at this point in the history
This option had originally been added in D83069 to allow disabling the
check that something is going to get run at all when a specific test name
is used on the command-line. Since we now use getTestsForPath() (from D151664)
to get the tests to run for a specific path, we don't need a specific check
for this anymore -- Lit will produce the same complaint it would produce if
you provided a directory with no tests.

If one needs to run a specific test on the command-line and the Lit
configuration would normally not include that test, the configuration
should be set up as a "standalone" configuration or it should be fixed
to allow for that test to be found (i.e. probably fix the allowed test
suffixes).

Differential Revision: https://reviews.llvm.org/D153967
  • Loading branch information
ldionne committed Jul 17, 2023
1 parent 1cb3fbc commit 49b209d
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 63 deletions.
10 changes: 2 additions & 8 deletions llvm/docs/CommandGuide/lit.rst
Expand Up @@ -163,11 +163,6 @@ EXECUTION OPTIONS

Exit with status zero even if some tests fail.

.. option:: --no-indirectly-run-check

Do not error if a test would not be run if the user had specified the
containing directory instead of naming the test directly.

.. _selection-options:

SELECTION OPTIONS
Expand Down Expand Up @@ -458,9 +453,8 @@ executed, two important global variables are predefined:
tests in the suite.

**standalone_tests** When true, mark a directory with tests expected to be run
standalone. Test discovery is disabled for that directory and
*--no-indirectly-run-check* is in effect. *lit.suffixes* and *lit.excludes*
must be empty when this variable is true.
standalone. Test discovery is disabled for that directory. *lit.suffixes* and
*lit.excludes* must be empty when this variable is true.

**suffixes** For **lit** test formats which scan directories for tests, this
variable is a list of suffixes to identify test files. Used by: *ShTest*.
Expand Down
2 changes: 1 addition & 1 deletion llvm/utils/lit/lit/LitTestCase.py
Expand Up @@ -58,7 +58,7 @@ def load_test_suite(inputs):
)

# Perform test discovery.
tests = lit.discovery.find_tests_for_inputs(lit_config, inputs, False)
tests = lit.discovery.find_tests_for_inputs(lit_config, inputs)
test_adaptors = [LitTestCase(t, lit_config) for t in tests]

# Return a unittest test suite which just runs the tests in order.
Expand Down
8 changes: 0 additions & 8 deletions llvm/utils/lit/lit/cl_arguments.py
Expand Up @@ -190,14 +190,6 @@ def parse_args():
action="store_true",
help="Exit with status zero even if some tests fail",
)
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(
Expand Down
47 changes: 8 additions & 39 deletions llvm/utils/lit/lit/discovery.py
Expand Up @@ -130,7 +130,7 @@ def search(path_in_suite):
return search(path_in_suite)


def getTests(path, litConfig, testSuiteCache, localConfigCache, indirectlyRunCheck):
def getTests(path, litConfig, testSuiteCache, localConfigCache):
# Find the test suite for this input and its relative path.
ts, path_in_suite = getTestSuite(path, litConfig, testSuiteCache)
if ts is None:
Expand All @@ -146,12 +146,11 @@ def getTests(path, litConfig, testSuiteCache, localConfigCache, indirectlyRunChe
litConfig,
testSuiteCache,
localConfigCache,
indirectlyRunCheck,
)


def getTestsInSuite(
ts, path_in_suite, litConfig, testSuiteCache, localConfigCache, indirectlyRunCheck
ts, path_in_suite, litConfig, testSuiteCache, localConfigCache
):
# Check that the source path exists (errors here are reported by the
# caller).
Expand All @@ -164,41 +163,14 @@ def getTestsInSuite(
test_dir_in_suite = path_in_suite[:-1]
lc = getLocalConfig(ts, test_dir_in_suite, litConfig, localConfigCache)

# TODO: Stop checking for indirectlyRunCheck and lc.standalone_tests here
# once we remove --no-indirectly-run-check, which is not needed anymore
# now that we error out when trying to run a test that wouldn't be
# discovered in the directory.
fallbackOnSingleTest = lc.test_format is None or not indirectlyRunCheck or lc.standalone_tests
tests = [Test.Test(ts, path_in_suite, lc)] if fallbackOnSingleTest else \
# If we don't have a test format or if we are running standalone tests,
# always "find" the test itself. Otherwise, we might find no tests at
# all, which is considered an error but isn't an error with standalone
# tests.
tests = [Test.Test(ts, path_in_suite, lc)] if lc.test_format is None or lc.standalone_tests else \
lc.test_format.getTestsForPath(ts, path_in_suite, litConfig, lc)

for test in tests:
# 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.
# This check can be disabled by using --no-indirectly-run-check or
# setting the standalone_tests variable in the suite's configuration.
if (
indirectlyRunCheck
and lc.test_format is not None
and not lc.standalone_tests
):
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"
"(e.g. suffixes or standalone_tests variables)" % test.getFullName()
)

yield test
return

Expand Down Expand Up @@ -260,7 +232,6 @@ def getTestsInSuite(
litConfig,
testSuiteCache,
localConfigCache,
indirectlyRunCheck,
)
else:
subiter = getTestsInSuite(
Expand All @@ -269,7 +240,6 @@ def getTestsInSuite(
litConfig,
testSuiteCache,
localConfigCache,
indirectlyRunCheck,
)

N = 0
Expand All @@ -280,7 +250,7 @@ def getTestsInSuite(
litConfig.warning("test suite %r contained no tests" % sub_ts.name)


def find_tests_for_inputs(lit_config, inputs, indirectlyRunCheck):
def find_tests_for_inputs(lit_config, inputs):
"""
find_tests_for_inputs(lit_config, inputs) -> [Test]
Expand Down Expand Up @@ -315,7 +285,6 @@ def find_tests_for_inputs(lit_config, inputs, indirectlyRunCheck):
lit_config,
test_suite_cache,
local_config_cache,
indirectlyRunCheck,
)[1]
)
if prev == len(tests):
Expand Down
3 changes: 3 additions & 0 deletions llvm/utils/lit/lit/formats/base.py
Expand Up @@ -12,6 +12,9 @@ def getTestsForPath(self, testSuite, path_in_suite, litConfig, localConfig):
to that path. There can be zero, one or more tests. For example, some testing
formats allow expanding a single path in the test suite into multiple Lit tests
(e.g. they are generated on the fly).
Note that this method is only used when Lit needs to actually perform test
discovery, which is not the case for configs with standalone tests.
"""
yield lit.Test.Test(testSuite, path_in_suite, localConfig)

Expand Down
2 changes: 1 addition & 1 deletion llvm/utils/lit/lit/main.py
Expand Up @@ -44,7 +44,7 @@ def main(builtin_params={}):
)

discovered_tests = lit.discovery.find_tests_for_inputs(
lit_config, opts.test_paths, opts.indirectlyRunCheck
lit_config, opts.test_paths
)
if not discovered_tests:
sys.stderr.write("error: did not discover any tests for provided path(s)\n")
Expand Down
5 changes: 0 additions & 5 deletions llvm/utils/lit/tests/discovery.py
Expand Up @@ -144,11 +144,6 @@
# CHECK-ERROR-INPUT-CONTAINED-NO-TESTS: warning: input 'Inputs/discovery/test.not-txt' contained no tests
# CHECK-ERROR-INPUT-CONTAINED-NO-TESTS: error: did not discover any tests for provided path(s)

# Check that no error is emitted with --no-indirectly-run-check.
#
# RUN: %{lit} \
# RUN: %{inputs}/discovery/test.not-txt --no-indirectly-run-check

# Check that a standalone test with no suffixes set is run without any errors.
#
# RUN: %{lit} %{inputs}/standalone-tests/true.txt > %t.out
Expand Down
2 changes: 1 addition & 1 deletion llvm/utils/lit/tests/unit/TestRunner.py
Expand Up @@ -45,7 +45,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, False)
tests = lit.discovery.find_tests_for_inputs(lit_config, inputs)
assert len(tests) == 1 and "there should only be one test"
TestIntegratedTestKeywordParser.inputTestCase = tests[0]

Expand Down

0 comments on commit 49b209d

Please sign in to comment.