Add tests and workflow for package build / testing

[luca-della-vedova]

Based of #19, I can merge it there to have a single PR with functionality and tests or keep it separate and retarget main once #19 is merged.

Note that CI will be red until the upstream colcon/colcon-cargo#39 is merged, since this PR adds a test for colcon test, which is not implemented right now.
Furthermore, it currently uses a proposed strategy for injecting setup commands into the colcon reusable workflow in colcon/ci#33. If the PR is merged we should revert to main, if other strategies are proposed we should adopt them before merging this in.

[luca-della-vedova]

If no activity happens on the upstream colcon/ci PR I'll proceed and copy the action with the required change in this repo to make the workflow green

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@1d890ff). Learn more about missing BASE report.

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #22   +/-   ##
=======================================
  Coverage        ?   62.20%           
=======================================
  Files           ?        8           
  Lines           ?      381           
  Branches        ?       62           
=======================================
  Hits            ?      237           
  Misses          ?      136           
  Partials        ?        8           

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

[luca-della-vedova]

As mentioned in #22 (comment) I proceeded to copy the action here in 0abad0d, we can revisit that if some feedback comes from the upstream reusable workflow maintainers

It seems cargo-ament-build doesn't deal with binaries on Windows properly which makes the CI fail there. I created a temporary branch and added it to the CI to prove that it changes it from red to green in 566e6f9.

PR to merge the changes back to main and make sure this package works on Windows in ros2-rust/cargo-ament-build#8.

Once the cargo-ament-build PR is merged (and cargo-ament-build released) I'll revert the custom branch and this should be good to go

[mxgrey]

Just curious, whose name is this and how did it land here? I feel hesitant to use the name of someone who's not involved in the project.

[luca-della-vedova]

I copied the package from colcon-cargo hence kept the original attribution.
It is true that at the end of the day it's just an empty template where the only code contribution was made by me though (adding code that passes / fails test)

[mxgrey]

I see, it's an interesting dilemma. On one hand, I wouldn't want to use someone's contact info without their explicit consent. On the other hand, it could be interpreted as an act of attribution, as you're saying.

Since we're using the same code in the same way that the original author intended it, just in a different (but strongly related) package, I suppose I don't object to keeping it as-is. But if anyone has a different perspective, I'd appreciate hearing it.

I'll tag @bergercookie and maybe if he sees the notification, he can give a 👍/👎 on the usage.

[esteve]

Since this sample package is not meant to be published as is, I'd change the author and email to something non-specific (Test McTestface notareal@email.address) to avoid attribution, but also prevent @bergercookie getting spammed. I'd do the same for package.xml.

[luca-della-vedova]

a62ff22

[maspe36]

LGTM

[maspe36]

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.

[luca-della-vedova]

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