Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make tracing test assert messages more descriptive #93

Merged

Conversation

christophebedard
Copy link
Member

This is in the context of tracing tests failing on ROS 2 CI after parallelized testing was enabled. The goal is to improve the failed assertion messages to better understand how/why the tests failed just by looking at the test logs, which should hopefully help us fix the tests.

To do this, I:

  1. Improved some of the default messages in TraceTestCase's assert*() methods to include more information, like the full event(s) data, the field name(s), and the value(s) (if applicable).
  2. Removed some of the assert messages in the tests to rely on the default ones. Most (if not all) messages didn't contain the actual event data or value, which is really the part that's useful. The messages only explained the context, which we can figure out by looking at the code anyway, so it wasn't really that useful.

I also did some formatting/code style fixes.

@christophebedard christophebedard added the enhancement New feature or request label Mar 5, 2024
@christophebedard christophebedard self-assigned this Mar 5, 2024
Copy link
Contributor

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with green CI

@christophebedard
Copy link
Member Author

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@christophebedard christophebedard merged commit 9a80780 into rolling Mar 5, 2024
9 checks passed
@christophebedard christophebedard deleted the christophebedard/tests-more-helpful-assert-messages branch March 5, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants