Proposal for test_condition

[milancurcic]

Evolved from discussion in #116.

Summary

This is a proposal to add a subroutine test_condition to stdlib_experimental_error that will:

• Test the value of a logical condition like assert in stdlib_experimental_error does;
• Allow printing a custom error message to stderr;
• Optionally, allow passing an exit code like with assert;
• Optionally, only print the message without stopping the program, i.e. warn only;

This is useful for testing both programmer and user errors. It can be used regardless of how the program is built (debug or release mode).

It could also be used as a basic building block toward higher-level testing or exception handling proposed in #95.

Description

test_condition tests an input logical condition. If the condition evaluates to .false., the default behavior is to stop the program and print the error message. Optionally, the user can pass an exit code to stop the program with. Optionally, the user can choose to not stop the program, but only print the warning.

API

subroutine test_condition(condition, msg, code, warn)
  logical, intent(in) :: condition
  character(*), intent(in), optional :: msg
  integer, intent(in), optional :: code
  logical, intent(in), optional :: warn

Arguments

• condition: Logical condition to test
• msg: (optional) Character string to print to stderr. Default
• code: (optional) Integer exit code to set when stopping the program. (what should be the default value? Current implementation of assert doesn't set any default code as far as I can tell.)
• warn: (optional) Logical. If .false. (default), test_condition will stop the program if condition is also false. If .true., it will print the error message to the screen and resume.

Example

use stdlib_experimental_error, only: test_condition
...
call test_condition(n > 0, 'n > 0 failed', warn=.true.)

Implementation

subroutine test_condition(condition, msg, code, warn)
  logical, intent(in) :: condition
  character(*), intent(in), optional :: msg
  integer, intent(in), optional :: code
  logical, intent(in), optional :: warn
  if (.not. condition) then
    if (optval(warn, .false.)) then
      write(srderr,*) optval(msg, "Test failed.")
    else
      call error_stop(optval(msg, "Test failed."), code)
    end if
  end if
end subroutine test_condition

This implementation depends on optval and error_stop.

Requesting feedback from @certik @nncarlson @zbeekman @jvdp1 @ivan-pi.

For anybody reading, please explicitly thumbs up or down, and write a comment if thumbs down. This way we can get a clear idea on whether this is supported or not and how much.

[certik]

I kind of like the name assert for writing tests. Obviously we need to distinguish it from the Debug assert macro. Rust has assert (always executed in both Debug and Release modes) and debug_assert (only in Debug mode): https://doc.rust-lang.org/std/macro.assert.html.

Also this is probably going to be part of a testing framework. So we should look at what other Fortran testing frameworks use:

• funit: assert_true
• pfunit: assertTrue
• FRUIT: assert_true
• flibs: assert
• zofu: test%assert
• ObjexxFTK: CHECK
• vegetables: assertThat

And for comparison, C++ testing frameworks:

• doctest: CHECK
• catch2: REQUIRE
• Google Test: ASSERT_TRUE
• Boost Test: BOOST_CHECK
• cppunit: CPPUT_CHECK
• Cute: ASSERT

I am thinking maybe it could be called assert_true, and later we can add others like assert_eq, etc.

[pdebuyl]

• fortran_tester my_tester%assert_equal, my_tester%assert_positive, my_tester%assert_close (arguably, those only cover numerical tests. I only realize this now :-) ).

[jvdp1]

Nice proposal. I would prefer the name assert_true (or maybe test_true) over test_condition. Anyway, it will be quite useful for debugging when multiple test are present.

[urbanjost]

It has been on my list to make a public version of testing and debugging
code including unit testing so I found this topic particularly intriguing.

Since something related is starting here and I have reviewed most of the
listed utilities before I thought I would just list some functionality
unique to what I use as food for thought ...

Specifically focusing on tests comprising a unit test (versus parameter
checking and other run-time testing functionality) a few differences between
the referenced unit testers and what I like in my version are:

There is a number of global parameters that are set external to the 
main "assert" routine (I call mine UNIT_CHECK):

* A "level" value which is just an integer that all the tests can
  access that the user is free to use to determine what level of
  testing to perform and/or how verbose to be.

* As mentioned, an option to make a failure stop the test program
  or to optionally continue (some of the references have this,
  but as an option on "assert", not as a global mode).

* An option to skip producing messages when successful to make the output
  more succinct (some of the references have this, but as an option on
  "assert", not as a global mode).

In tests that I call that I want part of the run-time production routines
I want options like this to be parameters on the call to the "assert"
test routine, but in unit testing I want these to be run-time selectable
global options for the entire set of tests.

Other things I found different

• A name (almost always the procedure name being tested) is required as
  the first parameter in my "assert" routine.

• One I am pretty sure is totally unique is an optional command name
  to call when starting and stopping a test set.
  This lets you arbitrarily enter test results into an SQLite
  database file, a CSV log, send mail messages, or whatever arbitrary
  action you want, versus just printing a simple log or exiting with
  a non-zero exit status -- you provide the command.

• the "message field" is actually the "message fields" on mine, which is
  up to nine polymorphic variables so you can easily include numeric
  and logical values in the message, not just strings. Requires Modern
  Fortran, of course -- but a major convenience.

FOR MORE INFORMATION ON M_DEBUG

Some unpolished parts of what I had in mind for what I was going to
release have already been extracted and made available in the General
Purpose Fortran
repository, primarily in the M_debug module, as I needed some basic
testing functionallity just to start GPF. So I agree based on experience
that the functionality of unit testing is needed pretty early on in
stdlib.

I usually mix the unit tests in M_debug with generic routines similar
to the dp_accdig() routine in M_math (to compare REAL values with a
tolerance) and the ufpp(1) preprocessor $SYSTEM directive and the
numdiff(1) program... so M_debug is intentionally simple and not
complete by itself, but is more a framework than a full utility. I
think that is a good model -- keep the unit test routines simple and
use generic routines for comparing reals. These routines can be called
in the expression field of "assert" instead of being provided as a set
of routines. Powerful testing intrinsics like ANY() and ALL() already
exist and I do not think need re-invented.

Note that if anyone builds the GPF repository it tries to run at least a
few thousand tests at the end so there might not be a lot of documenation
but there are a lot of examples, at least if you use gfortran on a
*nix system (which is the default expected environment).

So lots of other things are possible, but normally you call very simple routines
like ...

