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.

Sign up for GitHub

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

Jump to bottom

Fix #435, add coverage tests for PSP modules #436

Merged
merged 2 commits into from
Sep 19, 2024
Merged

Fix #435, add coverage tests for PSP modules #436

merged 2 commits into from
Sep 19, 2024

Conversation

jphickey
Copy link
Contributor

Checklist (Please check before submitting)

Describe the contribution
Adds a coverage test implementation for all existing PSP modules. This should get 100% coverage on all lines and branches.

Fixes #435

Testing performed
Build and run all tests

Expected behavior changes
Complete coverage tests for PSP modules

System(s) tested on
Debian

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Aug 12, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 12, 2024
View reviewed changes
fsw/shared/src/cfe_psp_module.c

static uint32 CFE_PSP_ConfigPspModuleListLength = 0;
static uint32 CFE_PSP_StandardPspModuleListLength = 0;
CFE_PSP_ModuleListGlobal_t CFE_PSP_MODULE_LIST_GLOBAL;

Check notice

Code scanning / CodeQL

Global could be static Note

The global variable CFE_PSP_MODULE_LIST_GLOBAL is not accessed outside of cfe_psp_module.c and could be made static.
fsw/shared/src/cfe_psp_module.c

/***************************************************
*
* Helper function to initialize a list of modules (not externally called)
* Returns the number of modules initialized
*/
uint32_t CFE_PSP_ModuleInitList(uint32 BaseId, CFE_StaticModuleLoadEntry_t *ListPtr)
void CFE_PSP_ModuleInitList(CFE_PSP_ModuleListWrapper_t *WrapPtr, uint32 BaseId, CFE_StaticModuleLoadEntry_t *ListPtr)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
fsw/shared/src/cfe_psp_module.c
* See prototype for full description
*/
int32 CFE_PSP_Module_FindByName(const char *ModuleName, uint32 *PspModuleId)
uint32 CFE_PSP_Module_SearchNameInList(const CFE_PSP_ModuleListWrapper_t *WrapPtr, const char *ModuleName)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
fsw/shared/src/cfe_psp_module.c
*
* See prototype for full description
*/
int32 CFE_PSP_Module_FindByName(const char *ModuleName, uint32 *PspModuleId)

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
@jphickey jphickey force-pushed the fix-435-pspmod-covtests branch 2 times, most recently from fc41d65 to 43df4a1 Compare August 12, 2024 14:16
@jphickey jphickey requested a review from chillfig August 12, 2024 14:21
@skliper
Copy link
Contributor

skliper commented Aug 12, 2024

@jphickey Does this also resolve or at least partially address #284 and #274? Recommend refreshing/updating those issues to detail any remaining work (if there is any).

@jphickey
Copy link
Contributor Author

@jphickey Does this also resolve or at least partially address #284 and #274? Recommend refreshing/updating those issues to detail any remaining work (if there is any).

Yes, it does! Thanks for keeping an eye on the old issues. There are some doc fixes I need to merge in but after that we should be able to close both of those old issues as well. I will link it.

jphickey added a commit to jphickey/MD that referenced this pull request Aug 15, 2024
@jphickey
Fix nasa#77, deprecate PSP stubs from MD coverage test
f50c45f 
The PSP now provides the complete set of stubs.

IMPORTANT: This needs to be used in conjunction with nasa/PSP#436 to
get the complete set of stubs, otherwise linking will fail.
jphickey added a commit to jphickey/MM that referenced this pull request Aug 15, 2024
@jphickey
Fix nasa#94, deprecate PSP stubs in MM coverage tets
2d6482a 
The PSP now provides the full set of stubs.

IMPORTANT: This needs to be merged in conjunction with nasa/PSP#436 to
obtain the complete set of stubs.  Otherwise the link will fail.
This was referenced Aug 15, 2024
Merged
Merged
@dzbaker dzbaker added CCB: Provisionally-Approved and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Aug 15, 2024
@dzbaker
Copy link
Contributor

