[lit] Move the recursiveExpansionLimit setting to TestingConfig

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
This commit is contained in:
Louis Dionne 2020-04-03 12:49:09 -04:00
parent 6ddc525667
commit 8a42bf24ae
13 changed files with 37 additions and 29 deletions

View File

@ -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

View File

@ -132,13 +132,13 @@ class LibcxxTestFormat(object):
# 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:

View File

@ -191,7 +191,6 @@ class CxxStandardLibraryTest(lit.formats.TestFormat):
# 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')
@ -229,7 +228,7 @@ class CxxStandardLibraryTest(lit.formats.TestFormat):
# 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.
@ -239,6 +238,6 @@ class CxxStandardLibraryTest(lit.formats.TestFormat):
# 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)

View File

@ -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.

View File

@ -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`.

View File

@ -66,19 +66,6 @@ class LitConfig(object):
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):

View File

@ -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)

View File

@ -2,7 +2,7 @@ import os
import sys
class TestingConfig:
class TestingConfig(object):
""""
TestingConfig - Information on the tests inside a suite.
"""
@ -126,6 +126,19 @@ class TestingConfig:
# 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."""

View File

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

View File

@ -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

View File

@ -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"

View File

@ -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

View File

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