Use Ignition tooling action for Ubuntu CI

[chapulina]

An alternative to #255 using a custom action to reduce duplication across all Ignition libraries.

Compared to that PR, this approach is missing 2 items:

• make sdf_descriptions
  • I think we can add support for building custom additional targets to the action. I just wanted to make sure I opened this PR to get feedback.
• examples compilation
  • I think this is better tested using GTest like this

As a reference, gazebosim/gz-cmake#90 uses the same action for ign-cmake.

[codecov-io]

Codecov Report

Merging #266 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #266   +/-   ##
=======================================
  Coverage   86.45%   86.45%           
=======================================
  Files          59       59           
  Lines        9051     9051           
=======================================
  Hits         7825     7825           
  Misses       1226     1226           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9eb6b99...c9c4187. Read the comment docs.

[scpeters]

I like the use of multiple steps in #255, which makes it easier to see where the job failed (during cmake, make test, etc) and reduces the amount of console log you have to scroll through. Is it possible to refactor the ignition-tooling action to support that?

[chapulina]

I like the use of multiple steps... Is it possible to refactor the ignition-tooling action to support that?

Yeah the steps are nice. I suspect this isn't possible with a docker action, but I'll research a bit.

[scpeters]

make sdf_descriptions I think we can add support for building custom additional targets to the action. I just wanted to make sure I opened this PR to get feedback.

agreed, I think you could add a hook or an environment variable for building additional targets

examples compilation: I think this is better tested using GTest like this

I think using gtest has some advantages like generating a junit file, but unless you use a FAKE_INSTALL target like ign-math, then you can't run make test without running make install. you can also execute the compiled standalone examples easily when invoked from a CI script like this, while I'm not sure that we ever execute standalone examples when using gtest. So I don't see one approach as universally better

[chapulina]

unless you use a FAKE_INSTALL target like ign-math, then you can't run make test without running make install

That's a good point. I think that would be the ideal solution.

you can also execute the compiled standalone examples easily when invoked from a CI script like this

True, we haven't been executing them with GTest. I feel that there could be a solution for this though.

So I don't see one approach as universally better

I agree with that the 2 points you brought above need to be addressed. But for completeness, I'll mention some other advantages of integrating example builds into the test suite:

• It gets run by all CI, not just actions / pipelines
• When new examples are added they're automatically tested
• Developers can identify example failures locally running make test instead of waiting for CI to point that out
• We can track test progress over time together with all other tests, to see when it started failing and so on

[scpeters]

I opened gazebo-tooling/action-gz-ci#1 and gazebo-tooling/action-gz-ci#2 to discuss the improvements that could be made to the ignition-tooling action. I'm inclined to merge #255 for now and reconsider this PR once those issues have been resolved. Your thoughts?

[chapulina]

I'm inclined to merge #255 for now and reconsider this PR once those issues have been resolved.

Sure, sounds good. I started trying out script hooks in any case because I don't want to run into this again while opening a bunch of PRs for all the libs.

I wasn't able to find an easy way to separate the steps though, so I ticketed gazebo-tooling/action-gz-ci#3.

[scpeters]

ok, I just merged #255, so you may want to merge these changes with master

[chapulina]

Thanks, @scpeters . gazebo-tooling/action-gz-ci#4 is up for review addressing your 2 issues. This PR is already using it.

[chapulina]

I had forgot about this PR. Closing for now as the Ignition Tooling action is going over some refactoring.