Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using GENERATE macro in a for loop #1913

Closed
Sidelobe opened this issue Apr 16, 2020 · 4 comments
Closed

Using GENERATE macro in a for loop #1913

Sidelobe opened this issue Apr 16, 2020 · 4 comments
Labels

Comments

@Sidelobe
Copy link

Sidelobe commented Apr 16, 2020

When using a simple GENERATE expression inside a 'for' loop, the number of iterations increases dramatically

#include "catch/catch.hpp"

TEST_CASE("TESTTEST")
{
    for (int i=0; i<5; ++i) {
        int d = GENERATE(1, 2);
        REQUIRE(true);
    }
}

This results in 160 assertions (according to the catch console output). I would expect 10.
If I put the GENERATE() outside the for loop, I get 10 assertions.

Is this expected behaviour or a bug?

Using Catch v2.11.3 with clang-1103.0.32.29 on macos.

@horenmar horenmar added the Bug label Apr 16, 2020
@horenmar
Copy link
Member

horenmar commented Apr 16, 2020

That's definitely unintended behaviour. At a glance, what happens is that the for loop creates 5 distinct instances of the generators. Then, after the loop ends, the test is entered again because there are still unexhausted generators, but only the last one of the 5 is advanced. Then, the next time, the second to last one is advanced, and the last one is restarted...

What this boils down to is a 5-bit binary counter, where each number leads to 5 assertions: 5 * 2**5 == 160.

@Sidelobe
Copy link
Author

Sidelobe commented Apr 16, 2020

ah great, thanks for you reply, Martin - I was indeed wondering how the 160 came to be :-)
I guess, this could have a significant performance impact on many generator-based tests out there.

@horenmar
Copy link
Member

horenmar commented Jun 1, 2020

I have a prototype fix for this issue. The problem is that it is kinda brittle, and also that it silently breaks code like this:

TEST_CASE() {
    int i = GENERATE(1, 2); int j = GENERATE(3, 4);
}

because they are both on the same line. I can probably fix that too, but it will take a bit longer.

horenmar added a commit that referenced this issue Jul 22, 2020
* Successive executions of the same `GENERATE` macro (e.g. because
of a for loop) no longer lead to multiple nested generators.
* The same line can now contain multiple `GENERATE` macros without
issues.

Fixes #1913
@pisoir
Copy link

pisoir commented Oct 16, 2024

I have this code:

#include <catch2/catch_test_macros.hpp>
#include <catch2/generators/catch_generators.hpp>
#include <catch2/generators/catch_generators_adapters.hpp>
#include <catch2/generators/catch_generators_random.hpp>

#include <iostream>

TEST_CASE("TESTTEST")
{
    for (int n=0; n<2; ++n) {
        int i = GENERATE(1, 2); 
        int j = GENERATE(3, 4);

        std::cout << i << ":" << j << std::endl;

        REQUIRE(true);
    }
}

I would like to understand why there is 32 assertions and actually quite a lot of e.g. pairs (1,3). I would expect 2*2*2 = 8 assertions. Basically twice the set of {(1,3), (1, 4), (2, 3), (2, 4) } (same when the loop is only till n < 1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants