search

ros-infrastructure / rosdoc2

Command-line tool for generating documentation for ROS 2 packages.
Apache License 2.0
29 stars 9 forks source link

Add build tests #70

Closed rkent closed 3 months ago

rkent commented 3 months ago

A couple of years ago, I experimented with a number of expansions of rosdoc2 to make more of the documentation of a project, in standard locations, available by default. It is a lot easier to do these changes if there are unit tests that can demonstrate and test various features. This PR adds such build tests by adding and testing several test packages.

The 'full_package' test packages includes a lot of documentation features that are not currently implemented in rosdoc2, but I hope to add in future PRs.

The inclusion of an existing demo package in the test cases might be controversial rather than starting from scratch. I'm open to suggestions there.

tfoote commented 3 months ago

Thanks for the updates. I think that testing it on real packages as you were doing earlier is valuable too. To that end could you add a workflow which will clone a few selected packages and just run on those and check the return values etc. It will be more of an integration/system test than what's implemented here as unit tests.

rkent commented 3 months ago

"could you add a workflow which will clone a few selected packages"

That's a pretty significant change, and I am reluctant do it for several reasons. One important value of these unit tests is to the developer, as you can isolate issues that you want to address in a demo package, then add the needed changes with a quick change/run cycle by running the tests. Real packages with potential issues could be large enough to slow this down significantly, which makes development slower. Also, I've had the experience of trying to chase test regressions, and adding uncontrolled external clones is asking for many unpleasant hours of trying to figure out if a regression is due to some problem in rosdoc2, or to that external package. External packages tests with rosdoc2 could be useful, but I think they belong either in the test suite of the external package, or in a dedicated build farm job.

If you really want clones of external packages, could we do that as a follow-up PR and land this one? This is a pivotal PR for me to make progress, as any rosdoc2 issue I would address would typically have changes to the tests, so until this is landed I am blocked from more important changes.

ros-discourse commented 3 months ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/rosdoc2-undergoing-significant-change/36905/2