Unverified Commit 441702c6 authored by James Smith's avatar James Smith Committed by GitHub

Adds per test timeouts inside `run_all_tests.py` (#549)

# Description
This PR:
* enables per test timeout setting.
* reduces default timeout to 2 mins (120 secs)
* sets `test_environments.py` to have a timeout of 20 mins (1200 secs)
* mounts `tools` dir into docker container 

Fixes #548

<!-- As a practice, it is recommended to open an issue to have
discussions on the proposed pull request.
This makes it easier for the community to keep track of what is being
developed or added, and if a given feature
is demanded by more than one party. -->

## Type of change
- New feature (non-breaking change which adds functionality)

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [x] I have run all the tests with `./isaaclab.sh --test` and they pass
- [ ] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there
parent d651b829
...@@ -40,6 +40,9 @@ x-default-isaac-lab-volumes: &default-isaac-lab-volumes ...@@ -40,6 +40,9 @@ x-default-isaac-lab-volumes: &default-isaac-lab-volumes
- type: bind - type: bind
source: ../docs source: ../docs
target: ${DOCKER_ISAACLAB_PATH}/docs target: ${DOCKER_ISAACLAB_PATH}/docs
- type: bind
source: ../tools
target: ${DOCKER_ISAACLAB_PATH}/tools
# The effect of these volumes is twofold: # The effect of these volumes is twofold:
# 1. Prevent root-owned files from flooding the _build and logs dir # 1. Prevent root-owned files from flooding the _build and logs dir
# on the host machine # on the host machine
......
# Copyright (c) 2022-2024, The Isaac Lab Project Developers.
# All rights reserved.
#
# SPDX-License-Identifier: BSD-3-Clause
"""A dictionary of tests and their timeouts in seconds.
Any tests not listed here will use the default timeout.
"""
PER_TEST_TIMEOUTS = {
"test_environments.py": 1200, # This test runs through all the environments for 100 steps each
}
...@@ -30,12 +30,16 @@ from datetime import datetime ...@@ -30,12 +30,16 @@ from datetime import datetime
from pathlib import Path from pathlib import Path
from prettytable import PrettyTable from prettytable import PrettyTable
# Tests to skip # Local imports
from per_test_timeouts import PER_TEST_TIMEOUTS
from tests_to_skip import TESTS_TO_SKIP from tests_to_skip import TESTS_TO_SKIP
ISAACLAB_PATH = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) ISAACLAB_PATH = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
"""Path to the root directory of the Isaac Lab repository.""" """Path to the root directory of the Isaac Lab repository."""
DEFAULT_TIMEOUT = 120
"""The default timeout for each test in seconds."""
def parse_args() -> argparse.Namespace: def parse_args() -> argparse.Namespace:
"""Parse command line arguments.""" """Parse command line arguments."""
...@@ -65,7 +69,7 @@ def parse_args() -> argparse.Namespace: ...@@ -65,7 +69,7 @@ def parse_args() -> argparse.Namespace:
) )
parser.add_argument("--discover_only", action="store_true", help="Only discover and print tests, don't run them.") parser.add_argument("--discover_only", action="store_true", help="Only discover and print tests, don't run them.")
parser.add_argument("--quiet", action="store_true", help="Don't print to console, only log to file.") parser.add_argument("--quiet", action="store_true", help="Don't print to console, only log to file.")
parser.add_argument("--timeout", type=int, default=1200, help="Timeout for each test in seconds.") parser.add_argument("--timeout", type=int, default=DEFAULT_TIMEOUT, help="Timeout for each test in seconds.")
# parse arguments # parse arguments
args = parser.parse_args() args = parser.parse_args()
return args return args
...@@ -75,7 +79,8 @@ def test_all( ...@@ -75,7 +79,8 @@ def test_all(
test_dir: str, test_dir: str,
tests_to_skip: list[str], tests_to_skip: list[str],
log_path: str, log_path: str,
timeout: float = 1200.0, timeout: float = DEFAULT_TIMEOUT,
per_test_timeouts: dict[str, float] = {},
discover_only: bool = False, discover_only: bool = False,
quiet: bool = False, quiet: bool = False,
) -> bool: ) -> bool:
...@@ -85,7 +90,9 @@ def test_all( ...@@ -85,7 +90,9 @@ def test_all(
test_dir: Path to the directory containing the tests. test_dir: Path to the directory containing the tests.
tests_to_skip: List of tests to skip. tests_to_skip: List of tests to skip.
log_path: Path to the log file to store the results in. log_path: Path to the log file to store the results in.
timeout: Timeout for each test in seconds. Defaults to 600 seconds (10 minutes). timeout: Timeout for each test in seconds. Defaults to DEFAULT_TIMEOUT.
per_test_timeouts: A dictionary of tests and their timeouts in seconds. Any tests not listed here will use the
timeout specified by `timeout`. Defaults to an empty dictionary.
discover_only: If True, only discover and print the tests without running them. Defaults to False. discover_only: If True, only discover and print the tests without running them. Defaults to False.
quiet: If False, print the output of the tests to the terminal console (in addition to the log file). quiet: If False, print the output of the tests to the terminal console (in addition to the log file).
Defaults to False. Defaults to False.
...@@ -134,11 +141,20 @@ def test_all( ...@@ -134,11 +141,20 @@ def test_all(
test_paths.sort() test_paths.sort()
skipped_test_paths.sort() skipped_test_paths.sort()
# Initialize all tests to have the same timeout
test_timeouts = {test_path: timeout for test_path in all_test_paths}
# Overwrite timeouts for specific tests
for test_path_with_timeout, test_timeout in per_test_timeouts.items():
for test_path in all_test_paths:
if test_path_with_timeout in test_path:
test_timeouts[test_path] = test_timeout
# Print tests to be run # Print tests to be run
logging.info("\n" + "=" * 60 + "\n") logging.info("\n" + "=" * 60 + "\n")
logging.info(f"The following {len(all_test_paths)} tests were found:") logging.info(f"The following {len(all_test_paths)} tests were found:")
for i, test_path in enumerate(all_test_paths): for i, test_path in enumerate(all_test_paths):
logging.info(f"{i + 1:02d}: {test_path}") logging.info(f"{i + 1:02d}: {test_path}, timeout: {test_timeouts[test_path]}")
logging.info("\n" + "=" * 60 + "\n") logging.info("\n" + "=" * 60 + "\n")
logging.info(f"The following {len(skipped_test_paths)} tests are marked to be skipped:") logging.info(f"The following {len(skipped_test_paths)} tests are marked to be skipped:")
...@@ -160,7 +176,7 @@ def test_all( ...@@ -160,7 +176,7 @@ def test_all(
logging.info(f"[INFO] Running '{test_path}'\n") logging.info(f"[INFO] Running '{test_path}'\n")
try: try:
completed_process = subprocess.run( completed_process = subprocess.run(
[sys.executable, test_path], check=True, capture_output=True, timeout=timeout [sys.executable, test_path], check=True, capture_output=True, timeout=test_timeouts[test_path]
) )
except subprocess.TimeoutExpired as e: except subprocess.TimeoutExpired as e:
logging.error(f"Timeout occurred: {e}") logging.error(f"Timeout occurred: {e}")
...@@ -177,8 +193,8 @@ def test_all( ...@@ -177,8 +193,8 @@ def test_all(
except Exception as e: except Exception as e:
logging.error(f"Unexpected exception {e}. Please report this issue on the repository.") logging.error(f"Unexpected exception {e}. Please report this issue on the repository.")
result = "FAILED" result = "FAILED"
stdout = e.stdout stdout = str(e)
stderr = e.stderr stderr = str(e)
else: else:
# Should only get here if the process ran successfully, e.g. no exceptions were raised # Should only get here if the process ran successfully, e.g. no exceptions were raised
# but we still check the returncode just in case # but we still check the returncode just in case
...@@ -189,8 +205,21 @@ def test_all( ...@@ -189,8 +205,21 @@ def test_all(
after = time.time() after = time.time()
time_elapsed = after - before time_elapsed = after - before
# Decode stdout and stderr and write to file and print to console if desired # Decode stdout and stderr and write to file and print to console if desired
stdout_str = stdout.decode("utf-8") if stdout is not None else "" if stdout is not None:
stderr_str = stderr.decode("utf-8") if stderr is not None else "" if isinstance(stdout, str):
stdout_str = stdout
else:
stdout_str = stdout.decode("utf-8")
else:
stdout = ""
if stderr is not None:
if isinstance(stderr, str):
stderr_str = stderr
else:
stderr_str = stderr.decode("utf-8")
else:
stderr = ""
# Write to log file # Write to log file
logging.info(stdout_str) logging.info(stdout_str)
logging.info(stderr_str) logging.info(stderr_str)
...@@ -264,12 +293,14 @@ if __name__ == "__main__": ...@@ -264,12 +293,14 @@ if __name__ == "__main__":
# add tests to skip to the list of tests to skip # add tests to skip to the list of tests to skip
tests_to_skip = TESTS_TO_SKIP tests_to_skip = TESTS_TO_SKIP
tests_to_skip += args.skip_tests tests_to_skip += args.skip_tests
# run all tests # run all tests
test_success = test_all( test_success = test_all(
test_dir=args.test_dir, test_dir=args.test_dir,
tests_to_skip=tests_to_skip, tests_to_skip=tests_to_skip,
log_path=args.log_path, log_path=args.log_path,
timeout=args.timeout, timeout=args.timeout,
per_test_timeouts=PER_TEST_TIMEOUTS,
discover_only=args.discover_only, discover_only=args.discover_only,
quiet=args.quiet, quiet=args.quiet,
) )
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment