From 48cd670df1bbd68270e6113968f2a0ccd4243bea Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Fri, 23 Aug 2024 09:31:13 +0800 Subject: [PATCH] Add support for cargo test (#39) * Add support for cargo test Signed-off-by: Luca Della Vedova * Switch to minidom Signed-off-by: Luca Della Vedova * Remove standalone parameter Signed-off-by: Luca Della Vedova * Add cargo args parameter Signed-off-by: Luca Della Vedova * Remove color from test output Signed-off-by: Luca Della Vedova * Remove unnecessary assertion Signed-off-by: Luca Della Vedova * Change build verb to also build tests Signed-off-by: Luca Della Vedova * Fix cargo args passing Signed-off-by: Luca Della Vedova * Change target dir to install base for tests Signed-off-by: Luca Della Vedova * Split build and install folders / steps Signed-off-by: Luca Della Vedova * Add None check Signed-off-by: Luca Della Vedova * Change back to build base Signed-off-by: Luca Della Vedova * Add lockfile to cargo install Signed-off-by: Luca Della Vedova * Simplify cargo paths Signed-off-by: Luca Della Vedova * Reduce number of built targets Signed-off-by: Luca Della Vedova * Remove outdated comment Signed-off-by: Luca Della Vedova * Remove doctests Signed-off-by: Luca Della Vedova * Add documentation Signed-off-by: Luca Della Vedova * Consistency for test error handling Signed-off-by: Luca Della Vedova * Fix README Signed-off-by: Luca Della Vedova * Revert "Consistency for test error handling" This reverts commit dc8d9c998ca17b706ca9d5a5650c0842d8ca5609. Signed-off-by: Luca Della Vedova * Remove NO_COLOR argument Signed-off-by: Luca Della Vedova * Remove TODO Signed-off-by: Luca Della Vedova --------- Signed-off-by: Luca Della Vedova --- README.md | 24 +++++++ colcon_cargo/task/cargo/build.py | 27 ++++++- colcon_cargo/task/cargo/test.py | 101 ++++++++++++++++++++++++--- test/rust-sample-package/src/lib.rs | 5 ++ test/rust-sample-package/src/main.rs | 14 ++++ test/spell_check.words | 11 +++ test/test_build.py | 37 +++++++++- 7 files changed, 207 insertions(+), 12 deletions(-) create mode 100644 test/rust-sample-package/src/lib.rs diff --git a/README.md b/README.md index 1ac7288..d6f2cd2 100644 --- a/README.md +++ b/README.md @@ -74,3 +74,27 @@ $ hello-world2 Hello, world! ``` +### Testing + +Test the packages with cargo: + +```sh +$ colcon test +Starting >>> hello_world_2 +Starting >>> hello_world +Finished <<< hello_world [0.24s] +Finished <<< hello_world_2 [0.25s] + +Summary: 2 packages finished [0.39s] +``` + +Inspect the test results (`cargo test` and `cargo fmt --check`). +They should all succeed for the empty templates: + +```sh +$ colcon test-result --all +build/hello_world_2/cargo_test.xml: 2 tests, 0 errors, 0 failures, 0 skipped +build/hello_world/cargo_test.xml: 2 tests, 0 errors, 0 failures, 0 skipped + +Summary: 4 tests, 0 errors, 0 failures, 0 skipped +``` diff --git a/colcon_cargo/task/cargo/build.py b/colcon_cargo/task/cargo/build.py index a52ed0a..46dcdf5 100644 --- a/colcon_cargo/task/cargo/build.py +++ b/colcon_cargo/task/cargo/build.py @@ -80,6 +80,17 @@ async def build( # noqa: D102 if rc and rc.returncode: return rc.returncode + cmd = self._install_cmd(cargo_args) + + self.progress('install') + + # colcon-ros-cargo overrides install command to return None + if cmd is not None: + rc = await run( + self.context, cmd, cwd=self.context.pkg.path, env=env) + if rc and rc.returncode: + return rc.returncode + if not skip_hook_creation: create_environment_scripts( self.context.pkg, args, additional_hooks=additional_hooks) @@ -97,9 +108,21 @@ def _prepare(self, env, additional_hooks): def _build_cmd(self, cargo_args): args = self.context.args return [ - CARGO_EXECUTABLE, 'install', + CARGO_EXECUTABLE, + 'build', + '--quiet', + '--target-dir', args.build_base, + ] + cargo_args + + # Overridden by colcon-ros-cargo + def _install_cmd(self, cargo_args): + args = self.context.args + return [ + CARGO_EXECUTABLE, + 'install', '--force', '--quiet', - '--path', args.path, + '--locked', + '--path', '.', '--root', args.install_base, ] + cargo_args diff --git a/colcon_cargo/task/cargo/test.py b/colcon_cargo/task/cargo/test.py index 8b1db36..d204a4b 100644 --- a/colcon_cargo/task/cargo/test.py +++ b/colcon_cargo/task/cargo/test.py @@ -2,6 +2,8 @@ # Licensed under the Apache License, Version 2.0 import os +from xml.dom import minidom +import xml.etree.ElementTree as eTree from colcon_cargo.task.cargo import CARGO_EXECUTABLE from colcon_core.event.test import TestFailure @@ -22,38 +24,119 @@ def __init__(self): # noqa: D107 satisfies_version(TaskExtensionPoint.EXTENSION_POINT_VERSION, '^1.0') def add_arguments(self, *, parser): # noqa: D102 - pass + parser.add_argument( + '--cargo-args', + nargs='*', metavar='*', type=str.lstrip, + help='Pass arguments to Cargo projects. ' + 'Arguments matching other options must be prefixed by a space,\n' + 'e.g. --cargo-args " --help"') async def test(self, *, additional_hooks=None): # noqa: D102 + """ + Runs tests and style checks for the requested package. + + Results are compiled into a single result `cargo_test.xml` file + with two test results, one for all the tests (cargo test) and one for + style (`cargo fmt --check`). + Documentation tests (`cargo test --doc`) are not implemented + since it is not possible to distinguish between a test that failed + because of a failing case and one that failed because the crate + contains no library target. + """ pkg = self.context.pkg args = self.context.args logger.info( "Testing Cargo package in '{args.path}'".format_map(locals())) - assert os.path.exists(args.build_base) + assert os.path.exists(args.build_base), \ + 'Has this package been built before?' - test_results_path = os.path.join(args.build_base, 'test_results') - os.makedirs(test_results_path, exist_ok=True) + test_results_path = os.path.join(args.build_base, 'cargo_test.xml') try: env = await get_command_environment( 'test', args.build_base, self.context.dependencies) except RuntimeError as e: + # TODO(luca) log this as error in the test result file logger.error(str(e)) return 1 if CARGO_EXECUTABLE is None: raise RuntimeError("Could not find 'cargo' executable") + cargo_args = args.cargo_args + if cargo_args is None: + cargo_args = [] + # invoke cargo test - rc = await run( + unit_rc = await run( self.context, - [CARGO_EXECUTABLE, 'test', '-q', - '--target-dir', test_results_path], - cwd=args.path, env=env) + self._test_cmd(cargo_args), + cwd=args.path, env=env, capture_output=True) + + fmt_rc = await run( + self.context, + self._fmt_cmd(), + cwd=args.path, env=env, capture_output=True) + + error_report = self._create_error_report(unit_rc, fmt_rc) + with open(test_results_path, 'wb') as result_file: + xmlstr = minidom.parseString(eTree.tostring(error_report)) + xmlstr = xmlstr.toprettyxml(indent=' ', encoding='utf-8') + result_file.write(xmlstr) - if rc.returncode: + if unit_rc.returncode or fmt_rc.returncode: self.context.put_event_into_queue(TestFailure(pkg.name)) # the return code should still be 0 return 0 + + def _test_cmd(self, cargo_args): + args = self.context.args + return [ + CARGO_EXECUTABLE, + 'test', + '--quiet', + '--target-dir', + args.build_base, + ] + cargo_args + [ + '--', + '--color=never', + ] + + # Ignore cargo args for rustfmt + def _fmt_cmd(self): + return [ + CARGO_EXECUTABLE, + 'fmt', + '--check', + '--', + '--color=never', + ] + + def _create_error_report(self, unit_rc, fmt_rc) -> eTree.Element: + # TODO(luca) revisit when programmatic output from cargo test is + # stabilized, for now just have a suite for unit, and fmt tests + failures = 0 + testsuites = eTree.Element('testsuites') + # TODO(luca) add time + testsuite = eTree.SubElement(testsuites, + 'testsuite', {'name': 'cargo_test'}) + unit_testcase = eTree.SubElement(testsuite, 'testcase', + {'name': 'unit'}) + if unit_rc.returncode: + unit_failure = eTree.SubElement(unit_testcase, 'failure', + {'message': 'cargo test failed'}) + unit_failure.text = unit_rc.stdout.decode('utf-8') + failures += 1 + fmt_testcase = eTree.SubElement(testsuite, 'testcase', {'name': 'fmt'}) + if fmt_rc.returncode: + fmt_failure = eTree.SubElement(fmt_testcase, 'failure', + {'message': 'cargo fmt failed'}) + fmt_failure.text = fmt_rc.stdout.decode('utf-8') + failures += 1 + testsuite.attrib['errors'] = str(0) + testsuite.attrib['failures'] = str(failures) + testsuite.attrib['skipped'] = str(0) + testsuite.attrib['tests'] = str(2) + return testsuites diff --git a/test/rust-sample-package/src/lib.rs b/test/rust-sample-package/src/lib.rs new file mode 100644 index 0000000..72835eb --- /dev/null +++ b/test/rust-sample-package/src/lib.rs @@ -0,0 +1,5 @@ +/// Failing doctest example +/// ``` +/// invalid_syntax +/// ``` +pub struct Type; diff --git a/test/rust-sample-package/src/main.rs b/test/rust-sample-package/src/main.rs index e7a11a9..9b813e5 100644 --- a/test/rust-sample-package/src/main.rs +++ b/test/rust-sample-package/src/main.rs @@ -1,3 +1,17 @@ fn main() { println!("Hello, world!"); } + +#[cfg(test)] +mod tests { + + #[test] + fn ok() -> Result<(), ()> { + Ok(()) + } + + #[test] + fn err() -> Result<(), ()> { + Err(()) + } +} diff --git a/test/spell_check.words b/test/spell_check.words index fbe19ca..1d31f2a 100644 --- a/test/spell_check.words +++ b/test/spell_check.words @@ -4,11 +4,15 @@ asyncio autouse colcon completers +deps easymov +etree +getroot iterdir linter lstrip luca +minidom monkeypatch nargs noqa @@ -20,12 +24,19 @@ returncode rglob rmtree rtype +rustfmt scspell setuptools skipif symlink tempfile +testcase +testsuite +testsuites thomas tmpdir todo toml +toprettyxml +tostring +xmlstr diff --git a/test/test_build.py b/test/test_build.py index bd67396..8fdb6ae 100644 --- a/test/test_build.py +++ b/test/test_build.py @@ -7,9 +7,11 @@ import shutil import tempfile from types import SimpleNamespace +import xml.etree.ElementTree as eTree from colcon_cargo.package_identification.cargo import CargoPackageIdentification # noqa: E501 from colcon_cargo.task.cargo.build import CargoBuildTask +from colcon_cargo.task.cargo.test import CargoTestTask from colcon_core.event_handler.console_direct import ConsoleDirectEventHandler from colcon_core.package_descriptor import PackageDescriptor from colcon_core.subprocess import new_event_loop @@ -42,7 +44,7 @@ def test_package_identification(): @pytest.mark.skipif( not shutil.which('cargo'), reason='Rust must be installed to run this test') -def test_build_package(): +def test_build_and_test_package(): event_loop = new_event_loop() asyncio.set_event_loop(event_loop) @@ -83,5 +85,38 @@ def test_build_package(): if os.name == 'nt': app_name += '.exe' assert (install_base / 'bin' / app_name).is_file() + + # Now compile tests + task = CargoTestTask() + task.set_context(context=context) + + # Expect tests to have failed but return code will still be 0 + # since testing run succeeded + rc = event_loop.run_until_complete(task.test()) + assert not rc + build_base = Path(task.context.args.build_base) + + # Make sure the testing files are built + assert (build_base / 'debug' / 'deps').is_dir() + assert len(os.listdir(build_base / 'debug' / 'deps')) > 0 + result_file_path = build_base / 'cargo_test.xml' + assert result_file_path.is_file() + check_result_file(result_file_path) + finally: event_loop.close() + + +# Check the testing result file, expect cargo test and doc test to fail +# but fmt to succeed +def check_result_file(path): + tree = eTree.parse(path) + root = tree.getroot() + testsuite = root.find('testsuite') + assert testsuite is not None + unit_result = testsuite.find("testcase[@name='unit']") + assert unit_result is not None + assert unit_result.find('failure') is not None + fmt_result = testsuite.find("testcase[@name='fmt']") + assert fmt_result is not None + assert fmt_result.find('failure') is None