rosbag2_transport: parametrize test_rewrite

[james-rms]

runs test_rewrite using all tested storage IDs, for better test coverage.

Signed-off-by: James Smith james@foxglove.dev

[james-rms]

Gist: https://gist.githubusercontent.com/james-rms/782d820aa5fac60a9c4b27124b678895/raw/53f6d77935969c69bced3123482fb66971c53af3/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_transport
TEST args: --packages-above rosbag2_transport
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11257

• Linux
• Linux-aarch64
• Windows

[emersonknapp]

One non-blocking thought - depending how you respond maybe it needs a TODO at least.

Also, I have been thinking that maybe all of these test bags should be generated in code, rather than checked in to the repo. That way it would be easier to just iterate the storage plugins (or whatever other criteria) and test everything, without manual asset creation. The one place where I see actual checked-in file artifacts being useful is for old format backwards compatibility. But, that could be a really explicit list that has some standard battery run on them.

[emersonknapp]

Still feeling a bit awkward about this, is there a very strong reason we shouldn't add mcap to the exec_depend of default plugins? We are planning to ship it, so it doesn't seem to me that there's any harm in doing it.

Or, if we are trying to be very explicit about what we're testing, then maybe rosbag2_storage_sqlite3 is a test_depend here, and default_plugins is not.

[james-rms]

I'm trying to merge these test coverage PRs with the least externally visible disruption possible, which means not adding rosbag2_storage_mcap as a default plugin until it actually is one. That might mean a couple of extra lines like this one in some package.xml files which will be reverted later, but I don't think that's too bad.