Fix #1130, add test case types similar to NA

[jphickey]

Describe the contribution
Add two more test case variants, similar to NA, where failure of the test case does not translate to failure of the overall
test.

The additional cases have different levels of default visibility, and may be handled differently by the end user. That is, some
tests may be skipped because they are truly NA (and nothing for the user to do to change that) and some tests may be skipped
because the system was not set up in a way that allowed them to be run (and the user must fix that and re-run).

Fixes #1130

Testing performed
Build and run all tests. Also modify CFE test to use new casetype, confirm working as expected.

Expected behavior changes
None

System(s) tested on
Ubuntu

Additional context
Intended to differentiate between the nuances of all tests that are currently marked "N/A". For some cases, its purely informational because the check does not apply, and nothing more needs to be done. For other cases, a test might be skipped because the user did not start the system in a way where the tests could be run, and the user should correct that. Lastly, for some cases, the condition was NA simply because there was more than one acceptable result for that test case.

For the first two, they should remain visible in the log by default, because they add some value to indicate what was (and was not) tested. However the first requires no action of the user, and the second does require the user to correct something, so they should be marked differently. The third is only relevant to software developers, it doesn't add much value to the log, so it can be hidden by default, but can still be seen by increasing the test verbosity to the debug level.

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

[jphickey]

As an initial proposal, suggesting the words "SKIP" and "SOFT" for the new test cases. Both are functionally identical to the existing NA, except:

• SKIP will be highlighted similar to MIR because it requires an action from the user, whereas regular NA does not.
• SOFT has a visibility level similar to DEBUG, such that it won't be shown in the log when using default visibility levels (can still run in verbose mode to see it though).

[zanzaben]

Can this also update the OSAL Test that use UtAssert_NA and should be UtAssert_SKIP now.

[jphickey]

Can this also update the OSAL Test that use UtAssert_NA and should be UtAssert_SKIP now.

I reviewed all the uses of UtAssert_NA within OSAL tests and they all looked correct to me. That is, they should probably not be changed to SKIP - because SKIP means the user is supposed to take some action (it should run, but it was not run).

[astrogeco]

Jumping into the naming world, what about replacing SOFT with WARN?

[jphickey]

Jumping into the naming world, what about replacing SOFT with WARN?

Nope -- not a warning at all! It is really a "don't care"

[jphickey]

Further thought - if anything the "SKIP" should maybe be "WARN" - because a skipped test, in this context, is something the user might be concerned about. For practicality/flexibility reasons we don't consider this a hard failure in the test - as a developer you want to be able to run this without getting nuisance failures - but in the context of a "run for the record" you would definitely want to see the SKIP (or, possibly WARN) count be 0.

[astrogeco]

What are some use cases for SOFT? That'll help me think about it.

[jphickey]

What are some use cases for SOFT? That'll help me think about it.

In short, this type of log entry can pass a test case, but never fail a test case.

The impetus/motivation for this is the CFE cases where there are more than one possible "acceptable" value. The test code checks one at a time via the CFE_Assert_STATUS_MAY_BE macro (also new) which uses the SOFT casetype - at least in the proposal.

In this context, if it does match, then great - its a PASS. But if it does not match - that's a don't care, because there are other values it could be that might be acceptable. In fact, we don't even need to see it in the test log by default (it's just clutter if you include it).

[skliper]

I really like WARN in place of SKIP... and what if SOFT was just FALSE? It's not a FAIL, but the statement also isn't true...

[zanzaben]

I really like WARN in place of SKIP... and what if SOFT was just FALSE? It's not a FAIL, but the statement also isn't true...

I am fine with WARN instead of SKIP. I really don't like FALSE instead of SOFT, I would say changing that to SKIP would make more sense since we are in essence skipping over it if it doesn't give us what we want. HIDE could also work, since we are hiding the fails we don't care about.

[jphickey]

I'd be good with renaming as follows:

1. SKIP in current proposal -> WARN
2. SOFT in current proposal -> SKIP

I think this conveys the right information to the user (noting that they won't even see item 2 by default, it's hidden, so unlikely to cause any confusion there).

[skliper]

I'd be good with renaming as follows:

1. SKIP in current proposal -> WARN
2. SOFT in current proposal -> SKIP

I think this conveys the right information to the user (noting that they won't even see item 2 by default, it's hidden, so unlikely to cause any confusion there).