unit_check_start(...
unit_check(name,expression,messages(s)...)
unit_check(name,expression,messages(s)...)
unit_check(name,expression,messages(s)...)
unit_check_done(...

Generally for quick confidence tests to make sure things did not accidentally
change I find (in practice) that I just let it procedure basic messages,
resulting in a simple log like:

STARTED test_suite_m_sort
unit_check:       sort_shell           SUCCESS : sort string array, ascending
unit_check:       sort_shell           SUCCESS : sort string array, descending
unit_check:       sort_shell           SUCCESS : sort integer, ascending array
unit_check:       sort_shell           SUCCESS : sort integer, descending array
unit_check:       sort_shell           SUCCESS : sort real, ascending
unit_check:       sort_shell           SUCCESS : sort real, descending
unit_check:       sort_shell           SUCCESS : sort doubleprecision, ascending
unit_check:       sort_shell           SUCCESS : sort doubleprecision, descending
unit_check:       sort_shell           SUCCESS : sort complex by real component, ascending
unit_check:       sort_shell           SUCCESS : sort complex by real component, descending
unit_check:       sort_shell           SUCCESS : sort complex by imaginary component, ascending
unit_check:       sort_shell           SUCCESS : sort complex by imaginary component, descending
unit_check:       sort_shell           SUCCESS : sort complex array by magnitude, ascending
unit_check:       sort_shell           SUCCESS : sort complex array by magnitude, descending
unit_check:       sort_shell           SUCCESS : sort double complex by real component, ascending
unit_check:       sort_shell           SUCCESS : sort double complex by real component, descending
unit_check:       sort_shell           SUCCESS : sort double complex by imaginary component, ascending
unit_check:       sort_shell           SUCCESS : sort double complex by imaginary component, descending
unit_check:       sort_shell           SUCCESS : sort double complex by magnitude, ascending
unit_check:       sort_shell           SUCCESS : sort double complex by magnitude, descending
unit_check_done:  sort_shell           PASSED  : GOOD:20  BAD:0
                      :
                      :
unit_check:       cosd                 SUCCESS : cosd 1.00000000 1.00000000 value= 0.00000000 accuracy= 1.35631564E-19 asked for 5 digits
unit_check:       cosd                 SUCCESS : cosd 0.866025388 0.866025388 value= 30.0000000 accuracy= 1.35631564E-19 asked for 5 digits
unit_check:       cosd                 SUCCESS : cosd 0.707106769 0.707106769 value= 45.0000000 accuracy= 1.35631564E-19 asked for 5 digits
unit_check:       cosd                 SUCCESS : cosd 0.500000000 0.499999970 value= 60.0000000 accuracy= 1.35631564E-19 asked for 5 digits
unit_check:       cosd                 SUCCESS : cosd -3.48994955E-02 -3.48994620E-02 value= 92.0000000 accuracy= 6.01741600 asked for 5 digits

I have mine set up so a "test_suite" routine is searched for and
automatically called at the end of the build that uses a bash shell that
calls nm to find the test routine names and then builds a test program
to call the tests, but that is a whole other discussion, I think.

[milancurcic]

Based on minimal feedback on this so far, there seems to be some support for this proposal, and no objections. I'd like to get more support here.

Regarding the naming, now there seems to be more favor for assert_true. However @nncarlson has warned earlier not to associate the word "assert" with testing. I'm fine with staying away from the word assert for testing, however library review by @certik shows that this word is quite widely used for testing (and such is my experience with say, Python). It seems to me that Neil's interpretation of assert is thus more narrow than broadly used.

Ultimately, we should take the road that most of community seems to agree on. We need more feedback here, on the naming and the whole of the proposal alike.

@zbeekman @jacobwilliams @ivan-pi @leonfoks @everythingfunctional @arjenmarkus @scivision mind chiming in (and please upvote or downvote the original post)?

[certik]

In C++ I have been using doctest and catch2, which use CHECK and REQUIRE respectively. Here is an example: https://gitlab.com/lfortran/lfortran/blob/57d3b8077d884f0ff3945ad3a86b2da920e4b6b3/src/lfortran/tests/test_parse.cpp#L1014, I think CHECK is perfectly readable also. The only issue is that in C++ it is a macro, that somehow analyses the expression and can extract the left hands side and right hand side and print useful info. In Fortran, if this was to be just a subroutine, then it would probably be called check.

[everythingfunctional]

I think run time checking and unit testing functionality should be kept completely separate. And in fact I've found that run time checking that stops execution actually makes unit testing incredibly difficult.

I'm not sure having what amounts to a fancy stop statement is a good idea. If I'm going to perform unit testing, I don't want any chance the program just stops. I want errors reported back to me. In fact I wrote a library to make this easier and more user friendly, so I could avoid having the data flow for errors be different from the normal data and control flow.

I would not support the addition of a run time checking facility because I think it encourages poor software design.

[milancurcic]

Thank you for your feedback @everythingfunctional. I'm confused about some terminology so I need to ask.

I think run time checking and unit testing functionality should be kept completely separate. And in fact I've found that run time checking that stops execution actually makes unit testing incredibly difficult.

By "run time checking", do you universally mean "run time checking that stops the program"? If yes, I agree. For my own testing (any kind really, unit, integration...), I use procedures very similar to test_condition proposed here, that don't stop the program, but only report on what succeeded or failed. In some cases I have an accumulator of successes or failures.

Would you then suggest removing the option to stop the program here altogether (warn=.true. here would then be removed as an argument and used as default)? This would cleanly separate it from the existing assert from stdlib_experimental_error.

I'm not sure having what amounts to a fancy stop statement is a good idea.

In case of a test_condition that stops, it's a convenience wrapper very much like optval is a wrapper for present.

If I'm going to perform unit testing, I don't want any chance the program just stops.

Me too, but think beyond unit testing :). Some programs (yes, even in production) need to stop gracefully on some conditions. Sure, we can use an if-branch and a stop statement, and test_condition makes it easier to do so. Do you think this is a bad idea? If yes, why?

I would not support the addition of a run time checking facility because I think it encourages poor software design.

Can you explain why? I don't see it. So many languages and libraries do it successfully IMO.

[everythingfunctional]

By run time checking in that statement I meant, anything that causes the procedure to do something beyond it's normal operation and sends that result anywhere other than back through the argument list or function result. Whether it's stopping the program, printing to screen, writing to an output file or launching the missiles, that is something that my tests (or any calling procedure) no longer has control over or access to. I don't want my tests to stop, and I don't want them cluttered up with output from the production code (or to inadvertently launch the missiles).

I don't think it's a good idea to add features to a language that enable a design/behavior we should be discouraging. Especially if it's something that could be accomplished with a library anyway. Maybe I'm being naive and idealistic though, because I could see a code base using a pattern like the following extensively, which would still enable deterministic testing.

if (.not. some_condition) then
    if (present(error)) then
        error = code
        return
    else
        print *, message
        stop code
    end if
end if

Maybe if you could get the feature to mimic that pattern and actually cause the procedure to return or stop depending on the presence of a variable and the value of a logical. I.e.

assert(some_condition, message, code, error, fatal)

So, if error is not present or fatal is present and true, then it prints the given message to std_err and stops the program, otherwise error becomes defined with the value of code and the procedure returns without printing anything. It would really be cool if code and error only had to be of the same type, and not some intrinsic type. That would be a reasonable compromise I might support (or at least not speak against).

[milancurcic]

@everythingfunctional Okay, great, I understand better now.

By run time checking in that statement I meant, anything that causes the procedure to do something beyond it's normal operation and sends that result anywhere other than back through the argument list or function result. Whether it's stopping the program, printing to screen, writing to an output file or launching the missiles, that is something that my tests (or any calling procedure) no longer has control over or access to. I don't want my tests to stop, and I don't want them cluttered up with output from the production code (or to inadvertently launch the missiles).

Great, I agree, this indeed is not what test_condition (or however we name it) is for! :) I wouldn't put this in a function, which I tend to write as pure anyway. However, I would very much like to use this in top-level programs to check the results of functions and subroutines.

