catchorg / Catch2

A modern, C++-native, test framework for unit-tests, TDD and BDD - using C++14, C++17 and later (C++11 support is in v2.x branch, and C++03 on the Catch1.x branch)
https://discord.gg/4CWS9zD
Boost Software License 1.0
18.73k stars 3.05k forks source link

Seperate `REQUIRE` (total show-stopper) for checking unit-test integrity #1811

Open TysonRayJones opened 4 years ago

TysonRayJones commented 4 years ago

Sometimes unit tests need to perform their own calculations to compare against those of the tested code. For example, in my use-case, I check that an optimised simulator is producing the same results as a slow direct mathematical evaluation.

This often requires some quick checks of the internal testing code, to check pre- and post-conditions of the helper functions, to help distinguish a bad test from an error in the tested code. This is especially helpful for external contributors writing their own unit tests which use the helper funcitons.

Right now, I use REQUIRE in the testing, and in checking the integrity of the internal code. Here's an example; a unit-test which performs its own matrix calculations in order to replicate a tested code's result (which is computed in an entirely different way), and utilises internal testing code:

vector<vector<int>> getReferenceSum(vector<vector<int>> a, vector<vector<int>> b) {
    REQUIRE( a.size() == b.size() );
    vector<vector<int>> s = a;
    for (size_t r=0; r<b.size(); r++)
        for (size_t c=0; c<b.size(); c++)
            s[r][c] += b[r][c];
    return s;
}

TEST_CODE( "matrix sum" ) {

    vector<vector<int>> a = ...
    vector<vector<int>> b = ....

    vector<vector<int>> librarySum = superAwesomeFancyFunc(a, b);
    vector<vector<int>> referenceSum = getReferenceSum(a, b);

    REQUIRE( librarySum == referenceSum );
}

This means though that in the ultimately performed unit-test, the checking of pre & post-conditions of the helper functions is included in the statistics of the unit-tests (total number of assertions passed).

Instead, it would be great if there was a seperate "meta" macro, which asserts conditions the test-author believes is tautological, and which when failed (indicating bad unit-testing), stops all testing.

E.g. something like

DEMAND( cond )

Is there currently any way to do this, or at least to seperate the statistics of 'internal' REQUIREs from the 'testing' REQUIREs? Otherwise, is there a way to write custom macros and 'hook' them into Catch's control-flow (rather than just having them force-exit)?

michaelvlach commented 4 years ago

Sounds like a good idea to me. The new keyword however is not necessary imho. It could be something like REQUIRE_INTERNAL. But then again it could be simulated already with plain if and FAIL in case the condition was false... that could be even wrapped in your DEMAND I suppose.

TysonRayJones commented 4 years ago

It's not immediately obvious to me how to do this well myself though. You're right that I can just do:

#define DEMAND( cond ) if (!(cond)) FAIL( ); 

but this has two problems

On another note, do you have any idea why this seemingly more idiomatic way fails?

#define DEMAND( cond ) { \
    CHECKED_ELSE( cond ) { \
        FAIL( ); \
    } \
}

It compiles fine and without any warnings, but when run, stranglely outputs (for tests where cond is always true!):

:4545469632: FAILED:

then freezes.

TysonRayJones commented 3 years ago

Are there any more thoughts on this? It remains exceptionally useful to my use-case