From 5201b283507b35b0c776755e500936cdc2c4e09d Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Tue, 23 Jul 2024 16:12:37 +0800 Subject: [PATCH 01/10] Add tests for building and testing packages Signed-off-by: Luca Della Vedova --- colcon_ros_cargo/task/ament_cargo/build.py | 7 +- test/rust-sample-package/Cargo.lock | 7 ++ test/rust-sample-package/Cargo.toml | 9 ++ test/rust-sample-package/package.xml | 11 ++ test/rust-sample-package/src/lib.rs | 5 + test/rust-sample-package/src/main.rs | 17 +++ test/test_build.py | 124 +++++++++++++++++++++ 7 files changed, 177 insertions(+), 3 deletions(-) create mode 100644 test/rust-sample-package/Cargo.lock create mode 100644 test/rust-sample-package/Cargo.toml create mode 100644 test/rust-sample-package/package.xml create mode 100644 test/rust-sample-package/src/lib.rs create mode 100644 test/rust-sample-package/src/main.rs create mode 100644 test/test_build.py diff --git a/colcon_ros_cargo/task/ament_cargo/build.py b/colcon_ros_cargo/task/ament_cargo/build.py index 6fd0826..2ef3bf5 100644 --- a/colcon_ros_cargo/task/ament_cargo/build.py +++ b/colcon_ros_cargo/task/ament_cargo/build.py @@ -105,7 +105,8 @@ def write_cargo_config_toml(package_paths): config_dir.mkdir(exist_ok=True) cargo_config_toml_out = config_dir / 'config.toml' cargo_config_toml_out.unlink(missing_ok=True) - toml.dump(content, cargo_config_toml_out.open('w')) + with cargo_config_toml_out.open('w') as toml_file: + toml.dump(content, toml_file) def find_installed_cargo_packages(env): @@ -118,8 +119,8 @@ def find_installed_cargo_packages(env): prefix_for_package = {} ament_prefix_path_var = env.get('AMENT_PREFIX_PATH') if ament_prefix_path_var is None: - logger.warn('AMENT_PREFIX_PATH is empty. ' - 'You probably intended to source a ROS installation.') + logger.warning('AMENT_PREFIX_PATH is empty. ' + 'You probably intended to source a ROS installation.') prefixes = [] else: prefixes = ament_prefix_path_var.split(os.pathsep) diff --git a/test/rust-sample-package/Cargo.lock b/test/rust-sample-package/Cargo.lock new file mode 100644 index 0000000..e3300c9 --- /dev/null +++ b/test/rust-sample-package/Cargo.lock @@ -0,0 +1,7 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "rust-sample-package" +version = "0.1.0" diff --git a/test/rust-sample-package/Cargo.toml b/test/rust-sample-package/Cargo.toml new file mode 100644 index 0000000..273b638 --- /dev/null +++ b/test/rust-sample-package/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "rust-sample-package" +version = "0.1.0" +authors = ["Nikos Koukis "] +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/test/rust-sample-package/package.xml b/test/rust-sample-package/package.xml new file mode 100644 index 0000000..bc6f8b9 --- /dev/null +++ b/test/rust-sample-package/package.xml @@ -0,0 +1,11 @@ + + rust-sample-package + 0.0.1 + A sample package for testing + Luca Della Vedova + Apache License 2.0 + + + ament_cargo + + 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 new file mode 100644 index 0000000..9b813e5 --- /dev/null +++ b/test/rust-sample-package/src/main.rs @@ -0,0 +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/test_build.py b/test/test_build.py new file mode 100644 index 0000000..d80e977 --- /dev/null +++ b/test/test_build.py @@ -0,0 +1,124 @@ +# Copyright 2024 Open Source Robotics Foundation, Inc. +# Licensed under the Apache License, Version 2.0 + +import asyncio +import os +from pathlib import Path +import shutil +import tempfile +from types import SimpleNamespace +import xml.etree.ElementTree as eTree + +from colcon_core.event_handler.console_direct import ConsoleDirectEventHandler +from colcon_core.package_descriptor import PackageDescriptor +from colcon_core.subprocess import new_event_loop +from colcon_core.task import TaskContext +from colcon_ros_cargo.package_identification.ament_cargo import AmentCargoPackageIdentification # noqa: E501 +from colcon_ros_cargo.task.ament_cargo.build import AmentCargoBuildTask +from colcon_ros_cargo.task.ament_cargo.test import AmentCargoTestTask +import pytest + +TEST_PACKAGE_NAME = 'rust-sample-package' + +test_project_path = Path(__file__).parent / TEST_PACKAGE_NAME + + +@pytest.fixture(autouse=True) +def monkey_patch_put_event_into_queue(monkeypatch): + event_handler = ConsoleDirectEventHandler() + monkeypatch.setattr( + TaskContext, + 'put_event_into_queue', + lambda self, event: event_handler((event, 'cargo')), + ) + + +def test_package_identification(): + cpi = AmentCargoPackageIdentification() + desc = PackageDescriptor(test_project_path) + cpi.identify(desc) + assert desc.type == 'ament_cargo' + assert desc.name == TEST_PACKAGE_NAME + + +@pytest.mark.skipif( + not shutil.which('cargo'), + reason='Rust must be installed to run this test') +def test_build_and_test_package(): + event_loop = new_event_loop() + asyncio.set_event_loop(event_loop) + + try: + cpi = AmentCargoPackageIdentification() + package = PackageDescriptor(test_project_path) + cpi.identify(package) + + with tempfile.TemporaryDirectory() as tmpdir: + tmpdir = Path(tmpdir) + # TODO(luca) Also test clean build and cargo args + context = TaskContext(pkg=package, + args=SimpleNamespace( + path=str(test_project_path), + build_base=str(tmpdir / 'build'), + install_base=str(tmpdir / 'install'), + clean_build=None, + cargo_args=None, + lookup_in_workspace=None, + ), + dependencies={} + ) + + task = AmentCargoBuildTask() + task.set_context(context=context) + + src_base = test_project_path / 'src' + + source_files_before = set(src_base.rglob('*')) + rc = event_loop.run_until_complete(task.build()) + assert not rc + source_files_after = set(src_base.rglob('*')) + assert source_files_before == source_files_after + + # Make sure the binary is compiled + install_base = Path(task.context.args.install_base) + app_name = TEST_PACKAGE_NAME + executable = TEST_PACKAGE_NAME + # Executable in windows have a .exe extension + if os.name == 'nt': + executable += '.exe' + assert (install_base / 'lib' / app_name / executable).is_file() + + # Now compile tests + task = AmentCargoTestTask() + 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 From 09ea5907e6a93db422bbe0d182ebfc43b065935b Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Tue, 23 Jul 2024 16:21:08 +0800 Subject: [PATCH 02/10] Ubuntu 20.04 compatibility Signed-off-by: Luca Della Vedova --- colcon_ros_cargo/task/ament_cargo/build.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/colcon_ros_cargo/task/ament_cargo/build.py b/colcon_ros_cargo/task/ament_cargo/build.py index 2ef3bf5..3f7540f 100644 --- a/colcon_ros_cargo/task/ament_cargo/build.py +++ b/colcon_ros_cargo/task/ament_cargo/build.py @@ -2,6 +2,7 @@ import os from pathlib import Path +import sys from colcon_cargo.task.cargo import CARGO_EXECUTABLE from colcon_cargo.task.cargo.build import CargoBuildTask @@ -104,7 +105,15 @@ def write_cargo_config_toml(package_paths): config_dir = Path.cwd() / '.cargo' config_dir.mkdir(exist_ok=True) cargo_config_toml_out = config_dir / 'config.toml' - cargo_config_toml_out.unlink(missing_ok=True) + # missing_ok flag was introduced in Python 3.8, older versions + # raise a FileNotFoundError exception + if sys.version_info.minor >= 8: + cargo_config_toml_out.unlink(missing_ok=True) + else: + try: + cargo_config_toml_out.unlink() + except FileNotFoundError: + pass with cargo_config_toml_out.open('w') as toml_file: toml.dump(content, toml_file) From 11fb75ca39c591bc6d85696e62c5fada4b291d11 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Tue, 23 Jul 2024 16:28:14 +0800 Subject: [PATCH 03/10] Add rust to action Signed-off-by: Luca Della Vedova --- .github/actions/local_pytest/action.yml | 11 +++++++++++ .github/workflows/ci.yaml | 14 +++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 .github/actions/local_pytest/action.yml diff --git a/.github/actions/local_pytest/action.yml b/.github/actions/local_pytest/action.yml new file mode 100644 index 0000000..9d585b3 --- /dev/null +++ b/.github/actions/local_pytest/action.yml @@ -0,0 +1,11 @@ +inputs: + codecov_token: + description: Codecov secret token + required: true + +runs: + using: composite + steps: + - uses: colcon/ci/.github/workflows/pytest.yaml@main + env: + CODECOV_TOKEN: ${{ inputs.codecov_token }} diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ec8f82b..222d428 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -8,6 +8,14 @@ on: jobs: pytest: - uses: colcon/ci/.github/workflows/pytest.yaml@main - secrets: - CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + repository: ${{ inputs.repository }} + # - run: cargo install cargo-ament-build + # Use local action because running a reusable workflow as a step + # is not currently possible and we need to install cargo-ament-build + # for the CI to be successful + # ref https://github.com/actions/runner/issues/2079 + - uses: ./.github/actions/local_pytest From 466f910ef02a8147ac5963b16542bc88b2eb9a6e Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Tue, 23 Jul 2024 17:40:26 +0800 Subject: [PATCH 04/10] Switch to custom workflow Signed-off-by: Luca Della Vedova --- .github/workflows/ci.yaml | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 222d428..b81aea9 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -8,14 +8,8 @@ on: jobs: pytest: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - repository: ${{ inputs.repository }} - # - run: cargo install cargo-ament-build - # Use local action because running a reusable workflow as a step - # is not currently possible and we need to install cargo-ament-build - # for the CI to be successful - # ref https://github.com/actions/runner/issues/2079 - - uses: ./.github/actions/local_pytest + uses: luca-della-vedova/ci/.github/workflows/pytest.yaml@add_init + with: + prerun-step: 'cargo install cargo-ament-build' + secrets: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} From 7faaa0f084e2dc44c850c0c70950d2b94be6389a Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Fri, 23 Aug 2024 09:52:30 +0800 Subject: [PATCH 05/10] Remove lockfile Signed-off-by: Luca Della Vedova --- test/rust-sample-package/Cargo.lock | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 test/rust-sample-package/Cargo.lock diff --git a/test/rust-sample-package/Cargo.lock b/test/rust-sample-package/Cargo.lock deleted file mode 100644 index e3300c9..0000000 --- a/test/rust-sample-package/Cargo.lock +++ /dev/null @@ -1,7 +0,0 @@ -# This file is automatically @generated by Cargo. -# It is not intended for manual editing. -version = 3 - -[[package]] -name = "rust-sample-package" -version = "0.1.0" From 0abad0d384233c600c90690236c73bd9671ca8e4 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Fri, 23 Aug 2024 10:00:56 +0800 Subject: [PATCH 06/10] Vendor reusable CI from colcon/ci Signed-off-by: Luca Della Vedova --- .github/workflows/ci.yaml | 2 +- .github/workflows/pytest.yaml | 76 +++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/pytest.yaml diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b81aea9..219e157 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -8,7 +8,7 @@ on: jobs: pytest: - uses: luca-della-vedova/ci/.github/workflows/pytest.yaml@add_init + uses: ./.github/workflows/pytest.yaml with: prerun-step: 'cargo install cargo-ament-build' secrets: diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml new file mode 100644 index 0000000..0907086 --- /dev/null +++ b/.github/workflows/pytest.yaml @@ -0,0 +1,76 @@ +# Forked from colcon/ci to allow cargo install step, ref https://github.com/colcon/ci/pull/33 +# TODO(anyone) revisit if a better solution pops up from the upstream PR +--- +name: Run tests + +on: # yamllint disable-line rule:truthy + workflow_call: + inputs: + codecov: + description: 'run codecov action after testing' + default: true + required: false + type: boolean + matrix-filter: + description: 'jq filter string indicating which configuration(s) + should be included' + default: '.' + required: false + type: string + repository: + description: 'repository to test if different from current' + default: '' + required: false + type: string + setup-repository: + description: 'repository used during job setup' + default: 'colcon/ci' + required: false + type: string + prerun-step: + description: 'instruction to run before the testing' + default: '' + required: false + type: string + secrets: + CODECOV_TOKEN: + description: 'token to use when running codecov action after testing' + required: false + +jobs: + setup: + runs-on: ubuntu-latest + outputs: + strategy: ${{ steps.load.outputs.strategy }} + steps: + - uses: actions/checkout@v4 + with: + repository: ${{ inputs.setup-repository }} + - id: load + run: | + strategy=$(jq -c -M '${{ inputs.matrix-filter }}' strategy.json) + echo "strategy=${strategy}" >> $GITHUB_OUTPUT + + pytest: + needs: [setup] + strategy: ${{ fromJson(needs.setup.outputs.strategy) }} + runs-on: ${{ matrix.os }} + + steps: + - uses: actions/checkout@v4 + with: + repository: ${{ inputs.repository }} + - run: bash -c "${{ inputs.prerun-step }}" + if: ${{ inputs.prerun-step != ''}} + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python }} + - uses: actions/checkout@v4 + with: + repository: ${{ inputs.setup-repository }} + path: ./.github-ci-action-repo + - uses: ./.github-ci-action-repo + - uses: codecov/codecov-action@v4 + env: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + if: ${{ inputs.codecov }} From 566e6f967ff913a86ede7b76fa068d7575541c54 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Fri, 23 Aug 2024 10:39:12 +0800 Subject: [PATCH 07/10] Temporary branch for cargo-ament-build Signed-off-by: Luca Della Vedova --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 219e157..5f1c7dc 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -10,6 +10,6 @@ jobs: pytest: uses: ./.github/workflows/pytest.yaml with: - prerun-step: 'cargo install cargo-ament-build' + prerun-step: 'cargo install --git https://github.com/luca-della-vedova/cargo-ament-build.git --branch luca/binaries_on_windows' secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} From 3fda2b03bc6c5197f28ff048d325d49d1c27efe9 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Mon, 26 Aug 2024 17:12:57 +0800 Subject: [PATCH 08/10] Revert "Temporary branch for cargo-ament-build" This reverts commit 566e6f967ff913a86ede7b76fa068d7575541c54. Signed-off-by: Luca Della Vedova --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 5f1c7dc..219e157 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -10,6 +10,6 @@ jobs: pytest: uses: ./.github/workflows/pytest.yaml with: - prerun-step: 'cargo install --git https://github.com/luca-della-vedova/cargo-ament-build.git --branch luca/binaries_on_windows' + prerun-step: 'cargo install cargo-ament-build' secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} From a62ff2221b0038b922d99702c9b2dd66bce91289 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Mon, 26 Aug 2024 17:16:10 +0800 Subject: [PATCH 09/10] Change authors to non-real address Signed-off-by: Luca Della Vedova --- test/rust-sample-package/Cargo.toml | 2 +- test/rust-sample-package/package.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/rust-sample-package/Cargo.toml b/test/rust-sample-package/Cargo.toml index 273b638..259e0d8 100644 --- a/test/rust-sample-package/Cargo.toml +++ b/test/rust-sample-package/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "rust-sample-package" version = "0.1.0" -authors = ["Nikos Koukis "] +authors = ["Test McTestface "] edition = "2018" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/test/rust-sample-package/package.xml b/test/rust-sample-package/package.xml index bc6f8b9..3b19611 100644 --- a/test/rust-sample-package/package.xml +++ b/test/rust-sample-package/package.xml @@ -2,7 +2,7 @@ rust-sample-package 0.0.1 A sample package for testing - Luca Della Vedova + Test McTestface Apache License 2.0 From 0612fda1daa42202bdc2b89e30c75b1f2e11bdbf Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Tue, 27 Aug 2024 11:50:07 +0800 Subject: [PATCH 10/10] Remove cargo config file deletion Signed-off-by: Luca Della Vedova --- colcon_ros_cargo/task/ament_cargo/build.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/colcon_ros_cargo/task/ament_cargo/build.py b/colcon_ros_cargo/task/ament_cargo/build.py index 3f7540f..52b128a 100644 --- a/colcon_ros_cargo/task/ament_cargo/build.py +++ b/colcon_ros_cargo/task/ament_cargo/build.py @@ -2,7 +2,6 @@ import os from pathlib import Path -import sys from colcon_cargo.task.cargo import CARGO_EXECUTABLE from colcon_cargo.task.cargo.build import CargoBuildTask @@ -105,15 +104,6 @@ def write_cargo_config_toml(package_paths): config_dir = Path.cwd() / '.cargo' config_dir.mkdir(exist_ok=True) cargo_config_toml_out = config_dir / 'config.toml' - # missing_ok flag was introduced in Python 3.8, older versions - # raise a FileNotFoundError exception - if sys.version_info.minor >= 8: - cargo_config_toml_out.unlink(missing_ok=True) - else: - try: - cargo_config_toml_out.unlink() - except FileNotFoundError: - pass with cargo_config_toml_out.open('w') as toml_file: toml.dump(content, toml_file)