Support multiple test suites and add helpful verifiers

[smurthys]

This PR adds support for multiple test suites (goal of M4) and also adds helpful verifiers. I welcome your review. Here is a summary of changes:

Support for multiple test suites:

• Introduce notion of "suite runner" which is any function with the declaration matching void f(), where f would be the name of a suite runner. array_test in array-test.cpp is an actual suite runner.
• Function run_suites in driver.cpp sets up the tester for each suite and invokes test suites. See the comment associated with the std::map variable suites for a brief on how to specify the available "suite runners". Also see the comment just preceding the function about declaring suite runners. The specification of suite runners could possibly be better, but I feel the current state suffices for this PR.
• Add -run cmd-line option to specify the subset of test suites to run. The value of this option is a semi-colon separated list of suite names to run. This option will be quite helpful during the development of a test suite.
• Introduce the macro $suite to stand for suite name. At present the macro expansion is done only in header text. The next round of changes might support expanding this macro also in the file
• Change default header text to Running $suite from Running $cmd
• Add new files utils.h and utils.cpp to hold truly general-purpose functions such as replace_all and the newly added function template split. The function template and its unit tests are at https://godbolt.org/z/LfPFVH. (EDIT: updated link with additional test cases)
• Documentation changes are pending. I will quite likely address them independent of this PR due to time shortage (and at least for now that change is in a separate repo anyway).

Changes to tester:

• Track suite-level tests-done and tests-failed
• Fail threshold is applied to total failed tests across all suites.
• Print suite-level summary
• Snake_case identifiers
• We need to discuss what if any cmd-line options need to be added

New helpful verifiers:

• Added: is_true, is_false, is_zero, is_nonzero, is_negative, is_nonnegative, is_positive,
• Additional helpers are possible and we can add them as we recognize the need.
• Replaced calls to verify in array_test with is_true only for illustration. Using the helpers in that test suite is not part of this PR.
• Function verify should not be called directly, but that call is not prevented: nothing really wrong in calling it, but using the helpers makes tests simpler and somewhat self-documenting.

[smurthys]

My apologies for the delay in creating this PR. I was first delayed by a meeting that went long. I was then sucked in by an "unhandled exception" and an issue with the new function template split. The fixes turned out to be fairly simple in both cases, but I did lose quite a bit of time locating the cause. I also spent a few minutes consoling myself that it was OK to not be able to see the obvious problem that cost me precious time. (What else could I do at this point?)

[cmcmone]

Everything looks great! here is my question:

1. line 89 of driver.cpp, is it better to use std::unordered_map? Because the order of the keys does not seem to matter.
2. line 97 of driver.cpp, do we need to determine whether options.suites_to_run is empty before calling the split function? if it's empty, we don't need call split function.
3. Why the'split' function is implemented in the header file and not in the '.cpp' file?
4. Some places did not use UIS. (Do we need to use UIS all the time?).
5. test.cpp line 85, do we need to throw an exception if the name is empty.

I will continue to review this PR tomorrow.

[smurthys]

Thanks @cmcmone for the review:

1. line 89 of driver.cpp, is it better to use std::unordered_map? Because the order of the keys does not seem to matter.

Good point. I am not sure why I used map instead of unordered map. I will weigh the pros and cons.

2. line 97 of driver.cpp, do we need to determine whether options.suites_to_run is empty before calling the split function? if it's empty, we don't need call split function.

I thought about this quite a bit when implementing it, but decided not to test because there are many conditions under which a non-empty input can still result in an empty list of suites to run and it is not clear if "run all suites" should be true only if the input string is empty. What if the string is ;; or just spaces? The better thing to do is to return early in split if the input is empty.

3. Why the'split' function is implemented in the header file and not in the '.cpp' file?

split is a function template.

4. Some places did not use UIS. (Do we need to use UIS all the time?).

Quite possible: old habit 😢. Also, initial values that from long/complex expressions are traditionally written with assignment. Regardless, it will be helpful to use in-situ comment and the "suggest" feature and point out the locations where UIS is missed. I too will look for them.

5. test.cpp line 85, do we need to throw an exception if the name is empty.

Possibly. I'll look into it.

[smurthys]

I just pushed changes with variable-renaming and comment edits I made during the call earlier this afternoon. I also added checks for duplicate test-suite name and other errors when adding a test suite.

[smurthys]

I just pushed the following two changes:

• Reject empty test suite name (though the macro does not permit an empty suite name)
• Define and use enum for error codes in driver.cpp

I expect these to be the last changes I expect to initiate on this PR. Of course, I'll address issues you report. I would appreciate being able to close this PR tonight. Thank you for your assistance.

[cmcmone]

I tested and everything looks great. Thank you @smurthys @Ndemco @Ajetski for your contributions.

A little personal thought may not be within the scope of this PR.

1. Is it more visual to print a table? (add an option to print the table)

name	completed	passed	failed
array_test	32	32	0
Total	32	32	0

2. If the parameter input is similar to -run array_test; string_test, the exception throw is incorrect number of options, but I think to throw an exception like wrong run command is more appropriate.

[smurthys]

Thanks @cmcmone for your review. It will be nice to format test summary as a table, and you are right that that task is not related to this PR or milestone. I recommend creating an issue so we can prioritize and track.

Any cmd-line parameter containing a has a space needs to be quoted if that item has to be treated as one value. This is an OS requirement. So, the OS considers the cmd-line -run array_test; string_test as having three parts -run, array_test, and string_test. Thus, the error message of "incorrect number of options" is correct. However, the cmd-line -run "array_test; string_test" would result in two parts -run and array_test; string_test.

That said, as far as the -run option goes, you need to be careful about the space in the option value. The value array_test; string_test asks to run a suite whose name begins with a space, and a suite with that exact name must exist. In theory, it is possible to have such suite names, but not with the TEST_SUITE macro.

A solution would be to trim leading and trailing spaces before looking up suite names in the collection, but I worry about spending valuable resources (time) on addressing extreme/unlikely cases. However, I also think it is unwise to not document or handle this issue at an appropriate time.

I propose you add an issue describing this problem and link to this (my) comment.

[smurthys]

I updated the cmd-line documentation to account for the notion of suites. It turned out to be lot of changes. 😮

BTW, I'm not happy with the background of the image at the start of the doc. I don't know why the background has a grid, but I am not able to attend to that at this moment. I welcome others to give it a shot.

EDIT: I am attaching the PPT file I used to generate the image containing the annotated example, in case someone wants to experiment.

cmd-line doc example.pptx

[cmcmone]

...I don't know why the background has a grid....

@smurthys I don’t quite understand, I don’t see any grid. 😕

[smurthys]

Compare the gray background in the image (the one with annotations) with the background in the Examples section. The background in the examples are smooth, whereas the background in the image is not as smooth.

I believe the image's background was much smoother in the previous version, and @Ndemco had created that image. Clearly, I'm doing something wrong in how I created the image.

All this said, the image background is not a big issue. I am willing to live with it, at least for now.

[cmcmone]

@smurthys like this?

cmd-line.doc.example.pptx

[smurthys]

Yes @cmcmone. The image you show does have a smooth background even with the annotations.

[cmcmone]

Set by shape outline -> theme colors.

[smurthys]

Thanks @cmcmone. I'll try it next time. In the meantime, I used the image you shared in your earlier comment.

[cmcmone]

Everything looks great! Thank you @smurthys @Ndemco @Ajetski for your contributions.

[Ndemco]

All looks good! Thanks @smurthys, @cmcmone, and @Ajetski for your contributions to this PR and milestone.