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 #1650, Add File Utility Functional Tests. #1673

Merged
merged 4 commits into from
Jul 27, 2021
Merged

Fix #1650, Add File Utility Functional Tests. #1673

merged 4 commits into from
Jul 27, 2021

Conversation

zanzaben
Copy link
Contributor

Describe the contribution
Fixes #1650
Add Functional tests for File Utility Functional Tests.

Testing performed
Build and run unit test

Expected behavior changes
No impact to behavior

System(s) tested on
Ubuntu 20.04

Contributor Info - All information REQUIRED for consideration of pull request
Alex Campbell GSFC

@zanzaben zanzaben added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Jul 19, 2021
@zanzaben zanzaben force-pushed the fix1650_FS_File_Util_Func_Tests branch from b13bf28 to 63134bc Compare July 19, 2021 15:58
@skliper skliper added this to the 7.0.0 milestone Jul 20, 2021
@zanzaben zanzaben removed the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Jul 21, 2021
zanzaben
zanzaben commented Jul 22, 2021
View reviewed changes
modules/cfe_testcase/src/fs_util_test.c
}

/* FT helper stub compatible with background file write DataGetter */
bool FS_DataGetter(void *Meta, uint32 RecordNum, void **Buffer, size_t *BufSize)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When these two functions exists it causes CFE_FS_InitHeader to stall, and then the test timeout detection kicks in.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a bug or an issue w/ the test?

@jphickey
Copy link
Contributor

The issue here is that we need to make sure we adhere to the API spec of the function, specifically this one:

cFE/modules/core_api/fsw/inc/cfe_fs.h

Lines 293 to 296 in 33a4f19

** \par Assumptions, External Events, and Notes:
** Metadata structure should be stored in a static memory area (not on heap) as it
** must persist and be accessible by the file writer task throughout the asynchronous
** job operation.

Actually I should probably correct that to say "not on stack" rather than "not on heap". I just added a delay loop to make it pass, but really should make it static or move the object to the global data space.

@jphickey jphickey mentioned this pull request Jul 23, 2021
Closed
@zanzaben zanzaben requested a review from jphickey July 23, 2021 15:09
@zanzaben zanzaben added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Jul 23, 2021
@astrogeco
Copy link
Contributor

@zanzaben can you add the issue number to @jphickey's commit message? Otherwise, is this ready to merge?

@zanzaben zanzaben force-pushed the fix1650_FS_File_Util_Func_Tests branch from d045f39 to 81c598a Compare July 26, 2021 16:29
@astrogeco astrogeco requested a review from skliper July 26, 2021 19:59
@astrogeco
Copy link
Contributor

@skliper and @jphickey is this good to go?

skliper
skliper approved these changes Jul 27, 2021
View reviewed changes
Copy link
Contributor

@skliper skliper left a comment

Choose a reason for hiding this comment

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

Looks fine by me as long as it's working consistently as expected.

jphickey
jphickey approved these changes Jul 27, 2021
View reviewed changes
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

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

Minor suggestions.... Its OK as-is I suppose, but there are few nitpicks I'd recommend improving.

modules/cfe_testcase/src/fs_util_test.c Show resolved Hide resolved
modules/cfe_testcase/src/fs_util_test.c Outdated Show resolved Hide resolved
jphickey
jphickey requested changes Jul 27, 2021
View reviewed changes
modules/cfe_testcase/src/fs_util_test.c Outdated Show resolved Hide resolved
@astrogeco astrogeco changed the base branch from main to integration-candidate July 27, 2021 19:03
@astrogeco astrogeco merged commit ea754b6 into nasa:integration-candidate Jul 27, 2021
@astrogeco astrogeco mentioned this pull request Jul 27, 2021
Merged
astrogeco added a commit to nasa/cFS that referenced this pull request Jul 27, 2021
@astrogeco
IC:2021-07-27, Add cfe testing PRs
34dbc0e 
nasa/cFE#1673, Add File Utility Functional Tests.

nasa/cFE#1711, Add custom epoch support to TIME UT
@astrogeco astrogeco mentioned this pull request Jul 29, 2021
Merged
astrogeco added a commit to nasa/cFS that referenced this pull request Jul 29, 2021
@astrogeco @skliper
@jphickey @zanzaben @pepepr08 @nmullane @PavLL
IC:2021-07-27, cFE, osal and GroundSystem
1a59db7 
Combines:

nasa/cFE#1701, v6.8.0-rc1+dev789
nasa/osal#1116, v5.1.0-rc1+dev578
nasa/cFS-GroundSystem#192, v2.2.0-rc1+dev58

Includes:

**cFE**

nasa/cFE#1699, correct return code check
nasa/cFE#1700, documentation for FS APIs that return OSAL codes
nasa/cFE#1695, Adding coverage tests for cfe_es_apps.c
nasa/cFE#1673, Add File Utility Functional Tests.
nasa/cFE#1711, Add custom epoch support to TIME UT
nasa/cFE#1720, Requirements update for Caelum
nasa/cFE#1721, Add null check to CFE_ES_TaskID_ToIndex.
nasa/cFE#1719, scrub command documentation
nasa/cFE#1715, Add time arithmetic functional tests
nasa/cFE#1704, update docs for CFE_FS_BackgroundFileDumpRequest
nasa/cFE#1706, correct return code mismatches

**osal**

nasa/osal#1114, Add unit test branch coverage

**cFS-GroundSystem**

nasa/cFS-GroundSystem#185, Update tlm for ES Blockstats/memstats and TBL HK
nasa/cFS-GroundSystem#188, * might be referenced before assignment

Co-authored-by: Jacob Hageman <[email protected]>
Co-authored-by: Joseph Hickey <[email protected]>
Co-authored-by: Alex Campbell <[email protected]>
Co-authored-by: Jose F Martinez Pedraza <[email protected]>
Co-authored-by: Niall Mullane <[email protected]>
Co-authored-by: Paul <[email protected]>
@astrogeco astrogeco added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Aug 4, 2021
@astrogeco
Copy link
Contributor

CCB:2021-08-04 APPROVED

@zanzaben zanzaben deleted the fix1650_FS_File_Util_Func_Tests branch August 13, 2021 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skliper skliper skliper approved these changes

@jphickey jphickey jphickey approved these changes

Assignees
No one assigned
Labels
CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Milestone
Caelum
Development

Successfully merging this pull request may close these issues.

Add functional tests for cFE FS File Utility APIs
4 participants
@zanzaben @jphickey @astrogeco @skliper