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

Fix #2323, Remove unnecessary asserts in TIME invalid command length UT #2324

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thnkslprpt
Copy link
Contributor

Checklist

Describe the contribution

Testing performed
GitHub CI actions all passing successfully (incl. Build + Run, Unit/Functional Tests etc.).
Local testing with cFS suite confirms no change to coverage.

Expected behavior changes
This simplifies the coverage tests and makes them easier to maintain. Also, TIME is now consistent with the other modules.

System(s) tested on
Debian GNU/Linux 11 (bullseye)
Current main branch of cFS bundle.

Contributor Info
Avi Weiss @thnkslprpt

@thnkslprpt
Copy link
Contributor Author

Side note: TBL actually only tests for an invalid command length once:

/* Test command pipe messages handler response to an invalid
* message length
*/
UT_InitData();
UT_CallTaskPipe(CFE_TBL_TaskPipe, &CmdBuf.Msg, sizeof(CmdBuf.NoopCmd) - 1, UT_TPID_CFE_TBL_CMD_NOOP_CC);
CFE_UtAssert_EVENTSENT(CFE_TBL_LEN_ERR_EID);
/* Test command pipe messages handler response to an invalid

TBL does not test an invalid command length for every single command like the other modules.

I wonder if a single test is considered sufficient, and if so, can the other modules also be reduced to a single test of invalid command length.

@thnkslprpt thnkslprpt force-pushed the fix-2323-remove-unnecessary-asserts-in-TIME-invalid-length-UT branch from ed59875 to 79e2ce7 Compare December 14, 2023 04:07
@thnkslprpt thnkslprpt force-pushed the fix-2323-remove-unnecessary-asserts-in-TIME-invalid-length-UT branch from 79e2ce7 to d24de7b Compare December 14, 2023 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TIME includes unnecessary asserts in invalid command length tests
2 participants