Add build tests #70
Add build tests #70
Conversation
There was a problem hiding this comment.
I think that testing on real packages make sense. However I don't think that the value of having the code copied in here makes sense.
This tool is not highly productized and focused on deployment into areas where there's not network expected. So we can take advantage of that in the unit tests.
To that end I would recommend that instead of embedding the code we add to the CI to checkout a few representative samples and run the tests against those. We should use some pinned versions of those things to avoid cross coupling potential breakages. But then we don't end up with the documentation only working on some stale version that the rosdoc2 maintainers aren't familiar with.
.github/workflows/test.yml
Outdated
| strategy: | ||
| matrix: | ||
| python-version: ['3.8', '3.9'] |
There was a problem hiding this comment.
Are these platform changes necessary? It's good, but it would be better to do these changes separately.
.gitignore
Outdated
| @@ -57,3 +57,10 @@ docs/_build/ | |||
| # PyBuilder | |||
| target/ | |||
| .pytest_cache | |||
|
|
|||
| # Editors | |||
| .vscode/ | |||
There was a problem hiding this comment.
Please put these user specific changes into your user's .gitignore
|
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. |
|
"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. |
|
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 |
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.