Add AMENT_LINT_AUTO_SKIP_PREEXISTING_TESTS flag

[xfiderek]

Summary

Added a flag AMENT_LINT_AUTO_SKIP_PREEXISTING_TESTS to every test in the repo. If the flag is defined and set to ON, then already added tests (matched by name) are ignored. This is contrary to default behavior of ament_lint_auto, as by default duplicate test names cause build failure.
The code is backward compatible -> added piece of code has no effect, unless the flag is set.

Use case

This flag helps to solve space-ros/space-ros#57. The main idea for resolution of that issue is to inject additional cmake script during build using CMAKE_PROJECT_INCLUDE variable, that will add linters to packages. Check linked issue and README in the PR for more details.

[fujitatomoya]

CC: @mjcarroll /assign

[mjcarroll]

I'm not sure I fully understand what you are trying to accomplish here. Can you expand a bit more on the use case and what you are trying to solve with this flag?

[xfiderek]

Sure!

So basically we want to add functionality to spaceros, that will allow users to overwrite linters defined in CMakeLists.txt files of upstream packages, to ensure that all ros packages are analysed with the same set of linters.

One way to solve that would be to modify CMakeLists.txt files directly. What we plan to do instead is to inject cmake script during build process of upstream packages using CMAKE_PROJECT_INCLUDE variable. In that script, we would specify linters by using ament_lint_auto and calling find_package() on linters that we want to add (e.g. ament_cobra, ament_cppcheck).

Lets now imagine that an upstream package already includes one of our linters, lets say ament_cppcheck. Prior to this PR, the build would fail because the same test would have been defined twice -> once in our script and once in original CMakeLists.txt.
After this PR, when we set the flag to true, the test added in CMakeLists.txt will be ignored, hence avoiding duplication error.

Hope it makes sense. Ive tried to find another approach that would allow us to achieve this functionality, but i couldnt really come up with simpler and cleaner altenrative that does not involve modifying cmakelists.txt files of upstream packages.

[Bckempa]

Hey ament team, sorry for the bump - I fully recognize how taxing and thankless maintaining these low-level infrastructure packages can be - but this is blocking two other PRs which are now going to miss this quarter's space-ros release and leave our static analysis coverage compromised. Is there anything we can do to support you in progressing this review?

[mjcarroll]

Thanks. I'm clear on the use case now. Can you add an entry to the docs to describe the use of that variable. What you have stated here is sufficient for me, just to make sure it is documented.

I would recommend here: https://github.com/ament/ament_lint/blob/rolling/ament_lint_auto/doc/index.rst

[xfiderek]

Hi, sorry for the delay on my side. Thanks a lot for the review, I have uploaded the documentation as you suggested.
Also, note that this PR targets spaceros branch, in case you wonder why documentation is shorter than in the file you linked above.

Ideally we would update spaceros branch to include the latest changes to the ament repo, but we should probably handle that as a separate issue :).

[xfiderek]

Hey @mjcarroll, could you take a look at my changes?

[Bckempa]

@mjcarroll it's time for our next release, will you have time to take a look soon?