`InstancioExtension`: Wrong Seed printed when using a custom seed by method or `@WithSettings` annotation about instancio HOT 7 CLOSED

@MatthiasOs ok, I think your suggestion makes sense. I updated how the seed gets reported in #948 (the PR description has a few examples of tests and failure messages). Hopefully this makes it less confusing to users.

Regarding logging the seed for every object that was generated, I will need to think about it some more and try it out to see if it makes sense to add.

from instancio.

Nice, thanks!

from instancio.

@MatthiasOs Thanks for the sample project! InstancioExtension only reports the seed that was generated internally for a given test method.

Using the annotation @Seed(123) sets an explicit test method seed 123, so the extension would also report 123.

Seeds specified manually via withSeed() or Settings are not reported since they are already known to the user.

Consider this example:

@Test
void example() {
    String s1 = Instancio.create(String.class);
    String s2 = Instancio.create(String.class);
    String s3 = Instancio.of(String.class).withSeed(111).create();
    String s4 = Instancio.of(String.class).withSeed(222).create();
    // ... then some test failure
}

When this test fails, the message might be:

Test method 'example' failed with seed: 4344810661868793309

The reported seed is the one we need to reproduce the value of s1 and s2. Using this seed we can reproduce all the objects created within the test method. There's no need to report s3 and s4 seeds since the seed is hardcoded and those are effectively static strings.

Same applies to seeds provided manually using Settings.

The docs have some info on this. I'll try to include some more to make it a bit clearer.

https://www.instancio.org/user-guide/#reproducing-failed-tests

from instancio.

Hey @armandino thanks for your reply.
I see your reasoning concerning the seed method when creating.

But I find it missleading, that when using the Seed annotation, the seed is corretly printed, but not when using the WithSettings annotation.

Moreover the output is incorrect, when using them both at the same time.
Consider the following:
https://github.com/MatthiasOs/Instancio/blob/WrongSeedPrintedWhenUsingMethod/src/test/java/BothAnnotationsTest.java

    @WithSettings
    private final Settings settings = Settings.create()
                                              .set(Keys.SEED, 123L);

    @Seed(Constants.seed)
    @Test
    void shouldPrintCorrectSeedWithBothAnnotation() {
        Integer integer = Instancio.of(Integer.class).create();
        Assertions.assertThat(integer).isEqualTo(Constants.integer_from_seed);
    }

Test fails, because the seed used to create the Integer is 123L from WithSettings, but the output printed is:
timestamp = 2024-03-02T15:13:30.614264200, Instancio = Test method 'shouldPrintCorrectSeedWithAnnotation' failed with seed: 3003117528796338262
Maybe the output printed is even correct, but the wrong seed is used to generate the Integer, since the Scope of Seed is narrower than WithSettings?

from instancio.

Maybe the output printed is even correct, but the wrong seed is used to generate the Integer, since the Scope of Seed is narrower than WithSettings?

It's the opposite :) @WithSettings has higher precedence than @Seed. I updated the user guide in the linked PR #942 but haven't published the changes yet:

I guess it might seem confusing why @WithSettings seed wins over the test method @Seed. The reason for this is that the @WithSettings instance is applied to all the objects created within the test class. In other words, this setup:

@WithSettings
private final Settings settings = Settings.create()
                                              .set(Keys.SEED, 123L);

@Seed(Constants.seed)
@Test
void shouldPrintCorrectSeedWithBothAnnotation() {
    Integer integer = Instancio.of(Integer.class).create();

    // incorrect assertion: the seed used to create the integer is 123
    //assertThat(integer).isEqualTo(Constants.integer_from_seed);
}

is effectively the same as this setup:

@Seed(Constants.seed)
@Test
void shouldPrintCorrectSeedWithBothAnnotation() {
    Integer integer = Instancio.of(Integer.class)
            .withSettings(Settings.create().set(Keys.SEED, 123L))
            .create();

    // same as above
}

If you need to verify the seed per object, you can also use the asResult() method:

@WithSettings
private final Settings settings = Settings.create()
                                              .set(Keys.SEED, 123L);

@Seed(Constants.seed)
@Test
void checkObjectSeed() {
    Result<Integer> result = Instancio.of(Integer.class).asResult();

    long seed = result.getSeed(); // seed that was used to generate the object
    Integer integer = result.get(); // generated value
    //...
}

Hopefully this clarifies things. If you have any suggestions for improving the documentation please let me know.

from instancio.

Hey @armandino ,

ok that clears it up.

But I still think the output seed should be the one actually used. For the WithSettings Annotation the fix to display the correct seed should be minimal:
In ExtensionSupport#processWithSettingsAnnotation the Settings are already present, the only thing left is to check if Keys.SEED is present and if it is, set the threadLocalRandom with the seed from settings. (The order of the two process methods needs to be change to represent, that the seed from WithSettings is used over the Seed annotation)
Maybe I could create a pullrequest for it?

Some thoughts: Maybe it would be better to just log the seed, when it acutally used to generate the data (configurable with a properties) and not after the test failed. Then the seed would always be the correct one.

from instancio.

Some thoughts: Maybe it would be better to just log the seed, when it acutally used to generate the data (configurable with a properties) and not after the test failed. Then the seed would always be the correct one.

+1 on this, it would eliminate the issue completely

from instancio.