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

UtAssert "N/A" test case type is slightly overloaded #1130

Closed
jphickey opened this issue Aug 16, 2021 · 0 comments · Fixed by #1131 or #1132
Closed

UtAssert "N/A" test case type is slightly overloaded #1130

jphickey opened this issue Aug 16, 2021 · 0 comments · Fixed by #1131 or #1132
Milestone

Comments

@jphickey
Copy link
Contributor

Is your feature request related to a problem? Please describe.
There are a number of similar situations for which the "N/A" (aka UTASSERT_CASETYPE_NA) log type is used, but have different nuances:

  1. When a certain functionality is not implemented at all (e.g. network tests on a system without network stack, the API returns OS_ERR_NOT_IMPLEMENTED). In this case a single "N/A" entry is used and the whole group of tests is skipped.
  2. When a certain functionality is implemented, but cannot be tested due to external factors, such as the state of the system, how it was booted, etc. (e.g. in the "CDS" test in ES, when calling CFE_ES_RegisterCDS(), if the CDS memory was not cleared before the test, the result may be "CFE_ES_CDS_ALREADY_EXISTS"). In this case the "N/A" entry is used to advise the user that the test is incomplete due these external factors, and that the user should take action to correct this and get a complete test result.
  3. When there is more than one possible acceptable result from a specific function call. In this case, the test code checks for each acceptable result, but are checked with UTASSERT_CASETYPE_NA to indicate that although this value was checked and does not match, it does not constitute a failure of the test (the test can go on to check for the other possible acceptable results).

Describe the solution you'd like
These three different "N/A" nuances should ideally have a different case type. (specific words/abbreviations can be discussed). As the traditional meaning of "N/A" is "Not applicable", it likely line up best with the use case (1) above, where a test is truly not applicable on that platform/setup.

For the other two, a separate casetype should be added to better convey the intent:

For item (2) above is to warn the user that the results are incomplete due to external factors, and they need to take action to correct those external factors to get a complete result. This shouldn't necessarily be a failure; the test can still succeed in reduced form, but the report should be very clear that tests were skipped or not complete, and the user needs to take manual action to correct it.

For item (3) above, the intent is allow a "soft" test - where multiple values are acceptable, it is necessary to "PASS" (and log it) if a value does match the acceptable value, but not fail if the value does not match - because there are other acceptable values. Typically these checks wouldn't need to go in the log at all; they may only be of interest to developers implementing the test, it does not provide much value in a final report log to see values that were checked for but didn't match. (In that sense, visibility of these tests should be similar to DEBUG, but we should not overload DEBUG either).

Describe alternatives you've considered
Leave all these case types as N/A

Additional context
Existing overload of N/A is not horribly broken/wrong, but it makes the logs a little harder to process, and not as clear as to what action (if any) the user needs to take for the N/A reports.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

jphickey added a commit to jphickey/osal that referenced this issue Aug 17, 2021
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).
jphickey added a commit to jphickey/osal that referenced this issue Aug 17, 2021
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).
jphickey added a commit to jphickey/osal that referenced this issue Aug 17, 2021
For truly N/A tests, the count does not matter much, since the
test is not applicable anyway.  However a skipped test should
be more concerning to a user, as it means the test log is not
complete.

In that sense, it is more important to report skips than N/A, so
the user can confirm that the count was 0.
jphickey added a commit to jphickey/osal that referenced this issue Aug 18, 2021
jphickey added a commit to jphickey/osal that referenced this issue Aug 18, 2021
jphickey added a commit to jphickey/osal that referenced this issue Aug 18, 2021
Add two more test case variants, similar to NA, where failure
of the test case does not translate to failure of the overall
test:

 - UTASSERT_CASETYPE_WARN
 - UTASSERT_CASETYPE_FLOW

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).  For the
latter case, the WARN type can be used, to more clearly
indicate it is an action item for the user.

Lastly the "FLOW" message is intended to indicate internal decisions
in the test case implementation (not actual test cases).
astrogeco added a commit that referenced this issue Aug 19, 2021
@skliper skliper added this to the 6.0.0 milestone Sep 24, 2021
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Applies standard format using the .clang-format configuration from the cFS bundle
Hand edits some files to account for comment splits
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Removes trailing whitespace in:

- CMakeLists.txt
- *.cmake
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Removes trailing whitespace in:
- *.dox
- *doxyfile.in
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Excludes .md, .pdf, and .doc
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants