Proposal for test_condition #121
Comments
milancurcic
added
topic: utilities
|
I kind of like the name Also this is probably going to be part of a testing framework. So we should look at what other Fortran testing frameworks use:
And for comparison, C++ testing frameworks:
I am thinking maybe it could be called |
|
|
Nice proposal. I would prefer the name |
|
It has been on my list to make a public version of testing and debugging Since something related is starting here and I have reviewed most of the Specifically focusing on tests comprising a unit test (versus parameter In tests that I call that I want part of the run-time production routines Other things I found different
FOR MORE INFORMATION ON M_DEBUGSome unpolished parts of what I had in mind for what I was going to I usually mix the unit tests in M_debug with generic routines similar Note that if anyone builds the GPF repository it tries to run at least a So lots of other things are possible, but normally you call very simple routines unit_check_start(... Generally for quick confidence tests to make sure things did not accidentally I have mine set up so a "test_suite" routine is searched for and |
|
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 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)? |
|
In C++ I have been using |
|
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 I would not support the addition of a run time checking facility because I think it encourages poor software design. |
|
Thank you for your feedback @everythingfunctional. I'm confused about some terminology so I need to ask.
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 Would you then suggest removing the option to stop the program here altogether (
In case of a
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
Can you explain why? I don't see it. So many languages and libraries do it successfully IMO. |
|
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. 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. So, if |
|
@everythingfunctional Okay, great, I understand better now.
Great, I agree, this indeed is not what
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.
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 :: codeThis subroutine emits Would you at least not object to this variant? |
|
I have a tweak to make the usage alluded to a bit more convenient. so that one could write a line like I would still lean towards not having the function emit
|
|
One thing that I am still confused is what the intended audience of this Also, a good testsuite framework (such as Wouldn't it make sense to simply leave such |
|
The Additionally, I don't think test code should be mixed in with production code. Test code should call production code, not the other way around. You may be right, that
The basic use case for this is to be able to write code that, in production stops execution, but under test does not. You could have a procedure with optional arguments that could be used to return error conditions under test, but would not be utilized in production code. I.e. so that in production code you would just but in test code you could I think this provides a reasonable way to improve code that simply uses |
|
@certik Here are some use cases:
As you can see from the use cases, two essential elements of this subroutine are:
So rather than testing, perhaps think about this as a simple warn subroutine, that can optionally also stop. I agree with Brad that this is not meant to make a testing framework. I think I understand Brad's use case, but I personally wouldn't use it like that. I do object to this being a function and not a subroutine. The key part of this procedure is that it causes a side-effect (print to stderr and/or stop the program). That's pretty much all it does. One of the few things that I think Fortran got right over other languages is that it has a function and a subroutine. Functions can be used only in expressions. Subroutines can't be used in expressions. Expressions shouldn't cause side-effects. This is especially troublesome if the expression is a logical condition being tested (Brad's use case), or if its passed as an input argument to another procedure. If I understand correctly, Regarding Brad's wish that subroutine check(condition, msg, fatal, code)
logical, intent(in) :: condition
character(*), intent(in), optional :: msg
logical, intent(in), optional :: fatal
integer, intent(in), optional :: codeNow you can use it like this: or: What do you think? |
|
The discussion is already pretty lengthy - I will read it later today. Being of an older generation and using an optical device to enhance my vision, I have to ask: is there any easy way to get this thread on paper? I really prefer to read from that ancient medium ;). |
|
If I agree that in most cases side effects should be performed by subroutines, but in this case I think the general use case warrants it. i understand the use cases you have shown are prevalent in many code bases, but I don't think they are a good design. If the following is inside a procedure that needs to be inside a loop you'll see this warning tons of times, without necessarily any context as to what's causing it, and you may only really want to see it once. Not to mention if I hit this condition in a test it may clutter up the output from my tests, and makes it much more difficult to test that this condition does emit the message. If the following is in a procedure, now I can't test that procedure to see that inputs causing My thought is that we could use this as a way to demonstrate that the use case I've shown is a relatively straightforward change from patterns that are in common use today, but that makes code more testable. Once that's been demonstrated we can request a I think this is a good use of an (experimental) standard library; demonstrate a use case for a feature that should be added to the language, with a clear upgrade path from existing code to the stdlib feature, and a clear upgrade path from the stdlib to the new langauge feature. |
|
@arjenmarkus unfortunately I can't quickly figure out how to print a GitHub issue. However, if this is something that would allow you to participate here, we can use GitHub API to get the Markdown contents of the issue and all the comments, and then we can convert to pdf that can be printed. It's some work, so I can't do it right away. Let me know if this would be a high priority for you. |
|
@certik Thanks for the offer - it is most a matter of convenience: I could read the thread from paper while commuting. Since there does not seem to be a convenient way to do it, I will just read it from screen instead. |
|
Having read the discussion, I would like to add the following:
Using an object in this way, rather than a single routine or a set of routines, is customary in languages like C++ and Java where object-orientation is the preferred programming model. The thing we should avoid is over-designing. It is easy to make a collection of routines (or methods) that allow us to change the behaviour in a myriad of ways, but let us stick to those aspects that are evidently useful at first. Extensions can be made in a later stage. Having said that, I know it is hard to answer the question "what is evidently useful?" ;). |
|
To be honest I am also struggling to understand the motivation for this. I understand the motivation for Debug time assert, which is #72. I also understand the motivation for a testing framework, which I feel might be beyond the scope of stdlib (yes, we have to write tests in stdlib somehow, for now we simply call That leaves the use cases 1., 2., and 3. from #121 (comment). Of those I usually just write an So I personally do not see the motivation for this yet, but if others find this useful, I am not against. |
Exactly, this is the crux. It's a convenience wrapper over
I can understand this. However, how do you motivate the need for the current implementation of I'd argue that What motivated me to propose this is the idea that stdlib would also provide convenience routines like this that simplify repetitive boilerplate. Do you think that stdlib should't provide utilities like this? |
|
I see. Yes, the If you see it as a better "assert", then I am for it. Essentially something that we can use today to write tests. And if we want more, then we would switch to a full blown test framework. Is that your idea? |
|
This started as a backward compatible improvement to If current implementation of assert is indeed to eventually retire in favor of a preprocessor macro, then I don't think it hurts to "upgrade" the current implementation of
I do think @everythingfunctional ideas brought up here are important and we should discuss them. They do seem outside of scope of this proposal and seem to be more akin to #95. |
|
If the goal is to retire our current |
|
Let's pick this discussion up. It seems like there is satisfactory level of interest to retire Let's look at the API again: subroutine check(condition, msg, code, warn)
logical, intent(in) :: condition
character(*), intent(in), optional :: msg
integer, intent(in), optional :: code
logical, intent(in), optional :: warnThis API is compatible with current Implemenation could be incremental:
Are there any objections to this API before I proceed with a PR? |
Yes, I think so. It would preserve the behavior of
This is quite easy and I don't mind doing it. We don't have that many modules and functions. All 4 steps above can be in a single PR.
No. We're already hitting problems in development with current |
|
Is there any consensus/progress in this issue? Could I be of any help? |
|
I'm glad you asked! I meant to tackle this next, sometime this coming week. I have a new baby and today is the last day of my 3-week parental leave, and thus lower activity on projects. If you're eager to move it forward, please! Otherwise, I'd tackle it in the next few days. I think this is the implementation we're looking for: subroutine check(condition, msg, code, warn)
logical, intent(in) :: condition
character(*), intent(in), optional :: msg
integer, intent(in), optional :: code
logical, intent(in), optional :: warn
character(*), parameter :: msg_default = 'Test failed.'
if (.not. condition) then
if (optval(warn, .false.)) then
write(srderr,*) optval(msg, msg_default)
else
call error_stop(optval(msg, msg_default), code)
end if
end if
end subroutine checkImportant points are:
How does that sound? |
|
I still don't understand how this provides functionality different from the current I would like to see the following implementation function check(condition, code, msg, fatal, stat)
logical, intent(in) :: condition
integer, intent(in) :: code
character(len=*), intent(in), optional :: msg
logical, intent(in), optional :: fatal
integer, intent(out), optional :: stat
logical :: check
character(len=*), parameter :: DEFAULT_MESSAGE = 'Check Failed.'
if (.not. condition) then
if (optval(fatal, .false.)) then
call error_stop(optval(msg, DEFAULT_MESSAGE), code)
else
if (present(stat)) then
stat = code
check = .true.
else
call error_stop(optval(msg, DEFAULT_MESSAGE), code)
end if
end if
else
check = .false.
end if
end function checkI think @milancurcic use case could be accomplished by simply adding an optional |
|
@everythingfunctional #116 shows how we got to here. |
|
I like Brad's proposed implementation also, if either we:
To my eyes, Brad's As for why make I am not opposed to having multiple functions and subroutines with somewhat overlapping but different capabilities. |
|
@jvdp1 Let's keep this at the drawing board until we have a majority agreement. |
Ok, I think I see what we've got now. Multiple uses cases/ideas trying to get in on one feature. Here's what I think the uses cases are:
I think checking in the context of testing should not be in the scope a the standard library. That is the responsibility of a testing library/framework. Besides, if I can tell which testing framework you're using by looking at your production code, then you're doing it wrong. I don't think static/compile time checking would fall under the purview of the standard library either. It would probably need to be implemented by some sort of macro/preprocessor anyway. In order for something to be optimized away under different compiler options, it would need to be a language feature (or maybe make use of macros or preprocessors), which would again put it outside the purview of the standard library. That leaves run time checking of user inputs and run time checking of intermediate results. I think the solution to these problems needs to be designed with respect to the kind of usage patterns we would like to see encouraged, and possibly discouraged, not simply the patterns currently in common usage. Sometimes it's better to remove capabilities from a language than it is to add new ones. Fortran's history is littered with bad design decisions. Like computed goto and alternate entry points. With great power comes great spaghetti code. |
|
Thanks for the summary of use cases. I agree that the problem is exactly "Multiple uses cases/ideas trying to get in on one feature".
I don't see it as black and white. Recent and popular languages like Python and Go do it successfully. I bet many others too. A small project will often roll its own testing functions rather than relying on a heavy testing framework. Though I tend to agree that a general and powerful testing framework may be out of scope for stdlib, you still need a way to test stdlib functions internally. Adding an external framework as a dependency is not feasible (not even vegetables, my favorite of the bunch), and current We can safely ignore here the preprocessor macro and language developments. As you say, let's focus on run time checking of user inputs and run time checking of intermediate results. |
Congratulations @milancurcic ! |
|
My point is that stdlib functions shouldn't be tested internally. A testing framework should be used to test the stdlib from the outside. That way a user of the stdlib doesn't have to rely on the testing code. I understand that some languages make use of doctests and inline tests, but they make use of capabilities of those languages that Fortran doesn't have. Introspection and standardized docstrings in the case of Python, and fully AST aware macros and a builtin |
|
A couple of questions to try to understand pros/cons in this thread:
Why is it not feasible? My main issue in this thread is that the use of multiple |
Okay, your point is now clear to me. However, I still don't understand why (they shouldn't be tested internally). Are there practical reasons or is it more of a dogma?
I don't think this is an answer to my question above because if you take care of testing internally, then this problem goes away. Moreover, I want as many users to run stdlib tests and as seamlessly as possible. This will help us discover bugs and cross-platform issues. As a user myself, I want to run tests for most libraries I use, and I don't want to have to install Perl, Ruby, or Haskell to do so. |
I think it would add too much friction to development (it would for me), and it would discourage users from running tests.
Same. There's an easy solution that works, now. @everythingfunctional Recall also that we are now working only with experimental API, where we're supposed to try out and play with various approaches, and drop ones that don't work as we go. Nothing is set in stone here. This kind of analysis paralysis prevents work from getting done. Perfect is the killer of good enough. |
With Fortran, if your tests are inside your procedures, they'll be run every time a user calls that procedure. That adds overhead to your library.
I don't think the problem goes away, I just think it's no longer apparent to the user.
I'm not sure I understand this statement. How does having more people run the same tests help to find bugs? To me, the best way to find bugs is to have more people use the code in various ways. Otherwise your tests would have already found the bug. I'm not saying the tests don't have value to outside users. In fact they are a great example of how your code is intended to be used, and done well describe the requirements the code is intended to fulfill. But your users should be able to see that your CI system is running the tests and trust that they pass. They shouldn't really need to run them themselves. |
|
@milancurcic I understand your point of view. An extension of the current Meanwhile, a testing framework could be thought/set up for when we will move procedures from experimental part to the non-experimental part. Of course, this will not encourage the users to run tests. |
I agree with that. However, I think we discussed different things. Testing are outside the procedures, but well inside the directory/project of call assert( (mean(x) - 1) < sptol )As implemented now, it is a bit as it is in BLAS/LAPACK: the project includes tests, that the the user may or may not run. There are no tests inside proedures of BLAS/LAPACK (except for validation of the values passed to procedures of course). Do I miss something? |
Okay, I agree and I didn't have that in mind at all. I think of tests as separate programs that call library functions, and these programs may or may not ship alongside the library. What we're doing now with
You're right, it doesn't really help if a user runs tests in the same environment as developers, and using the code in various ways is indeed more beneficial. However the output of tests will not always be the same across compilers, compiler versions, and operating systems. |
Ok, so you're using |
|
I wrote the current In the long run, I agree it's not a good solution for either use case. For testing, we should use a testing framework that is integrated with For Debug time tests, probably the only way currently in Fortran is to use a macro (until j3-fortran/fortran_proposals#70 is accepted). However, I think this macro could be maintained in stdlib, as it would be very useful. |
|
Per my previous comment, here is my proposal:
|
|
Thank you @certik. I like the long term plan and also support introducing In the meantime, do you object extending existing I will take some time to tally up current support/objections and elicit further support for this. @everythingfunctional Can you please open an issue to propose an external testing framework for stdlib testing? |
|
sure thing |
|
@milancurcic I don't object at all. If it makes your life easier, go ahead and submit a PR. This is in experimental, so we should be free to experiment until we figure out what we want. |
milancurcic
added
API
|
Fixed by #163 |
Evolved from discussion in #116.
Summary
This is a proposal to add a subroutine
test_conditiontostdlib_experimental_errorthat will:assertinstdlib_experimental_errordoes;assert;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_conditiontests 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
Arguments
condition: Logical condition to testmsg: (optional) Character string to print to stderr. Defaultcode: (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_conditionwill stop the program ifconditionis also false. If.true., it will print the error message to the screen and resume.Example
Implementation
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.
The text was updated successfully, but these errors were encountered: