Skip to content

Commit

Permalink
[lit] Move the recursiveExpansionLimit setting to TestingConfig
Browse files Browse the repository at this point in the history
The LitConfig is shared across the whole test suite. However, since
enabling recursive expansion can be a breaking change for some test
suites, it's important to confine the setting to test suites that
enable it explicitly.

Note that other issues were raised with the way recursiveExpansionLimit
operates. However, this commit simply moves the setting to the right
place -- the mechanism by which it works can be improved independently.

Differential Revision: https://reviews.llvm.org/D77415
  • Loading branch information
ldionne committed Apr 6, 2020
1 parent 6ddc525 commit 8a42bf2
Show file tree
Hide file tree
Showing 13 changed files with 37 additions and 29 deletions.
3 changes: 3 additions & 0 deletions libcxx/test/lit.cfg
Expand Up @@ -20,6 +20,9 @@ config.suffixes = ['.pass.cpp', '.fail.cpp', '.sh.cpp', '.pass.mm']
# test_source_root: The root path where tests are located.
config.test_source_root = os.path.dirname(__file__)

# Allow expanding substitutions that are based on other substitutions
config.recursiveExpansionLimit = 10

loaded_site_cfg = getattr(config, 'loaded_site_config', False)
if not loaded_site_cfg:
import libcxx.test.config
Expand Down
4 changes: 2 additions & 2 deletions libcxx/utils/libcxx/test/format.py
Expand Up @@ -132,13 +132,13 @@ def _execute(self, test, lit_config):

# Apply substitutions in FILE_DEPENDENCIES markup
data_files = lit.TestRunner.applySubstitutions(test.file_dependencies, substitutions,
recursion_limit=10)
recursion_limit=test.config.recursiveExpansionLimit)
local_cwd = os.path.dirname(test.getSourcePath())
data_files = [f if os.path.isabs(f) else os.path.join(local_cwd, f) for f in data_files]
substitutions.append(('%{file_dependencies}', ' '.join(data_files)))

script = lit.TestRunner.applySubstitutions(script, substitutions,
recursion_limit=10)
recursion_limit=test.config.recursiveExpansionLimit)

test_cxx = copy.deepcopy(self.cxx)
if is_fail_test:
Expand Down
5 changes: 2 additions & 3 deletions libcxx/utils/libcxx/test/newformat.py
Expand Up @@ -191,7 +191,6 @@ def addCompileFlags(self, config, *flags):

# Modified version of lit.TestRunner.executeShTest to handle custom parsers correctly.
def _executeShTest(self, test, litConfig, steps, fileDependencies=None):
recursiveExpansionLimit = 10 # TODO: Use the value in litConfig once we set it.
if test.config.unsupported:
return lit.Test.Result(lit.Test.UNSUPPORTED, 'Test is unsupported')

Expand Down Expand Up @@ -229,7 +228,7 @@ def _executeShTest(self, test, litConfig, steps, fileDependencies=None):
# need to resolve %{file_dependencies} now, because otherwise we won't be able to
# make all paths absolute below.
fileDependencies = lit.TestRunner.applySubstitutions(fileDependencies, substitutions,
recursion_limit=recursiveExpansionLimit)
recursion_limit=test.config.recursiveExpansionLimit)

# Add the %{file_dependencies} substitution before we perform substitutions
# inside the script.
Expand All @@ -239,6 +238,6 @@ def _executeShTest(self, test, litConfig, steps, fileDependencies=None):

# Perform substitution in the script itself.
script = lit.TestRunner.applySubstitutions(script, substitutions,
recursion_limit=recursiveExpansionLimit)
recursion_limit=test.config.recursiveExpansionLimit)

return lit.TestRunner._runShTest(test, litConfig, useExternalSh, script, tmpBase)
6 changes: 6 additions & 0 deletions libcxxabi/test/lit.cfg
Expand Up @@ -20,9 +20,15 @@ config.name = 'libc++abi'
# suffixes: A list of file extensions to treat as test files.
config.suffixes = ['.cpp', '.s']

# Allow expanding substitutions that are based on other substitutions
config.recursiveExpansionLimit = 10

# test_source_root: The root path where tests are located.
config.test_source_root = os.path.dirname(__file__)

# Allow expanding substitutions that are based on other substitutions
config.recursiveExpansionLimit = 10

