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

Skip install step when a package has nothing to install #40

Merged

Conversation

luca-della-vedova
Copy link
Member

As mentioned in #39 (comment). Originally authored in luca-della-vedova#2 by @mxgrey, I only contributed unit tests to make sure the functionality works as intended and no regressions are added unintentionally.

Original PR description:

While testing out colcon test for colcon-cargo, I found that we get a build error from colcon build if there's a Rust package that does not contain any binary crates (i.e. it only contains a library crate).

This PR avoids that problem by checking each cargo package before running cargo install to ensure it contains at least one binary crate. If there are no binary crates in the package then we simply skip the cargo install step to avoid the spurious error code.

With this, we can successfully use colcon build followed by colcon test to run tests for library-only cargo packages.

mxgrey and others added 6 commits August 23, 2024 09:37
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>
Signed-off-by: Michael X. Grey <mxgrey@intrinsic.ai>
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 76.47059% with 4 lines in your changes missing coverage. Please review.

Project coverage is 66.96%. Comparing base (48cd670) to head (18ad6b5).

Files Patch % Lines
colcon_cargo/task/cargo/build.py 76.47% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #40      +/-   ##
==========================================
+ Coverage   66.02%   66.96%   +0.93%     
==========================================
  Files           6        6              
  Lines         209      224      +15     
  Branches       30       36       +6     
==========================================
+ Hits          138      150      +12     
- Misses         50       52       +2     
- Partials       21       22       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@luca-della-vedova
Copy link
Member Author

The functionality is quite uncontroversial and with good coverage, but I'll leave the PR open for some time in case some other members of the community want to pitch in @maspe36, @esteve @jhdcs

@luca-della-vedova luca-della-vedova merged commit 79d4e88 into colcon:main Aug 26, 2024
17 checks passed
@luca-della-vedova luca-della-vedova deleted the luca/cargo_test_support branch August 26, 2024 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants