complain if kcov_tool fails

[rpoyner-tri]

This change is 

[rpoyner-tri]

I'll be keeping this in draft form for a bit. it turns out that kcov has been failing for two reasons: (1) kcov_tool bugs fixed in RobotLocomotion/drake#21859, and (2) bazel_tools implicitly depends on zip, and accidentally deletes coverage data if zip is absent.

So: I will add zip to Drake dependencies, then ask for updated images, and then come back to polishing these error checks.

Reviewable status: 0 of 1 files reviewed, all discussions resolved

[rpoyner-tri]

Request for new images is here: RobotLocomotion/drake#21865

Reviewable status: 0 of 1 files reviewed, all discussions resolved

[rpoyner-tri]

FWIW, bazelbuild/bazel#23479 is also now fixed and slated for bazel 7.4.0, but we still should have images that contain zip.

Reviewable status: 0 of 1 files reviewed, all discussions resolved

[rpoyner-tri]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @rpoyner-tri)

---

driver/configurations/bazel/step-build.cmake line 59 at r1 (raw file):

else()
  set(BUILD_ARGS
    "${DASHBOARD_BAZEL_STARTUP_OPTIONS} test ${DASHBOARD_BAZEL_BUILD_OPTIONS} ${DASHBOARD_BAZEL_TEST_OPTIONS} //common:drake_assert_test //common:ssize_test")

This change will get reverted before merge. It's just here to make test cycles go faster.

[rpoyner-tri]

Image updates are done, but the Jammy nodes are still running the old one. Will wait until they are updated, and then run an end-to-end test before trying to merge.

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion

[rpoyner-tri]

Things are in good shape now. For examples of success and failure with this changes, see these build logs:

• success: https://drake-jenkins.csail.mit.edu/job/linux-jammy-clang-bazel-experimental-coverage/33/
• failure: https://drake-jenkins.csail.mit.edu/job/linux-jammy-clang-bazel-experimental-coverage/31/

At r2, I've reverted the build target changes that made the above test builds conveniently short.

+@BetsyMcPhail for review.

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @BetsyMcPhail)

[rpoyner-tri]

As long as non-zero exit codes from kcov_tool get noticed, I'm happy. The scope of the changes in r3 probably wants a more cmake-and-cdash-literate reviewer, maybe @mwoehlke-kitware ?

Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @BetsyMcPhail)

[BetsyMcPhail]

Sorry for the confusion, I was experimenting and accidentally pushed to this branch. I was hoping to get a cleaner failure, with a report of the command, push the results to CDash, etc. I do have a WIP (#290) that should be ready soon. Should I push to this branch directly?

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @BetsyMcPhail)

[rpoyner-tri]

Sure, pushing here is fine. Let me know if you need me to help test, or anything.

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @BetsyMcPhail)

[rpoyner-tri]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @BetsyMcPhail and @rpoyner-tri)

a discussion (no related file):
Just adding a blocked discussion so it doesn't get merged before Betsy is satisfied.

---

[BetsyMcPhail]

@rpoyner-tri how do these test results look to you?

Test at r4 (with small change to speed up testing) and breaking change in drake (RobotLocomotion/drake#21915): https://drake-jenkins.csail.mit.edu/view/Coverage/job/linux-jammy-clang-bazel-experimental-coverage/37/

Test at r4 with drake master: https://drake-jenkins.csail.mit.edu/view/Coverage/job/linux-jammy-clang-bazel-experimental-coverage/38/

+@mwoehlke-kitware for CMake code review

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @mwoehlke-kitware)

[rpoyner-tri]

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @BetsyMcPhail and @mwoehlke-kitware)

[rpoyner-tri]

Any updates? I'd like to see this land soon.

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @BetsyMcPhail and @mwoehlke-kitware)

[mwoehlke-kitware]

Nested if/else chains aren't entirely unprecedented. The only obvious alternative would be that, in many other places, rather than nesting, each subsequent step is guarded with if(NOT DASHBOARD_UNSTABLE). However, I'm guessing that in this case we want to submit even if a prior failure occurred, so that technique would not be appropriate?

Reviewed 1 of 2 files at r4, 1 of 1 files at r5.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @rpoyner-tri)

---

driver/configurations/bazel/step-build.cmake line 167 at r5 (raw file):

      endif()

      if(DASHBOARD_SUBMIT)

FYI, this gets duplicated a lot. At some point it might be worth refactoring this into a helper function.

[BetsyMcPhail]

Reviewed 1 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @rpoyner-tri)

---

driver/configurations/bazel/step-build.cmake line 167 at r5 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

FYI, this gets duplicated a lot. At some point it might be worth refactoring this into a helper function.

Not a bad idea, let's leave this for future work so these checks can get in.

[mwoehlke-kitware]

driver/configurations/bazel/step-build.cmake line 167 at r5 (raw file):

Previously, BetsyMcPhail (Betsy McPhail) wrote…

Not a bad idea, let's leave this for future work so these checks can get in.

Right; I didn't intend for this PR to do so. Note "at some point". 🙂