diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 07136dcad..da33b0cff 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -219,12 +219,10 @@ Or you can run them on a fish, without involving cmake:: cargo build cargo test # for the unit tests - tests/test_driver.py --cachedir=/tmp target/debug # for the script and interactive tests + tests/test_driver.py target/debug # for the script and interactive tests Here, the first argument to test_driver.py refers to a directory with ``fish``, ``fish_indent`` and ``fish_key_reader`` in it. In this example we're in the root of the git repo and have run ``cargo build`` without ``--release``, so it's a debug build. -The ``--cachedir /tmp`` argument means it will keep the fish_test_helper binary in /tmp instead of recompiling it for every test. -This saves some time, but isn't strictly necessary. Git hooks --------- diff --git a/build_tools/check.sh b/build_tools/check.sh index e1be44e54..f2e3327f0 100755 --- a/build_tools/check.sh +++ b/build_tools/check.sh @@ -15,5 +15,4 @@ cargo test --no-default-features --workspace --all-targets cargo test --doc --workspace cargo doc --workspace -# TODO: parallelize "$repo_root/tests/test_driver.py" "$build_dir" diff --git a/cmake/Tests.cmake b/cmake/Tests.cmake index 8bc525039..849a6c7e4 100644 --- a/cmake/Tests.cmake +++ b/cmake/Tests.cmake @@ -63,7 +63,7 @@ foreach(CHECK ${FISH_CHECKS}) get_filename_component(CHECK_NAME ${CHECK} NAME) get_filename_component(CHECK ${CHECK} NAME_WE) add_test(NAME ${CHECK_NAME} - COMMAND ${CMAKE_SOURCE_DIR}/tests/test_driver.py --cachedir=${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_BINARY_DIR} + COMMAND ${CMAKE_SOURCE_DIR}/tests/test_driver.py ${CMAKE_CURRENT_BINARY_DIR} checks/${CHECK}.fish WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests ) @@ -76,7 +76,7 @@ FILE(GLOB PEXPECTS CONFIGURE_DEPENDS ${CMAKE_SOURCE_DIR}/tests/pexpects/*.py) foreach(PEXPECT ${PEXPECTS}) get_filename_component(PEXPECT ${PEXPECT} NAME) add_test(NAME ${PEXPECT} - COMMAND ${CMAKE_SOURCE_DIR}/tests/test_driver.py --cachedir=${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_BINARY_DIR} + COMMAND ${CMAKE_SOURCE_DIR}/tests/test_driver.py ${CMAKE_CURRENT_BINARY_DIR} pexpects/${PEXPECT} WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/tests ) diff --git a/tests/littlecheck.py b/tests/littlecheck.py index 8c025e20b..62c872075 100755 --- a/tests/littlecheck.py +++ b/tests/littlecheck.py @@ -4,20 +4,21 @@ from __future__ import unicode_literals from __future__ import print_function - import argparse +import asyncio import datetime +from difflib import SequenceMatcher import io import re import shlex -import subprocess import sys +import unicodedata try: from itertools import zip_longest except ImportError: from itertools import izip_longest as zip_longest -from difflib import SequenceMatcher + # Directives can occur at the beginning of a line, or anywhere in a line that does not start with #. COMMENT_RE = r"^(?:[^#].*)?#\s*" @@ -93,9 +94,6 @@ def output(*args): print("".join(args) + "\n") -import unicodedata - - def esc(m): map = { "\n": "\\n", @@ -382,28 +380,28 @@ def perform_substitution(input_str, subs): return re.sub(r"%(%|[a-zA-Z0-9_-]+)", subber, input_str) -def runproc(cmd): +def runproc(cmd, env=None): """Wrapper around subprocess.Popen to save typing""" - PIPE = subprocess.PIPE - proc = subprocess.Popen( - cmd, - stdin=PIPE, - stdout=PIPE, - stderr=PIPE, - shell=True, - close_fds=True, # For Python 2.6 as shipped on RHEL 6 + return asyncio.run(runproc_async(cmd, env=env)) + + +async def runproc_async(cmd, env=None): + """Wrapper around subprocess.Popen to save typing""" + PIPE = asyncio.subprocess.PIPE + return await asyncio.create_subprocess_shell( + cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE, env=env ) - return proc class TestRun(object): - def __init__(self, name, runcmd, checker, subs, config): + def __init__(self, name, runcmd, checker, subs, config, env=None): self.name = name self.runcmd = runcmd self.subbed_command = perform_substitution(runcmd.args, subs) self.checker = checker self.subs = subs self.config = config + self.env = env def check(self, lines, checks): # Reverse our lines and checks so we can pop off the end. @@ -477,6 +475,10 @@ class TestRun(object): def run(self): """Run the command. Return a TestFailure, or None.""" + return asyncio.run(self.run_async()) + + async def run_async(self): + """Run the command. Return a TestFailure, or None.""" def split_by_newlines(s): """Decode a string and split it by newlines only, @@ -489,8 +491,8 @@ class TestRun(object): if self.config.verbose: print(self.subbed_command) - proc = runproc(self.subbed_command) - stdout, stderr = proc.communicate() + proc = await runproc_async(self.subbed_command, env=self.env) + stdout, stderr = await proc.communicate() # HACK: This is quite cheesy: POSIX specifies that sh should return 127 for a missing command. # It's also possible that it'll be returned in other situations, # most likely when the last command in a shell script doesn't exist. @@ -668,7 +670,14 @@ class Checker(object): ] -def check_file(input_file, name, subs, config, failure_handler): +def check_file(input_file, name, subs, config, failure_handler, env=None): + """Check a single file. Return a True on success, False on error.""" + return asyncio.run( + check_file_async(input_file, name, subs, config, failure_handler, env=env) + ) + + +async def check_file_async(input_file, name, subs, config, failure_handler, env=None): """Check a single file. Return a True on success, False on error.""" success = True lines = Line.readfile(input_file, name) @@ -677,8 +686,8 @@ def check_file(input_file, name, subs, config, failure_handler): # Run all the REQUIRES lines first, # if any of them fail it's a SKIP for reqcmd in checker.requirecmds: - proc = runproc(perform_substitution(reqcmd.args, subs)) - proc.communicate() + proc = await runproc_async(perform_substitution(reqcmd.args, subs), env=env) + await proc.communicate() if proc.returncode > 0: return SKIP @@ -687,16 +696,22 @@ def check_file(input_file, name, subs, config, failure_handler): # Only then run the RUN lines. for runcmd in checker.runcmds: - failure = TestRun(name, runcmd, checker, subs, config).run() + failure = await TestRun( + name, runcmd, checker, subs, config, env=env + ).run_async() if failure: failure_handler(failure) success = False return success -def check_path(path, subs, config, failure_handler): +def check_path(path, subs, config, failure_handler, env=None): + return asyncio.run(check_path_async(path, subs, config, failure_handler, env=env)) + + +async def check_path_async(path, subs, config, failure_handler, env=None): with io.open(path, encoding="utf-8") as fd: - return check_file(fd, path, subs, config, failure_handler) + return await check_file_async(fd, path, subs, config, failure_handler, env=env) def parse_subs(subs): diff --git a/tests/test_driver.py b/tests/test_driver.py index 4a02c05bc..b59696771 100755 --- a/tests/test_driver.py +++ b/tests/test_driver.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 import argparse +import asyncio from dataclasses import dataclass from datetime import datetime import os @@ -25,49 +26,29 @@ BLUE = "\033[34m" RED = "\033[31m" -def makeenv(script_path, home, test_helper_path): - xdg_config = home + "/xdg_config_home" - func_dir = xdg_config + "/fish/functions" +def makeenv(script_path: Path, home: Path) -> dict[str, str]: + xdg_config = home / "xdg_config_home" + func_dir = xdg_config / "fish" / "functions" os.makedirs(func_dir) - os.makedirs(xdg_config + "/fish/conf.d/") + os.makedirs(xdg_config / "fish" / "conf.d") for func in (script_path / "test_functions").glob("*.fish"): - shutil.copy(func, func_dir + "/" + func.parts[-1]) + shutil.copy(func, func_dir / func.parts[-1]) shutil.copy( - script_path / "interactive.config", xdg_config + "/fish/conf.d/interactive.fish" + script_path / "interactive.config", + xdg_config / "fish" / "conf.d" / "interactive.fish", ) - xdg_data = home + "/xdg_data_home" + xdg_data = home / "xdg_data_home" os.makedirs(xdg_data) - xdg_runtime = home + "/xdg_runtime_home" + xdg_runtime = home / "xdg_runtime_home" os.makedirs(xdg_runtime) - xdg_cache = home + "/xdg_cache_home" + xdg_cache = home / "xdg_cache_home" os.makedirs(xdg_cache) - tmp = home + "/temp" + tmp = home / "temp" os.makedirs(tmp) - # Compile fish_test_helper if necessary. - # If we're run multiple times, allow keeping this around to save time. - if test_helper_path: - thp = Path(test_helper_path) - if not os.path.exists(thp / "fish_test_helper"): - subprocess.run( - [ - "cc", - script_path / "fish_test_helper.c", - "-o", - thp / "fish_test_helper", - ] - ) - shutil.copy(thp / "fish_test_helper", home + "/fish_test_helper") - else: - subprocess.run( - [ - "cc", - script_path / "fish_test_helper.c", - "-o", - home + "/fish_test_helper", - ] - ) + # Set up the environment variables for the test. + env = os.environ.copy() # unset LANG, TERM, ... for var in [ @@ -80,30 +61,44 @@ def makeenv(script_path, home, test_helper_path): "TERM_PROGRAM_VERSION", "VTE_VERSION", ]: - if var in os.environ: - del os.environ[var] + if var in env: + del env[var] langvars = [key for key in os.environ.keys() if key.startswith("LC_")] for key in langvars: - del os.environ[key] + del env[key] - os.environ.update( + env.update( { - "HOME": home, - "TMPDIR": tmp, + "HOME": str(home), + "TMPDIR": str(tmp), "FISH_FAST_FAIL": "1", "FISH_UNIT_TESTS_RUNNING": "1", - "XDG_CONFIG_HOME": xdg_config, - "XDG_DATA_HOME": xdg_data, - "XDG_RUNTIME_DIR": xdg_runtime, - "XDG_CACHE_HOME": xdg_cache, - "fish_test_helper": home + "/fish_test_helper", + "XDG_CONFIG_HOME": str(xdg_config), + "XDG_DATA_HOME": str(xdg_data), + "XDG_RUNTIME_DIR": str(xdg_runtime), + "XDG_CACHE_HOME": str(xdg_cache), + "fish_test_helper": str(home.parent / "fish_test_helper"), "LANG": "C", "LC_CTYPE": "en_US.UTF-8", } ) + return env -def main(): + +def compile_test_helper(source_path: Path, binary_path: Path) -> None: + subprocess.run( + [ + "cc", + source_path, + "-o", + binary_path, + ], + check=True, + ) + + +async def main(): if len(sys.argv) < 2: print("Usage: test_driver.py FISH_DIRECTORY TESTS") return 1 @@ -113,14 +108,6 @@ def main(): argparser = argparse.ArgumentParser( description="test_driver: Run fish's test suite" ) - argparser.add_argument( - "-f", - "--cachedir", - type=str, - help="Path to keep outputs to speed up the next run", - action="store", - default=None, - ) argparser.add_argument("fish", nargs=1, help="Fish to test") argparser.add_argument("file", nargs="*", help="Tests to run") args = argparser.parse_args() @@ -174,28 +161,36 @@ def main(): f"{arg.ljust(longest_test_name_length)} {color}{result}{RESET} {duration_str}{suffix_str}" ) - tmp_root = tempfile.mkdtemp(prefix="fishtest-root-") + with tempfile.TemporaryDirectory(prefix="fishtest-root-") as tmp_root: + tmp_root = Path(tmp_root) - for f, arg in files: - match run_test(tmp_root, f, arg, script_path, args, def_subs, lconfig, fishdir): - case TestSkip(arg): - skipcount += 1 - print_result(arg, "SKIPPED", BLUE) - case TestFail(arg, duration_ms, error_message): - failcount += 1 - failed += [arg] - print_result(arg, "FAILED", RED, duration_ms, error_message) - case TestPass(arg, duration_ms): - passcount += 1 - print_result(arg, "PASSED", GREEN, duration_ms) + compile_test_helper( + script_path / "fish_test_helper.c", + tmp_root / "fish_test_helper", + ) - shutil.rmtree(tmp_root) + tasks = [ + run_test(tmp_root, f, arg, script_path, def_subs, lconfig, fishdir) + for f, arg in files + ] + for task in asyncio.as_completed(tasks): + match await task: + case TestSkip(arg): + skipcount += 1 + print_result(arg, "SKIPPED", BLUE) + case TestFail(arg, duration_ms, error_message): + failcount += 1 + failed += [arg] + print_result(arg, "FAILED", RED, duration_ms, error_message) + case TestPass(arg, duration_ms): + passcount += 1 + print_result(arg, "PASSED", GREEN, duration_ms) if passcount + failcount + skipcount > 1: print(f"{passcount} / {passcount + failcount} passed ({skipcount} skipped)") if failcount: failstr = "\n ".join(failed) - print(f"{RED}Failed tests{RESET}: \n {failstr}") + print(f"{RED}Failed tests{RESET}:\n {failstr}") if passcount == 0 and failcount == 0 and skipcount: return 125 return 1 if failcount else 0 @@ -222,22 +217,35 @@ class TestPass: TestResult = TestSkip | TestFail | TestPass -def run_test( - tmp_root, path, arg, script_path, args, def_subs, lconfig, fishdir +async def run_test( + tmp_root: Path, + test_file_path: str, + arg, + script_path: Path, + def_subs, + lconfig, + fishdir, ) -> TestResult: - if not path.endswith(".fish") and not path.endswith(".py"): + if not test_file_path.endswith(".fish") and not test_file_path.endswith(".py"): return TestFail(arg, None, f"Not a valid test file: {arg}") starttime = datetime.now() - home = tempfile.mkdtemp(prefix="fishtest-", dir=tmp_root) - makeenv(script_path, home, args.cachedir) + home = Path(tempfile.mkdtemp(prefix="fishtest-", dir=tmp_root)) + test_env = makeenv(script_path, home) os.chdir(home) - if path.endswith(".fish"): + if test_file_path.endswith(".fish"): subs = def_subs.copy() - subs.update({"s": path, "fish_test_helper": home + "/fish_test_helper"}) + subs.update( + { + "s": test_file_path, + "fish_test_helper": str(tmp_root / "fish_test_helper"), + } + ) # littlecheck - ret = littlecheck.check_path(path, subs, lconfig, lambda x: print(x.message())) + ret = await littlecheck.check_path_async( + test_file_path, subs, lconfig, lambda x: print(x.message()), env=test_env + ) endtime = datetime.now() duration_ms = round((endtime - starttime).total_seconds() * 1000) if ret is littlecheck.SKIP: @@ -246,10 +254,8 @@ def run_test( return TestPass(arg, duration_ms) else: return TestFail(arg, duration_ms, f"Tmpdir is {home}") - elif path.endswith(".py"): - # environ for py files has a few changes. - pyenviron = os.environ.copy() - pyenviron.update( + elif test_file_path.endswith(".py"): + test_env.update( { "PYTHONPATH": str(script_path), "fish": str(fishdir / "fish"), @@ -261,34 +267,28 @@ def run_test( ) if not PEXPECT: return TestSkip(arg) - try: - proc = subprocess.run( - ["python3", path], - capture_output=True, - env=pyenviron, - # Timeout of 120 seconds, about 10 times what any of these takes - timeout=120, - ) - except subprocess.TimeoutExpired as e: - error_message = f"{RED}FAILED due to timeout{RESET}" - if e.output: - error_message += e.output.decode("utf-8") - if e.stderr: - error_message += e.stderr.decode("utf-8") - return TestFail(arg, None, error_message) - + PIPE = asyncio.subprocess.PIPE + proc = await asyncio.subprocess.create_subprocess_exec( + "python3", + test_file_path, + stdout=PIPE, + stderr=PIPE, + env=test_env, + ) + stdout, stderr = await proc.communicate() endtime = datetime.now() duration_ms = round((endtime - starttime).total_seconds() * 1000) - if proc.returncode == 0: + returncode = proc.returncode + if returncode == 0: return TestPass(arg, duration_ms) - elif proc.returncode == 127: + elif returncode == 127: return TestSkip(arg) else: error_message = "" - if proc.stdout: - error_message += proc.stdout.decode("utf-8") - if proc.stderr: - error_message += proc.stderr.decode("utf-8") + if stdout: + error_message += stdout.decode("utf-8") + if stderr: + error_message += stderr.decode("utf-8") error_message += f"Tmpdir is {home}" return TestFail(arg, duration_ms, error_message) else: @@ -297,7 +297,7 @@ def run_test( if __name__ == "__main__": try: - ret = main() + ret = asyncio.run(main()) sys.exit(ret) except KeyboardInterrupt: sys.exit(130)