-
Notifications
You must be signed in to change notification settings - Fork 443
Add overload for assert functions for unit test #28341
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: stoutes <[email protected]>
Signed-off-by: stoutes <[email protected]>
Signed-off-by: stoutes <[email protected]>
Signed-off-by: stoutes <[email protected]>
Signed-off-by: stoutes <[email protected]>
| var msg = "assertTrue failed. Given expression is False"; | ||
|
|
||
| // Append additional arguments to error message | ||
| if n > 0 { | ||
| msg += " - "; | ||
| for param i in 0..<n { | ||
| msg += args(i): string; | ||
| if i < n-1 then msg += " "; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking inspiration from the existing Errors.chpl assert code, this can be simplified to just use chpl_stringify_wrapper. This will make it more consistent with the behavior of assert and writeln
i.e.
var msg = "assertTrue failed. Given expression is False - " + chpl_stringify_wrapper((...args));
throw new AssertionError(msg);
| */ | ||
| pragma "insert line file info" | ||
| pragma "always propagate line file info" | ||
| proc assertTrue(test: bool, args...?n) throws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, note that n is always greater than 0, so no need to have the if statement
| /* | ||
| Assert that ``first != second``. If they are equal, | ||
| prints the two values along with any additional arguments provided. | ||
|
|
||
| :arg first: The first object to compare | ||
| :arg second: The second object to compare | ||
| :arg args: additional values to print on failure | ||
| */ | ||
| proc assertNotEqual(first, second, args...?n) throws { | ||
| if first.type == second.type { | ||
| if all(first == second) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementations of assertNotEqual, assertGreaterThan, and assertLessThan are not right, they should be using the same logic as the existing overloads.
| /* | ||
| Assert that ``test`` is `true`. If it is false, prints | ||
| 'assert failed' along with any additional arguments provided. | ||
|
|
||
| :arg test: the boolean condition | ||
| :type test: `bool` | ||
| :arg args: additional values to print on failure | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is false, prints 'assert failed' along with any additional arguments provided.
This looks copied from the Errors module, it should be adjusted for this use case.
e..g, "If it is false, adds the args to the thrown errors message as if those args were printed using :proc:`~IO.write()`"
This should also have the :throws: tag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested this with start_test test/library/packages/UnitTest/assertWithArgs.chpl? It doesn't look as if this good file will pass?
regardless, while these tests are fine and should be kept, I think you should also expand tests in test/library/packages/UnitTest/Assert*/*.chpl tests to include your new overloads, as those are more exhaustive in terms of types passed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is missing a .good file
Resolves #28311
This feature adds the overloads for assert functions:
This will allow additional args on an assert failure to be printed.
Also added unit test files in UnitTest.