Thanks for opening the issue! There are some details I need clarifications on because I'm not quite getting what you describe. (Excuse me if these are dumb question. 😃)

It has to store one seed value at the class level, for example if static constants are initialized using our random mechanism. [... much later ...] We note the class seed in a BeforeAllCallback, which is the perfect place.

What exactly do you mean by "note"? Since staticAddress is initialized as part of the class' initialization, the Random instance must also already have been constructed before any BeforeAllCallback is called. How can your extension then access the seed used to create that Random? Is it stored somewhere?

The switch to using the instance seed however, has to be right before the test instance is created

What do you mean with "switch"? Do you create a new Random instance when the first instance field is initialized? If so, how? Or would you want to make that switch but the current extension model does not allow you to?

Another, more general question: Obviously the fields must always be initialized in the same order for the randomization to be reproducible. Does the JLS or JVMLS guarantee an order? Because if not, the entire premise is built on an unspecified implementation detail.

Sorry, that it took me a while to answer!! Thanks for taking the time to look into this 😃 I'm not amazed, that I did not get across clearly, it's not the easiest thing to describe somehow 😉

By "note" I just meant to set and store the current seed somewhere for later (set and store static seed -> static init -> set and store instance seed -> instance init)

The BeforeAllCallback is executed before the class initialization, so it is early enough to set the 'static seed' for the class initialization block. If a BeforeAllCallback sets the static seed, then staticAddress as part of the class init, will be able to use it. In case that is an internal detail of JUnit 5 that may change in time, we might have to introduce some sort of lazy static seed creation on first access (similar to our workaround for the instance seed problem, but hopefully then less shady).

Yes, at the moment right before any instance related initialization first happens (field init, constructor, ...) we want to create a new Random, where the seed is derived from the static Random, so we can make the whole process deterministic. However this is where we are missing an extension point, that is called right before any testClass.newInstance() is called by JUnit.

As far as I understand http://docs.oracle.com/javase/specs/jls/se8/html/jls-12.html#jls-12.5 the order should be deterministic, "in the left-to-right order in which they appear textually in the source code", so theoretically (and empirically so far 😉) this should work.

Thanks for clearing this up, this is an interesting problem. 😃

Do you think, adding the BeforeTestInstantiationCallback would be harmful?

Forgot to answer that... No, not at all. It's of course up to people with more knowledge of the internals (summoning @sormuras) to decide that but I see no conceptual reason why that would be a bad idea.

Still, gnawing on your problem as it is... Did you ever consider injecting Random into setup methods? I created a little proof of concept that injects an instance of Random into setup and test methods and also catches exceptions to print the seed of the failed test. You would then use the setup methods to populate whatever fields you need.

One caveat is that reflection order is not guaranteed, so injecting the same Random instance into multiple setup methods might not be the best idea. The extension could of course check that and emit a warning...

@codecholeric,

The BeforeAllCallback is executed before the class initialization, so it is early enough to set the 'static seed' for the class initialization block. If a BeforeAllCallback sets the static seed, then staticAddress as part of the class init, will be able to use it. In case that is an internal detail of JUnit 5 that may change in time, we might have to introduce some sort of lazy static seed creation on first access (similar to our workaround for the instance seed problem, but hopefully then less shady).

When you say that the "BeforeAllCallback is executed before the class initialization," are you referring to a static initialization block? If so, then I don't see how that could be true since JUnit must have access to a Class object for the test class before invoking any BeforeAllCallback. Hence, the class would already have been loaded in the ClassLoader by that point, and any static initialization blocks would already have been executed.

Please note as well that if a user annotates a test class with @TestInstance(Lifecycle.PER_CLASS) (feature introduced in 5.0 M5)... then JUnit will instantiate the test class before invoking any BeforeAllCallback. 😉

Yes, I was referring to a static initialization block, and I think JUnit 5 at the moment does sth. like Class.forName(testClassName, false, classLoader), because the static block is indeed (at least with my old example using M3) evaluated after the callback. E.g.

public class FooBeforeAll implements BeforeAllCallback {
    @Override
    public void beforeAll(ContainerExtensionContext context) throws Exception {
        System.out.println("BEFORE ALL");
    }
}

@ExtendWith(FooBeforeAll.class)
public class FooTest {
    static {
        System.out.println("STATIC");
    }

    @Test
    void name() {
        System.out.println("TESTS");
    }
}

will print

BEFORE ALL
STATIC
TESTS

But anyway, I'm not convinced, that it is a good idea, to rely on that 😉

So, we would have to add some sort of lazy mechanism for this, too.

I think if I would start from scratch, I would follow @nicolaiparlog 's approach, and kick all this evil static factory code (even though it causes a little more boiler plate to set all those fields, but in my opinion we could just forbid static random fields in tests anyway).

However, the other question is, if it is not a valid request in general, to have an extension point right before a test class is instantiated.

But anyway, I'm not convinced, that it is a good idea, to rely on that 😉

I agree: that's fragile at best.

In your example it's obviously true that static initialization blocks of the test class are not invoked prior to invocation of BeforeAllCallback callbacks. That's because there is no "active use" of the class prior to that.

However, the following (a bit contrived) example demonstrates that that will not always be the case.

@ExtendWith(FooBeforeAll.class)
class FooTest {

	static final String TEST_CLASS_NAME = FooTest.class.getName();

	static {
		System.out.println("STATIC");
	}

	@org.junit.jupiter.api.Test
	void name() {
		System.out.println("TESTS");
	}

}

class FooBeforeAll implements BeforeAllCallback {

	@Override
	public void beforeAll(ExtensionContext context) throws Exception {
		System.out.println("BEFORE ALL: " + FooTest.TEST_CLASS_NAME);
	}

}

If any other class actively uses the test class earlier, the output to the console will be the following.

STATIC
BEFORE ALL: FooTest
TESTS

So, in summary, I would not implement an extension that hopes that the test class has not yet been fully initialized, because as a wise person once said, "Hope is not a strategy!". 😉

Okay, I guess this concrete example makes it even more clear, that relying on the order of static vs beforeAll is a bad idea 😉
I guess the lazy mechanism is the only choice then...

However, the other question is, if it is not a valid request in general, to have an extension point right before a test class is instantiated.

We're still pondering that.

In other words, we're basically waiting to see if the community at large really needs such a feature (i.e., whether it warrants inclusion in the extension API).

Hi again,
with JUnit 5.5, you provided the InvocationInterceptor, which solves our problem elegantly via the interceptTestClassConstructor​()-method.

Good job :-), definitely a much more powerful and general enhancement than our original proposal.

One thing that I wasn't aware of in that context is the fact that the test class may not have been initialized yet (clinit), when the interceptTestClassConstructor​()-method is invoked. It may be worth mentioning that in the javadoc of the method.

@jonashoef, can you please create a new issue (or even a PR) for that?

Otherwise comments like these get lost the backlog.

Thanks

Provided PR

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

This issue has been automatically closed due to inactivity. If you have a good use case for this feature, please feel free to reopen the issue.