search

RobotLocomotion / drake-external-examples

Examples of how to use Drake in your own project.
https://drake.mit.edu/
MIT No Attribution
107 stars 50 forks source link

Add GHA CI to subdirs and sync with main CI #333

Open nicolecheetham opened 1 month ago

nicolecheetham commented 1 month ago

Creates copies of the reusable workflows for each respective GHA subdir and adds a CI sync test to ensure the subdir CIs exactly match the two places it is copied from.

Issue: https://github.com/RobotLocomotion/drake-external-examples/issues/309

This change is Reviewable

nicolecheetham commented 3 weeks ago

@jwnimmer-tri For the GHA ci file sync test, is it preferrable to keep the the files sorted or have easily understood test logic?

The specific example ci file stays in the same position (3rd) however the other two types of files (the main ci.yml and specific reusable workflow) change places depending on the name of the drake example. If sorted, I cannot assume the position of these two positions. Therefore, I would check if both top and bottom part of the specific ci file would be in either of the two files instead of checking if one subset is in the relevant original file.

jwnimmer-tri commented 3 weeks ago

Big picture, for new cross-checking logic in this PR, it could live inside file_sync_test.py or it could be a new test program, either way is fine. However, it should NOT re-use the COPIES array at all. We want totally new/novel logic.

... if one subset is in the relevant original file.

The test criterion we want is not "subset". We must programmatically compute the exactly right answer for what flavor_subdir/.github/ci.yml should look like, and then fail if it differs even by a single line or byte. Just checking a rough "subset" alone isn't useful, because there exist subsets which would pass the test but be invalid GHA instructions.

jwnimmer-tri commented 1 day ago

I carved #342 out of this pull request so that it can land sooner than later. After #342 merges, you'll need to merge up this PR to the latest main.

Note that in #342 I removed the superfluous ./ from all of the ./subdir/command lines. When providing a relative subdirectory path, is not conventional to start with a no-op ./ in front of the path. Just say subdir/command directly.