Add checkVector test helper equivalent to checkMatrix, NativePrint for std::unordered_map and std::array

[PDoakORNL]

Expanded Utilities/for_testing.

Proposed changes

See title
Incoming PR's will have tests that use these and provide coverage.

What type(s) of changes does this code introduce?

Delete the items that do not apply

• New feature
• Documentation or build script changes

Does this introduce a breaking change?

• No

What systems has this change been tested on?

Checklist

• Yes This PR is up to date with current the current state of 'develop'
• Yes. Code added or changed in the PR has been clang-formatted
• No. This PR adds tests to cover any new code, or to catch a bug that is being fixed
• Yes/No. Documentation has been added (if appropriate)

[ye-luo]

Can we use empty return error_message for pass instead of this two element struct? Just return the error_string.

[ye-luo]

Did you miss this one?

[PDoakORNL]

I'm not a big fan of check for null as success semantics and this turns out quite readable with the catch macros for checkMatrix. I want this to have the same semantics. See: test_ConstantSPOSet.cpp, test_cuBLAS_LU.cpp, test_spline_applyrotation.cpp, etc.

[ye-luo]

The current CheckXXXResults is not concise.
If results=true, it is unclear what non-empty result_message should be used for or should I care?
If results=false, but results_message is empty. Should I concerned about an empty result_message? Is that a bug?
I don't like undefined behvaviors.
A single string is neat. If empty, pass. If non-empty, error info is given.
Do you have good ways to prevent any UB with CheckXXXResults?

[PDoakORNL]

A single string is ambiguous it because both a message and a success value (if empty). Where's the UB? This isn't kernel code every byte isn't important. This is a test function so the overhead of explicitly returning a structured strongly typed result that is compatible with our testing frameworks' macros seems fine.

[ye-luo]

You didn't answer my question.
If a caller receives results=true but the message string is not empty. What action should be taken for the string or we should enforce empty string in the true case.
If a caller receives results=false but the message string is empty. Is this a bug?
That is what I mean UB and the needed action is unclear to me.
results=true/false, string=empty/non-emtpy. There are 4 combinations but you only intended to use 2 cases. That is why I don't see the benefit of struct over a simple string. If empty, pass. If non-empty, message is an error for users.

[ye-luo]

In addition, do we really need both CheckVectorResult and CheckMatrixResult or we can simply use one CheckResult? Do you have plan to differentiate them?

[PDoakORNL]

It's nicer for me to have matrix indexing available for the error reporting when its a matrix and vector indices when it is not. This also allows each matrix or vector check to count as one "assertion" rather than say like 24000 assertions which ends up making that metric useless for a particular test. It's proved useful to me when using the unit tests to actually develop and debug as opposed to just check for no change ex post factor. It's code to facilitate using the unit test code as a tool for development, its somewhat flexible but still can be left in place and check correctness simply. I'm not sure what the big deal is I provided multiple files that show useful usage of the extant checkMatrix, I just want to facility for large vectorized data, I want to use the more self documenting idiom of not overloading the semantics of a single return value. I already have code I've used this with I guess I can just dumb the testing down.

[ye-luo]

It is a good thing to record all the failed assertion and report once. If you intended to record more details of matrix or vector explicitly instead of expressing via the message, then CheckVectorResult and CheckMatrixResult is necessary. If not, a single CheckResult seems sufficient.

Regarding a single vs multi return value, If you insist on keeping both bool and message. Please use ways to protect the corner cases which cause confusion. For example, use the constructor to error out the undesired corner cases. Or you can do

class checkResult
{
   std::string err_msg;
public:
   isSuccess() const { return err_msg.empty(); }
   const& std::string getErrorMsg() const {
     if(isSuccess()) throw runtime_error("no error message when successful!");
     return err_msg;
   };
}

You can still make the return typed but not exposing the empty/non-empty logic to the caller.

[PDoakORNL]

It's good to get a committed review but this is utility code for testing and I think this is style disagreement. For test code I think easy to understand, read, and adapt is good. I have plenty of code ready to PR that is in application code paths and the effort will be more worthwhile there. I'd like to put more effort in there.

Primarily having the structured return type defined right over the function in the header is simple and I'm not intending to set a consistent return type used by multiple functions. But as you guessed If I do decide I want to add indexes to the return value or other information it is very straight forward to do so and doesn't require touching the other function or existing users of the function. It's coincidental that the return of both of these checks is the same for now.

Our developers that actually write tests have been using checkMatrix without needing my help or anything past the source for a couple of years now. So I think its ok enough, I don't see any misuse or confusion about its usage.

If someone wants to add a message for the success case of either it breaks nothing. Current usage just doesn't do anything with the message when the check passes. Accessing an empty string is also safe, but i can't see why someone would add a new fail with no message but for this code I don't think it needs to be enforced.
So I don't really see an invariant that needs to be defended here.
its seems nice to be able to look at the test and see

return {false|true, "message of some sort"}

You can easily see what each path means.

I don't want to throw any exceptions just provide a boolean check and some information that can be reported or used for development. This is intended for use in a catch2 test section.

There is nothing wrong with your class I'd just rather do it this way.

[PDoakORNL]

test this please