Add support for cargo test (#39)

* Add support for cargo test Signed-off-by: Luca Della Vedova <[email protected]> * Switch to minidom Signed-off-by: Luca Della Vedova <[email protected]> * Remove standalone parameter Signed-off-by: Luca Della Vedova <[email protected]> * Add cargo args parameter Signed-off-by: Luca Della Vedova <[email protected]> * Remove color from test output Signed-off-by: Luca Della Vedova <[email protected]> * Remove unnecessary assertion Signed-off-by: Luca Della Vedova <[email protected]> * Change build verb to also build tests Signed-off-by: Luca Della Vedova <[email protected]> * Fix cargo args passing Signed-off-by: Luca Della Vedova <[email protected]> * Change target dir to install base for tests Signed-off-by: Luca Della Vedova <[email protected]> * Split build and install folders / steps Signed-off-by: Luca Della Vedova <[email protected]> * Add None check Signed-off-by: Luca Della Vedova <[email protected]> * Change back to build base Signed-off-by: Luca Della Vedova <[email protected]> * Add lockfile to cargo install Signed-off-by: Luca Della Vedova <[email protected]> * Simplify cargo paths Signed-off-by: Luca Della Vedova <[email protected]> * Reduce number of built targets Signed-off-by: Luca Della Vedova <[email protected]> * Remove outdated comment Signed-off-by: Luca Della Vedova <[email protected]> * Remove doctests Signed-off-by: Luca Della Vedova <[email protected]> * Add documentation Signed-off-by: Luca Della Vedova <[email protected]> * Consistency for test error handling Signed-off-by: Luca Della Vedova <[email protected]> * Fix README Signed-off-by: Luca Della Vedova <[email protected]> * Revert "Consistency for test error handling" This reverts commit dc8d9c9. Signed-off-by: Luca Della Vedova <[email protected]> * Remove NO_COLOR argument Signed-off-by: Luca Della Vedova <[email protected]> * Remove TODO Signed-off-by: Luca Della Vedova <[email protected]> --------- Signed-off-by: Luca Della Vedova <[email protected]>
colcon · Aug 23, 2024 · 48cd670 · 48cd670
1 parent 9fdb14f
commit 48cd670
Show file tree

Hide file tree

Showing 7 changed files with 207 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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