Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests and workflow for package build / testing #22

Merged
merged 10 commits into from
Aug 29, 2024
11 changes: 11 additions & 0 deletions .github/actions/local_pytest/action.yml
Original file line number Diff line number Diff line change
@@ -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 }}
4 changes: 3 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ on:

jobs:
pytest:
uses: colcon/ci/.github/workflows/pytest.yaml@main
uses: ./.github/workflows/pytest.yaml
with:
prerun-step: 'cargo install cargo-ament-build'
secrets:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
76 changes: 76 additions & 0 deletions .github/workflows/pytest.yaml
Original file line number Diff line number Diff line change
@@ -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 }}
18 changes: 14 additions & 4 deletions colcon_ros_cargo/task/ament_cargo/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -104,8 +105,17 @@ 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)
toml.dump(content, cargo_config_toml_out.open('w'))
# 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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we delete the previous config.tomlif it exists?

Nit: Perhaps unnecessary for folks who spend a lot of time in python, but I would suggest wrapping this in a small free function with a clear name.

def _delete_file(f) or something similar.

Also, we should probably explain why we pass on the exception
# No work for us to do, the file already doesn't exist or something like that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After looking at this further I feel the step might not be necessary, unless there is some extreme corner case, opening a file as w by default already deletes its whole content and replaces it, so I removed the whole block in 0612fda

The only case in which this would behave differently (I believe) is if users created a .cargo/config.toml that was a symlink to their own file, then ran a colcon build, previously the link would be deleted and a new config file would be created, now the symlink would be followed and edited.
I'd argue that it is such a corner case that I don't even know what the intended behavior would be

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)


def find_installed_cargo_packages(env):
Expand All @@ -118,8 +128,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)
Expand Down
9 changes: 9 additions & 0 deletions test/rust-sample-package/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[package]
name = "rust-sample-package"
version = "0.1.0"
authors = ["Test McTestface <notareal@email.address>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
11 changes: 11 additions & 0 deletions test/rust-sample-package/package.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<package format="3">
<name>rust-sample-package</name>
<version>0.0.1</version>
<description>A sample package for testing</description>
<maintainer email="notareal@email.address">Test McTestface</maintainer>
<license>Apache License 2.0</license>

<export>
<build_type>ament_cargo</build_type>
</export>
</package>
5 changes: 5 additions & 0 deletions test/rust-sample-package/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/// Failing doctest example
/// ```
/// invalid_syntax
/// ```
pub struct Type;
17 changes: 17 additions & 0 deletions test/rust-sample-package/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
fn main() {
println!("Hello, world!");
}

#[cfg(test)]
mod tests {

#[test]
fn ok() -> Result<(), ()> {
Ok(())
}

#[test]
fn err() -> Result<(), ()> {
Err(())
}
}
124 changes: 124 additions & 0 deletions test/test_build.py
Original file line number Diff line number Diff line change
@@ -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
Loading