[Development] Supporting helper functions in auto tests by providing throwing Qt Test macros

[Mitch Curtis]

> -----Original Message-----
> From: Edward Welbourne
> Sent: Thursday, 4 April 2019 10:51 AM
> To: Mitch Curtis <mitch.curtis at qt.io>
> Cc: development at qt-project.org; Joerg Bornemann
> <Joerg.Bornemann at qt.io>
> Subject: Re: [Development] Supporting helper functions in auto tests by
> providing throwing Qt Test macros
> 
> Edward Welbourne (Wednesday, 3 April 2019 5:07 PM)
> >> It's probably worth having a macro with which to wrap code, that'll
> >> catch any exception it raises, add the current location to what'll be
> >> output to log the failure and then rethrows.  This would give us the
> >> extra file/line information that your proposed macro gives, compared
> >> to a naive throw/catch solution (where the failure throws and
> >> TestLib's driver system catches, reporting the helper but not its
> >> caller).
> 
> Mitch Curtis (4 April 2019 08:43)
> > What would the effective difference be from QCHECK_EXCEPTION in the
> > proposed patch, in terms of behaviour and output?
> 
> Probably very little - I haven't closely reviewed your QCHECK_EXCEPTION
> patch, nor am I used to using C++ exceptions.  However, in an exception-
> based QTest, QCHECK_EXCEPTION would re-raise the exception it caught
> (after logging where it caught it) rather than returning; and this would mean
> that a helper could apply it also to places where the helper calls a sub-helper
> (see below) to get a full stack-trace for the failure.
> 
> >> I think the idea of rewriting TestLib (including the existing QFAIL,
> >> QCOMPARE, QVERIFY and friends) to use exceptions in Qt6 is a good way
> >> forward; adding some convenience macro for currentTestFailed() checks
> >> is probably sufficient to make helper functions easier to use until
> >> then.
> 
> > The currentTestFailed() macro is a nice idea, but it's not sufficient
> > if your helpers call other functions, because then you have to wrap
> > every call site of those functions with the macro.
> 
> If you're going to have helpers calling sub-helpers in this way, your rationale
> for the primary test function having QCHECK_EXCEPTION add data about its
> call-site of the helper surely applies also to the helper's call to the sub-helper,
> so surely you want to do the same kinds of wrapping with
> QCHECK_EXCEPTION in the helper.  Of course, if you leave it out at some
> layer in an exception-based solution, it only loses you knowledge of part of
> your stack-trace.
> 
> Then again, if you omit a currentTestFailed() check in a helper's call to the
> sub-helper, you just get the rest of the helper run, despite some part of the
> test code having failed already.  That can be a major problem if later parts of
> the helper rely on the sub-helper's success; but "I rely on its success" is
> exactly what you express by wrapping code in the QCHECK macro.  (And, of
> course, this macro can do logging of its call-site just the same as the
> exception-based one can.)
> 
> Whatever we do in Qt5, though, should be designed to be compatible with
> Qt 6 switching to an exception-based QTest; that would surely be a better
> way forward, if we can pull it off,
> 
> 	Eddy.

The nice thing about the exception solution is that users can decide whether they want to wrap all helper calls in QCHECK_EXCEPTION to get complete stack traces for failures, or if they don't want to use it at all, resulting in neater code but slightly more difficult traces. If they didn't use QCHECK_EXCEPTION at all (and we made sure we caught exceptions correctly in the test runner), they'd get output like this:

   FAIL!  : tst_App::openClose(ImageType) 'animationPanel->isVisible()' returned FALSE. ()
      Loc: [/home/mitch/dev/slate/tests/shared/testhelper.cpp(1990)]