I vote for the 2nd case to return FALSE since it was actually evaluated and that was the result which seems like useful information if someone actually does look at that (it wasn't skipped...), it just that it's not a failure. I'd interpret SKIP as having not been evaluated.

[jphickey]

I vote for the 2nd case to return FALSE since it was actually evaluated and that was the result which seems like useful information if someone actually does look at that (it wasn't skipped...), it just that it's not a failure. I'd interpret SKIP as having not been evaluated.

Yes, I was starting to update for this name change and came to the same conclusion. SKIP maybe isn't the best name, the test was not actually skipped, its just that we don't care about the fact that it evaluated false.

Ideally, I'd prefer to have named it UTASSERT_CASETYPE_DONTCARE (borrowing the term from EE when logic signal inputs are ignored). I think this really communicates the idea best, the problem its its 8 characters and the format only allows for 5. So we could just put "DC" in the far-left field.

[jphickey]

The only thing I don't like about FALSE is that it doesn't seem to convey that its OK that it's false. I would read that in the log similar to "FAIL". That is, if you see things evaluating false, you might think there is something wrong. I would prefer a term that emphasized that its OK to be false here.

[skliper]

The part I don't like about SKIP, SOFT, DC or any other status related to the action taken is you loose the fact that it was actually evaluated and the result was FALSE. The fact is you do care about the result (just not in terms of PASS/FAIL).. and it wasn't actually skipped, it's just that a FALSE result isn't a FAIL.

[skliper]

In other words, the fact the result was FALSE did impact the flow of the test, and seems useful to be very clear about that evaluation in terms of the flow and the way the log reads... XYZ was false, so these asserts were then performed.

[jphickey]

In other words, the fact the result was FALSE did impact the flow of the test, and seems useful to be very clear about that evaluation in terms of the flow and the way the log reads... XYZ was false, so these asserts were then performed.

Fair point. I just pushed an update with "DONTCARE" as food for thought, but I'm warming up to the "FALSE" term.

[skliper]

I also like that it is the actual return of the function call also, so not a "new" term.

[jphickey]

One more thought - this "casetype" is really just documenting the test flow at this point. That is, it is saying that the test checked for this condition, which in turn guides where it goes next.

So what about something like "FLOW" or "TRACE", documented as:

UTASSERT_CASETYPE_FLOW, /**< Other condition checks/messages that record test flow do not constitute test case failures */

[jphickey]

I also like that it is the actual return of the function call also, so not a "new" term.

That's actually part of what I don't like, because it does not necessarily return false. In the case of the MAY_BE macro, if it does match, then it returns true ...

[skliper]

I was just making a suggestion about what gets logged in the brackets. I prefer a CASETYPE_SOFT which either results in PASS or FALSE.

[skliper]

I'm not making any sense anymore (I don't think)... with the MAY_BE test for the FALSE result I do think CASETYPE_FLOW is fine as the level of reporting. Sorry, getting mixed up as to where and how these are used. I do still like FALSE in the log and I think it's helpful to categorize it as 'FLOW' type info. Does that make more sense?

[jphickey]

For the most part, I agree, except when it comes down to putting [FALSE] tag on the flow messages. Note that the underlying API used to log everything is UtAssert_Message, and this is a public API in utassert.h that any test program can call at will. It has the protoype here:

osal/ut_assert/inc/utassert.h

Lines 590 to 606 in ce11e7a

	/**
	* \brief Output an informational message to the console/log file
	*
	* Just like the standard printf except it will output to the given status channel (see utassert.h)
	*
	* This calls into the UT BSP function to actually write the message
	* to the current output device. This may be the console or a log file
	* or something else depending on what BSP is in use.
	*
	* \param MessageType Message case type
	* \param File File name containing the message
	* \param Line Line number containing the message
	* \param Spec printf style format followed by args of message
	*
	* \sa Helper macros: UtPrintf(), UtDebug()
	*/
	void UtAssert_Message(uint8 MessageType, const char *File, uint32 Line, const char *Spec, ...) OS_PRINTF(4, 5);

The intent is to provide just a general printf-like utility to log any message. For example, UtPrintf is just a wrapper around this that uses UTASSERT_CASETYPE_INFO.

The part that I struggle with is that it would be perfectly valid for a test implementer to document their own internal decision tree via something like:

UtAssert_Message(UTASSERT_CASETYPE_FLOW, __FILE__, __LINE__, "Checked condition X, result was Y", x, y);

But you'll see in the log (if you run with verbose, anyway):

[FALSE] 48.006 test.c: - Checked condition X, result was Y

The key is, there's nothing "FALSE" about that statement - its a flow/trace message. The only reason "FALSE" tag makes sense is specifically for the CFE_Assert_STATUS_MAY_BE use case, it doesn't make sense in the broader context of just a generic log entry to tag every flow message with "FALSE".

[astrogeco]

CCB:2021-08-18 APPROVED

• have splinter to agree on nomenclature

[jphickey]

Latest update just changes the tag/casetype to "FLOW" - again this is the tag assigned to a general-purpose informational line in the file, it is not strictly limited to boolean conditions. The information can be anything related to the control flow of the test.

Will update nasa/cFE#1816 with actual examples (see there)

[jphickey]

Updated to reflect latest discussions, and also rebased to latest "main" branch. Hopefully this one is a keeper.

NOTE: as part of this exercise I noted that we duplicated the "switch" statement which gets the abbreviated casetype name in CFE. This update also moves this switch statement into a separate function that CFE can call (so it does not need to be repeated there).

[skliper]

Do we want to make WARN also fail CI? Like TSF/TTF/FAIL?

[zanzaben]

Do we want to make WARN also fail CI? Like TSF/TTF/FAIL?

Hard NO. WARN is there for when you are doing a final certification check. CI is more to make sure it's not broken.

[skliper]

Do we want to make WARN also fail CI? Like TSF/TTF/FAIL?

Hard NO. WARN is there for when you are doing a final certification check. CI is more to make sure it's not broken.

I disagree there. CI should put you in the correct configuration for the test, so you shouldn't ever see a WARN. For open source, CI is the cert (or at least that's the point, continuous verification of meeting all the requirements, etc). In other words, if you see a WARN during CI your CI is misconfigured.