use colcon option for pytest coverage instead of custom pytest-cov options

[mikaelarguedas]

This results in the following pytest-args:

• --cov=<PATH/TO/ROS/PACKAGE>
• --cov-report=html:<PATH/TO/PKG/BUILD/DIR>/coverage.html
• --cov-report=xml:<PATH/TO/PKG/BUILD/DIR>/coverage.xml
• --cov-branch

https://github.com/colcon/colcon-core/blob/af7fa089962a9e9a35926734e17b00cfb4c780a6/colcon_core/task/python/test/pytest.py#L85-L100

The main advantage is that it prevents action-ros-ci to overwrite the output location of the coverage.xml (colcon places it in the build directory of the package) and to have branch coverage in the report.

Note: this needs colcon-core 0.5.1 so doesnt work as is as setup-ros pins colcon version to 0.4.3.
Should I open a separate issue to request a version bump of colcon packages in setup-ros?

[piraka9011]

Should I open a separate issue to request a version bump of colcon packages in setup-ros?

Yes but you will need to update the associated colcon extensions as well to make sure they are also compatible with the version you are pinning against.
They are minor version bumps but they do introduce breaking changes FYI.

[piraka9011]

The CI seems to be failing for a different reason...

[thomas-moulard]

You'll need to revert that now :)

[mikaelarguedas]

yup! just wanted to get someone to witness it did pass CI before removing that commit ;)

[mikaelarguedas]

Commit removed

[Arnatious]

This has been stale for about a month now, can we get this merged in? It's quite important for being able to get codecov results up.

[christophebedard]

This is just a suggestion, but you could also use the coverage-pytest mixin: https://github.com/colcon/colcon-mixin-repository/blob/1ddb69bedfd1f04c2f000e95452f7c24a4d6176b/coverage.mixin#L14

[mikaelarguedas]

Indeed I created the coverage-pytest mixin at the same time as this PR because I was not a fan of the way it was hard-coded in here.

I haven't tested recently but IIRC the fact that action-ros-ci hard-codes the pytest-cov parameters was still a problem even though there is a mixin for it.

I could update this PR to make use of the mixin by default, but given that this has been months approved and not merged for no apparent reason I'm not convinced it's worth the effort.

[mikaelarguedas]

@emersonknapp @thomas-moulard is there anything I can do to get this over the line ?

[thomas-moulard]

@mikaelarguedas thanks for the reminder, updated the branch, let's merge if the CI is OK.

[mikaelarguedas]

2 jobs failed. They don't seem related to this change.
MacOS latest -> fail on vcs import
log_workflow_status_to_cloudwatch -> looks like a missing argument: Input required and not supplied: aws-region

[piraka9011]

@mikaelarguedas Now that #247 is in, are there any changes that need to be made to this?

[mikaelarguedas]

Not that I know of. I'm not familiar with colcon-coveragepy-result but I would assume it picks up the coverage files inside the packages as well, @christophebedard would know more.

[christophebedard]

Not that I know of. I'm not familiar with colcon-coveragepy-result but I would assume it picks up the coverage files inside the packages as well

Yep! It was made to work with the usual way(s) colcon and ament set up Python code coverage generation, so it's all good.

[piraka9011]

@christophebedard to clarify: All good as in no longer needed or all good as in this is still necessary? :)

It seems that colcon-coveragepy-result will pick up the coverage files so we don't need to specify them here.

[christophebedard]

@christophebedard to clarify: All good as in no longer needed or all good as in this is still necessary? :)

It seems that colcon-coveragepy-result will pick up the coverage files so we don't need to specify them here.

@piraka9011 sorry! Not sure what you mean by "don't need to specify them here," but this PR is still necessary. We still need to tell colcon to tell ament to generate Python code coverage using the default behaviour/settings. Otherwise, with the way it is set up without this PR, colcon-coveragepy-result won't be able to find the data.