dzbaker commented Aug 15, 2024

CCB 15 August 2024: Approved pending removal of commented code.

dzbaker added a commit that referenced this pull request Aug 16, 2024
@dzbaker
Merge pull request #436 from jphickey:fix-435-pspmod-covtests
bf95d9b 
Fix #435, add coverage tests for PSP modules
dzbaker added a commit to nasa/cFS that referenced this pull request Aug 16, 2024
@dzbaker
IC: equuleus-rc1+dev16
ac78cec 
*Combines:*

cFE equuleus-rc1+dev187
osal equuleus-rc1+dev85
PSP equuleus-rc1+dev48

**Includes:**

*cFE*
- nasa/cFE#2591

*osal*
- nasa/osal#1419

*PSP*
- nasa/PSP#436

Co-authored by: Avi Weiss <[email protected]>
Co-authored by: Jose Martinez Pedraza <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>
This was referenced Aug 16, 2024
Merged
Merged
@jphickey
Fix nasa#435, add coverage tests for PSP modules
fb6c855 
Adds a coverage test implementation for all existing PSP modules.  This
should get 100% coverage on all lines and branches.
@jphickey
Fix nasa#435, correct init in PCS_malloc call
2336dde 
The handler executes after the stub count increments, thus this
needs to check for 1 to indicate the first call, not 0.
@jphickey
Copy link
Contributor Author

@dzbaker -- The workflow issue is fixed by the commit I just added. Due to the difference in where this code now executes in the overall flow, the counter is now 1 during the first call, not 0 (handlers execute post-increment).

@dzbaker dzbaker added CCB:Approved Indicates Approval by CCB and removed CCB: Provisionally-Approved labels Sep 16, 2024
dzbaker added a commit to nasa/cFS that referenced this pull request Sep 16, 2024
@dzbaker
IC: equuleus-rc1+dev18
ed5afce 
*Combines:*

cFE equuleus-rc1+dev199
PSP equuleus-rc1+dev49

**Includes:**

*cFS*
- #791
- #796

*cFE*
- nasa/cFE#2599

*PSP*
- nasa/PSP#439
- nasa/PSP#436

Co-authored by: Andrew Liounis <[email protected]>
Co-authored by: Rich Landau <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>
@dzbaker dzbaker mentioned this pull request Sep 16, 2024
Merged
2 tasks
@dzbaker dzbaker merged commit 61b1230 into nasa:main Sep 19, 2024
10 checks passed
dzbaker added a commit to nasa/cFS that referenced this pull request Sep 19, 2024
@dzbaker
IC: equuleus-rc1+dev18
53faec5 
*Combines:*

cFE equuleus-rc1+dev199
PSP equuleus-rc1+dev49

**Includes:**

*cFS*
- #791
- #796

*cFE*
- nasa/cFE#2599

*PSP*
- nasa/PSP#439
- nasa/PSP#436

Co-authored by: Andrew Liounis <[email protected]>
Co-authored by: Rich Landau <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>
jphickey added a commit to nasa/MD that referenced this pull request Sep 23, 2024
@jphickey
Fix #77, deprecate PSP stubs from MD coverage test
b7ebab5 
The PSP now provides the complete set of stubs.

IMPORTANT: This needs to be used in conjunction with nasa/PSP#436 to
get the complete set of stubs, otherwise linking will fail.
jphickey added a commit to jphickey/MM that referenced this pull request Sep 23, 2024
@jphickey
Fix nasa#94, deprecate PSP stubs in MM coverage tets
194b0f9 
The PSP now provides the full set of stubs.

IMPORTANT: This needs to be merged in conjunction with nasa/PSP#436 to
obtain the complete set of stubs.  Otherwise the link will fail.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chillfig chillfig Awaiting requested review from chillfig

Assignees
No one assigned
Labels
CCB:Approved Indicates Approval by CCB coverage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add coverage tests for PSP modules
4 participants
@jphickey @skliper @dzbaker @chillfig