I don't think it's a good idea to add features to a language that enable a design/behavior we should be discouraging. Especially if it's something that could be accomplished with a library anyway.

This is a proposal for stdlib, not the language (different repo). But otherwise, regarding discouraging certain style or behavior, it's not black and white. Any powerful feature can be used for good or harm.

Maybe if you could get the feature to mimic that pattern and actually cause the procedure to return or stop depending on the presence of a variable and the value of a logical. I.e.

assert(some_condition, message, code, error, fatal)

I think we're moving in a good direction. Considering the naming feedback from @certik (I like it also), and your feedback here, let's consider this API:

subroutine check(condition, msg, fatal, code)
  logical, intent(in) :: condition
  character(*), intent(in) :: msg
  logical, intent(in), optional :: fatal
  integer, intent(in), optional :: code

This subroutine emits msg to stderr if condition is .false.. Optionally, if fatal is .true. (default .false.), it stops the program with the exit code if provided. I like this even better than the original.
I changed the order of fatal and code (code is only meaningful if fatal is .true.). msg is also now required because warn-only is now the default behavior.

Would you at least not object to this variant?

[everythingfunctional]

I have a tweak to make the usage alluded to a bit more convenient.

function check(condition, msg, fatal, code, stat)
    logical, intent(in) :: condition
    character(len=*), intent(in) :: msg
    logical, intent(in), optional :: fatal
    integer, intent(in), optional :: code
    integer, intent(out), optional :: stat
    logical :: check

so that one could write a line like

if (check(condition, msg, fatal, code, stat)) return

I would still lean towards not having the function emit msg at all if it doesn't stop the program. Where the conditions that cause the program to stop are:

• fatal is provided and is .true.
• stat is not provided
• stat is provided but code is not (this is essentially invalid usage)