On 20/07/06, chromatic <[EMAIL PROTECTED]> wrote:
On Thursday 20 July 2006 14:46, Fergal Daly wrote:> Your example does not produce any incorrect output. Perhaps "incorrect" is a very poor word choice, but I certainly consider the stack trace behavior potentially unhelpful and potentially misleading. I will make one more attempt to demonstrate. use Test::More tests => 2; main(); sub main { run_first_tests(); run_second_tests(); } sub run_first_tests { # set up the variables here ok( $something, $other, $description ); # more tests.... } sub run_second_tests { # set up more variables here ok( $something $other, $description ); # more tests.... } I consider the current, default behavior of Test::Builder here to be correct. It reports the point at which the test file made the call to a test function that failed. Adding a stack trace on failure is not useful in this case, because it incorrectly reports call frames that are not germane to diagnosing the failure.
I agree, however you're only calling your subroutines once. As soon as you start calling a subroutine from more than one location in the test script a single line number is no longer enough to pinpoint the problem. There are two reasons to use subroutines. 1 to group related code in a single unit with a descriptive name 2 code reuse - calling the subroutine from multiple places Your example is #1 and T::B currently supports it perfectly. It does not support #2 and I have written far more tests involving #2 than #1. I would guess if you look on CPAN (certainly in my code) you'll find very few examples of #1, some examples of #2 and plenty of test code that would be #2 if it was refactored a little bit (perhaps it wasn't written as #2 precisely because T::B doesn't give enough information).
main() is not meaningful. All of the tests run above this frame in the call stack. run_first_tests() is not meaningful; this call has no effect on the tests as it takes no arguments and merely provides a method of grouping tests. run_second_tests() is not meaningful for the same reason. Yes, the trace will *include* the frame corresponding to the failing test, but it will also include this extra information, so that anyone who has to debug the failing test will have to dig through that extra information to find the single useful piece of information. In my mind, that is a step backwards in usability. This is not a contrived example. Consider Test::Class and its flow of control. Every test that runs runs from its runtests() method. What value is there in reporting that? It's noise. If there is some algorithm or heuristic that can distinguish between call frames useful for debugging and call frames that are not (that is, subroutine calls and loop constructs that have a semantic meaning on the tests being run versus subroutine calls and other constructs that have no such meaning), perhaps I would be more comfortable with this behavior. I don't believe that such a thing is possible. That's why $Level exists. It's definitely not perfect, but it is reliable for the single point of information it conveys. (I consider the idea that a $MaxLevel could exist, but that's certainly ugly and, I fear, prone to even more incorrect uses.)
This is a different point, Test::Class inverts the usual flow of control. I agree that a plain old stack trace in that situation would be troublesome. I also agree that a solution could be found but it might not be so pleasant. That's fine by me if Test::Class users don't want stack traces they don't have to have them, F
-- c
