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 #692, fix or statements #739

Conversation

zanzaben
Copy link
Contributor

@zanzaben zanzaben commented Jan 5, 2021

Describe the contribution
Fixes #692
fixed the or statement to correctly compare against both options.

Testing performed
Build and run unit test

Expected behavior changes
will correctly cath failures when they happen

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 Pull request is ready for discussion at the Configuration Control Board (CCB) label Jan 5, 2021
@zanzaben zanzaben force-pushed the fix692_network_api_state_or_assertions branch from 0427074 to ff06b57 Compare January 11, 2021 15:08
@skliper
Copy link
Contributor

skliper commented Jan 11, 2021

How about" Fix #692, Resolve typos in unit test assert OR statements"?

@zanzaben zanzaben force-pushed the fix692_network_api_state_or_assertions branch from ff06b57 to 4b65be0 Compare January 11, 2021 18:46
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.

Looking closer I'd recommend something like:

if (actual == OS_ERR_NOT_IMPLEMENTED)
{
    UtPrintf("XXX API not implemented");
}
else
{
... do rest of the test
}

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.

Needs auto-formatting applied, otherwise great improvement.

@skliper
Copy link
Contributor

skliper commented Jan 11, 2021

Side note - not implemented isn't actually considered and "error", test passes either way (and it's OK for OSAL/OS's to not implement all API functionality).

@zanzaben zanzaben force-pushed the fix692_network_api_state_or_assertions branch from ca5ed76 to 33aefa4 Compare January 11, 2021 19:46
@zanzaben
Copy link
Contributor Author

Side note - not implemented isn't actually considered and "error", test passes either way (and it's OK for OSAL/OS's to not implement all API functionality).

Well then it shouldn't be called os_ERR_not_implemented

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.

Good point! In the context of FSW it is an error, and brings up that we should probably be flagging the not implemented cases as UTASSERT_CASETYPE_NA so they get explicitly counted/reported. Lets address that as a separate issue though.

@zanzaben zanzaben changed the title Fix #692 fix or statements Fix #692, fix or statements Jan 12, 2021
@zanzaben zanzaben force-pushed the fix692_network_api_state_or_assertions branch from 33aefa4 to a928734 Compare January 13, 2021 14:52
@astrogeco
Copy link
Contributor

astrogeco commented Jan 13, 2021

CCB 2021-01-13 APPROVED

@skliper opened related issue to make finding "not implemented" items

@astrogeco astrogeco added CCB:2021-01-13 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Jan 13, 2021
@astrogeco astrogeco changed the base branch from main to integration-candidate January 25, 2021 02:35
@astrogeco astrogeco merged commit 4a87c68 into nasa:integration-candidate Jan 25, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Jan 25, 2021
@zanzaben zanzaben deleted the fix692_network_api_state_or_assertions branch February 23, 2021 17:16
@skliper skliper added this to the 6.0.0 milestone Sep 24, 2021
jphickey added a commit to jphickey/osal that referenced this pull request Aug 10, 2022
Add more hooks for additional flexibility when adding
modular code blobs into the build.

Three new directives are added:

MISSION_CORE_MODULES, for modular components which are
direct dependencies of CFE core and/or extend its functionality.

MISSION_GLOBAL_APPLIST, for applications/libraries which
should be built for every target, as if they were listed
in every TGTx_APPLIST setting.

MISSION_GLOBAL_STATIC_APPLIST, same as above but for the
TGTx_STATIC_APPLIST setting.

This also simplifies/reworks the search path to remove
some logic that was never really utilized.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect assertions in network-api-test
3 participants