pFUnit

pFUnit copied to clipboard

Adding incorrect arguments to @assertEqual gives vague errors

Context: When converting a test to pFUnit, the previous system required specifying the size of the array to compare when comparing that two arrays are equal.

What failed with the vague error:

expected = [4.0_dp]
actual = SUT()
call assert_equals(expected, actual, size(expected))

what worked correctly

expected = [4.0_dp]
actual = SUT()
call assert_equals(expected, actual)

pFUnit handles arrays nicely hooray! But the error message provided is super vague (to me)

e.g. error message for the failing code

   33 |   ! @test
      |        1
Error: There is no specific subroutine for the generic ‘assertequal’ at (1)
/media/store/krs/itar_build/heat/heat_main/heat/tests/unit_tests/core/utility/tblp/test_vector_linear_interp.pf:72:8:

Fatal Error: Cannot open module file ‘test_vector_linear_interp.mod’ for reading at (1): No such file or directory
compilation terminated.

I think this is regular fortran speak for I couldn't find a subroutine with the matching signature, but it took me a long time to figure out this silly mistake.

Can or does pFUnit generate warnings for errors like this, specifically a warning during the parsing of the .pf file that can generate a better warning? I think the part I was choking on was connecting @assertEqual to assertequal. Do you have any other suggestions on how that might be made more clear?

could the pre-processing catch this kind of error and fail to generate the .F90 file?

The details of just what was wrong with the @assert you attempted would probably help a little bit. But in general the preprocessor is written in a way that is semi-intentionally "dumb". This allows the framework to continue overloading with extensions without needing to go back and make corresponding modifications to the Python preprocessor. There are also limits to what the Python preprocessor can even do without parsing Fortran. There are some checks for the number of arguments, but even those are weak constraints due to the variety of overloads that are permitted.

Thank you @tclune. That is very helpful information. I updated my first comment with more clear description of the test comparing two arrays with 1 element.

Is there a debug option that generates more output or a way to generate a warning using those weak constraints?

also I am content with closing this issue due it fundamentally being an "error between chair and computer" but wanted to determine if the was a mechanism to generate a warning, or make it more clear to others when they encounter an error like this.

There is no flag that would give a useful message for that type of error. There are many cases of AssertEqual that require 3 arguments, so nothing more can be done in that regard. And there is no way for the parser to know that size(expected) is of a type that is not accepted in the 3rd argument.

I do require optional arguments (e.g., message and location) to be passed using keywords, but that still is only detected by the compiler. Doing something similar with the annotations should be possible and would provide more informative error messages. To flag your example though, we would have to require a keyword even for tolerance (which is otherwise a common 3rd argument). That unfortunately breaks backward compatibility.

OK, so maybe add a switch to the preprocessor that would optionally enforce keywords for some (all?) arguments to the macros? You would then at least get a more informative message during the processing phase if you were willing to live with the more verbose form. Note that it would still not trap cases where the keywords are "right" but the argument is of the wrong type. E.g., if you had typed

@assertEqual(expected=5, found=7, tolerance=size(expected))

You would still get the same confusing message from the compiler as "tolerance" does not accept an integer argument. (I think.)

I am not at all opposed to accepting a contribution to extend the preprocessor in such a fashion. It ought to be relatively straightforward, though probably a bit tedious to capture all the cases that are currently treated generically in one fell swoop.