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

String checks in UT should check only the spec/format string, not the fully rendered text. #97

Closed
jphickey opened this issue Sep 17, 2020 · 1 comment · Fixed by #99 or #98
Closed
Labels
bug Something isn't working
Milestone

Comments

@jphickey
Copy link
Contributor

Describe the bug
As the comment in the code suggests, the "fully-rendered" text is really a derived output that depends on a lot of external factors. It isn't a direct output of the unit under test.

As an example, the unit test is currently getting thrashed around due to a message ID check due to the fact that there is no specific text that is always "correct" here.

It broke when updated to 32 bit (#87), attempted to fix in #94, which only broke the original case (v1 header).

To Reproduce
Run unit test using v1 (16-bit) message IDs.
Currently failing in the integration-candidate branch

Expected behavior
Tests should pass.

System observed on:
Ubuntu 20.04 and Travis-CI (current integration candidate).

Additional context
Recommendation is to only check the SPEC STRING (0x%x) which will be the same regardless of what the "invalid msgid" value actually is. That's all that really matters.

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey added the bug Something isn't working label Sep 17, 2020
jphickey added a commit that referenced this issue Sep 17, 2020
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.
@jphickey jphickey linked a pull request Sep 17, 2020 that will close this issue
@skliper
Copy link
Contributor

skliper commented Sep 21, 2020

Note #94 does not break v1 when merged with nasa/cFE#880.

yammajamma added a commit that referenced this issue Sep 21, 2020
Fix #97, check only format string in UT event test
@astrogeco astrogeco added this to the 1.3.0 milestone Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants