Fix #97, check only format string in UT event test #99
Conversation
The fully-rendered strings are not entirely consistent, affected by many external factors outside the control of the test cases. There was even a warning in the comment against doing this, but it was included nonetheless as an example of how it can be done. This now keeps the original example but keeps it in the comment only, and converts the actual test to follow the recommended practice of only testing the spec/format string, not the fully-rendered string. This should be much more stable going forward, and should work with both CCSDS v1 and v2 configs.
|
I think we can fast track this. If this was a cFE, PSP, or OSAL ticket I wouldn't but since it's just a coverage test for sample_app I'm fine with not having it go through a super thorough review. Plus I think I kind of understand the intent of the change. |
|
As an "example" I'd actually think it would be nice to show how you can do either via #99... have a string to test the format or a string to test the simulated output and skip if NULL. Or even in the second case use strstr and allow a substring match to pass? Not a big deal though, just examples to help the user create whatever works for them (which I think #99 already meets that intent as-is). |
I was thinking about this, and one way to make it safer might be to to do the We could instead call "snprintf" here using the expected format and args and generate the complete reference string that way. It would accomplish the objective of validating all the arguments, while still adapting to changes from external symbols (i.e. like the specific value of CFE_SB_MSGID_INVALID, etc). |
|
I like that approach. How about if the format was a macro in the flight code (easier to document, less repetition of characters in testing and more resilient since then the format string is always just defined in one location). I'm not a fan of how the format is essentially defined 3 different times already in cFE, two in the events header file: Then finally in the implementation the third: Which illustrates inconsistent documentation since they don't match... |
Describe the contribution
For example "SendEvent" tests in sample_app, do not pass the test string through
vsnprintf()to get a fully rendered output. Instead test only that the format string matches. This is my recommended approach.Note that the "fully-rendered" output is affected my many external variables and is not necessarily going to be consistent. The most recent example is in #87/#94 which is the reason for this change, but it also can be affected by things totally outside of CFE like the user's locale settings in the OS.
Fixes #97
Testing performed
Build and run sample_app unit tests
Expected behavior changes
Fixes current failure in integration candidate 2020-09-15.
System(s) tested on
Ubuntu 20.04
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.