# Infer the libcxx_test_source_root for configuration import.
# If libcxx_source_root isn't specified in the config, assume that the libcxx
# and libcxxabi source directories are sibling directories.
Expand Down
8 changes: 4 additions & 4 deletions llvm/docs/CommandGuide/lit.rst
Expand Up @@ -457,10 +457,10 @@ modules :ref:`local-configuration-files`.
By default, substitutions are expanded exactly once, so that if e.g. a
substitution ``%build`` is defined in top of another substitution ``%cxx``,
``%build`` will expand to ``%cxx`` textually, not to what ``%cxx`` expands to.
However, if the ``recursiveExpansionLimit`` property of the ``LitConfig`` is
set to a non-negative integer, substitutions will be expanded recursively until
that limit is reached. It is an error if the limit is reached and expanding
substitutions again would yield a different result.
However, if the ``recursiveExpansionLimit`` property of the ``TestingConfig``
is set to a non-negative integer, substitutions will be expanded recursively
until that limit is reached. It is an error if the limit is reached and
expanding substitutions again would yield a different result.

More detailed information on substitutions can be found in the
:doc:`../TestingGuide`.
Expand Down
13 changes: 0 additions & 13 deletions llvm/utils/lit/lit/LitConfig.py
Expand Up @@ -66,19 +66,6 @@ def __init__(self, progname, path, quiet,
self.maxIndividualTestTime = maxIndividualTestTime
self.parallelism_groups = parallelism_groups
self.echo_all_commands = echo_all_commands
self._recursiveExpansionLimit = None

@property
def recursiveExpansionLimit(self):
return self._recursiveExpansionLimit

@recursiveExpansionLimit.setter
def recursiveExpansionLimit(self, value):
if value is not None and not isinstance(value, int):
self.fatal('recursiveExpansionLimit must be either None or an integer (got <{}>)'.format(value))
if isinstance(value, int) and value < 0:
self.fatal('recursiveExpansionLimit must be a non-negative integer (got <{}>)'.format(value))
self._recursiveExpansionLimit = value

@property
def maxIndividualTestTime(self):
Expand Down
2 changes: 1 addition & 1 deletion llvm/utils/lit/lit/TestRunner.py
Expand Up @@ -1552,6 +1552,6 @@ def executeShTest(test, litConfig, useExternalSh,
substitutions += getDefaultSubstitutions(test, tmpDir, tmpBase,
normalize_slashes=useExternalSh)
script = applySubstitutions(script, substitutions,
recursion_limit=litConfig.recursiveExpansionLimit)
recursion_limit=test.config.recursiveExpansionLimit)

return _runShTest(test, litConfig, useExternalSh, script, tmpBase)
15 changes: 14 additions & 1 deletion llvm/utils/lit/lit/TestingConfig.py
Expand Up @@ -2,7 +2,7 @@
import sys


class TestingConfig:
class TestingConfig(object):
""""
TestingConfig - Information on the tests inside a suite.
"""
Expand Down Expand Up @@ -126,6 +126,19 @@ def __init__(self, parent, name, suffixes, test_format,
# Whether the suite should be tested early in a given run.
self.is_early = bool(is_early)
self.parallelism_group = parallelism_group
self._recursiveExpansionLimit = None

@property
def recursiveExpansionLimit(self):
return self._recursiveExpansionLimit

@recursiveExpansionLimit.setter
def recursiveExpansionLimit(self, value):
if value is not None and not isinstance(value, int):
raise ValueError('recursiveExpansionLimit must be either None or an integer (got <{}>)'.format(value))
if isinstance(value, int) and value < 0:
raise ValueError('recursiveExpansionLimit must be a non-negative integer (got <{}>)'.format(value))
self._recursiveExpansionLimit = value

def finish(self, litConfig):
"""finish() - Finish this config object, after loading is complete."""
Expand Down
Expand Up @@ -9,4 +9,4 @@ config.substitutions = [("%rec1", "STOP"), ("%rec2", "%rec1"),
("%rec3", "%rec2"), ("%rec4", "%rec3"),
("%rec5", "%rec4")]

lit_config.recursiveExpansionLimit = 2
config.recursiveExpansionLimit = 2
Expand Up @@ -5,4 +5,4 @@ config.test_format = lit.formats.ShTest()
config.test_source_root = None
config.test_exec_root = None

lit_config.recursiveExpansionLimit = -4
config.recursiveExpansionLimit = -4
Expand Up @@ -5,4 +5,4 @@ config.test_format = lit.formats.ShTest()
config.test_source_root = None
config.test_exec_root = None

lit_config.recursiveExpansionLimit = "not-an-integer"
config.recursiveExpansionLimit = "not-an-integer"
Expand Up @@ -5,4 +5,4 @@ config.test_format = lit.formats.ShTest()
config.test_source_root = None
config.test_exec_root = None

lit_config.recursiveExpansionLimit = None
config.recursiveExpansionLimit = None
Expand Up @@ -9,4 +9,4 @@ config.substitutions = [("%rec1", "STOP"), ("%rec2", "%rec1"),
("%rec3", "%rec2"), ("%rec4", "%rec3"),
("%rec5", "%rec4")]

lit_config.recursiveExpansionLimit = 5
config.recursiveExpansionLimit = 5

0 comments on commit 8a42bf2

Please sign in to comment.