Add ExclusiveResource programmatically

[U1F984]

I have several test classes, each with multiple test methods inside. Some of those classes are injected with a parameter, and when running classes in parallel those classes should not run parallel. If I understand correctly, the @ResourceLock annotation can help here, but it would be very inconvenient to manually add that annotation to hundereds of tests. Ideally a extension could take a look at the class, see that it has this parameter and somehow add the ExclusiveResource to it.

My JUnit execution parameters:

-Djunit.jupiter.execution.parallel.enabled=true
-Djunit.jupiter.execution.parallel.mode.default=same_thread
-Djunit.jupiter.execution.parallel.mode.classes.default=concurrent
-Djunit.jupiter.execution.parallel.config.strategy=fixed
-Djunit.jupiter.execution.parallel.config.fixed.parallelism=8

[marcphilipp]

We could add a new extension point for this:

interface ExclusiveResourceProvider extends Extension {
	default Set<ExclusiveResource> provideExclusiveResourcesForClass(Class<?> testClass, Set<ExclusiveResource> declaredResources) {
		return declaredResources;
	};
	default Set<ExclusiveResource> provideExclusiveResourcesForNestedClass(Class<?> nestedClass, Set<ExclusiveResource> declaredResources) {
		return declaredResources;
	};
	default Set<ExclusiveResource> provideExclusiveResourcesForMethod(Class<?> testClass, Method testMethod, Set<ExclusiveResource> declaredResources) {
		return declaredResources;
	};
}
interface ExclusiveResource {
	String getKey();
	ResourceAccessMode getAccessMode();
	static ExclusiveResource of(String key);
	static ExclusiveResource of(String key, ResourceAccessMode accessMode);
}

[jbduncan]

This feature might be quite useful for my PR on junit-pioneer to allow "resources" - arbitrary objects - to be instantiated and shared amongst tests declaratively.

Specifically, if each "resource" were to have its own ResourceLock that could be created programmatically, then it would simplify making my extension thread-safe.

The proposed API leaves me with one question, though: if my extension were to implement ResourceLockProvider, when would the provideX methods be called?

[marcphilipp]

It would have to be called immediately before starting execution on the engine level.

[jbduncan]

Okay, thank you!

I wasn't sure at first if this would work for me, since my extension's resources are created at test runtime. But I think it could work now, as all shared resources in my extension are declared with @Shared annotations, which I imagine my potential ResourceLockProvider could search for reflectively.

So I'm happy with the proposed API. 👍

[marcphilipp]

Team decision: Let's go with the above proposal.

@U1F984 @jbduncan Is one of you interested in working on this?

[jbduncan]

@marcphilipp I'd love to! However I still have junit-pioneer/junit-pioneer#348 to finish, so I'm happy for anyone else to pick this up before me.

[jbduncan]

For clarity, my JUnit Pioneer work is not actually blocked on this feature, as I've found an alternative solution to programmatic ResourceLocks for my needs.

[jbduncan]

@marcphilipp A follow-up query from me: do you know if JUnit Jupiter already ensures that ResourceLocks are obtained in such an order as to prevent the dining philosophers problem, by any chance?

If not, then I, or whoever picks this up, will need to ensure the locks are sorted when being moved from ResourceLockProviders to the rest of the test framework, by some definition of "sorted".

[marcphilipp]

They are sorted here:

junit5/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/LockManager.java

Line 61 in 6a47ae1

.sorted(COMPARATOR)

[jbduncan]

@marcphilipp Great, thank you for answering so quickly. 👍

[jbduncan]

@marcphilipp I'd love it if you assigned this to me, as I've managed to park my junit-pioneer work and find time to start looking at this.

[marcphilipp]

@jbduncan Sorry it took 6 days, but you're now assigned.

[jbduncan]

Sorry for the silence on this! I was a bit lost looking through the JUnit 5 codebase when I looked at this issue last year. But now that my JUnit Pioneer PR that would have used this feature is finally done, I'll see if I can find the time to revisit this issue later this week, and write down anything I need help with. :)

[jbduncan]

@marcphilipp I have a few follow-up questions which I was hoping you could help me with.

For the proposed ExclusiveResourceProvider,

1. Does a provide* method's declaredResources contain just the resources found so far for the given class/method/nested class, or does it contain all resources found so far by JUnit?

2. Do the declaredResources contain resources found through @ResourceLocks?

   @Test
   @ResourceLock(value = "a")
   @ExtendWith(MyExclusiveResourceProvider.class)
   void test() {
   }

3. If so, does the ordering matter? As in, does MyExclusiveResourceProvider's declared resources contain resource "a" when running with test2, but not with test1?

   @Test
   @ExtendWith(MyExclusiveResourceProvider.class)
   @ResourceLock(value = "a")
   void test1() {
   }
   
   @Test
   @ResourceLock(value = "a")
   @ExtendWith(MyExclusiveResourceProvider.class)
   void test2() {
   }
   
   class MyExclusiveResourceProvider implements ExclusiveResourceProvider {
       @Override public Set<ExclusiveResource> provideExclusiveResourcesForMethod(Class<?> testClass, Method testMethod, Set<ExclusiveResource> declaredResources) {
           return declaredResources;
       }
   }

[jbduncan]

I've pushed my ongoing spike to #3096 for the sake of transparency. :)

[jbduncan]

@marcphilipp I'm feeling a bit stuck now, and I was wondering if you could help me.

It looks like NodeTreeWalker is responsible for discovering tests and, most importantly, their @ResourceLocks. However, to get resource locks from ExclusiveResourceProviders, JUnit would need to discover them as extensions, which are currently discovered by NodeTestTasks, which in turn depend on the NodeTreeWalker! So there's a cycle of dependencies (maybe even a web), and I can't see how to break it, so I was was wondering if you had any thoughts?

Alternatively, would you be up to discussing this remotely some time? If so, feel free to contact me on my GitHub email address.

[jbduncan]

@marcphilipp I don't suppose you've had the time to look at my last comment recently, have you? 🙂

[marcphilipp]

@jbduncan Sorry, I had fallen quite behind in reading my GitHub notifications. 😬

It looks like NodeTreeWalker is responsible for discovering tests and, most importantly, their @ResourceLocks. However, to get resource locks from ExclusiveResourceProviders, JUnit would need to discover them as extensions, which are currently discovered by NodeTestTasks, which in turn depend on the NodeTreeWalker!

I'm not sure I fully understand the question. ExclusiveResourceProvider can't be a regular extension since it needs to be applied at discovery time, i.e. it should not extend Extension. We already have a few of this kind of extension, e.g. DisplayNameGenerator and MethodOrderer. So I think we'll need a new annotation that goes along with ExclusiveResourceProvider, maybe @ExclusiveResources(MyExclusiveResourceProvider.class)?

[jbduncan]

@jbduncan Sorry, I had fallen quite behind in reading my GitHub notifications. 😬

No worries! Likewise, sorry for not responding immediately. :)

I'm not sure I fully understand the question. ExclusiveResourceProvider can't be a regular extension since it needs to be applied at discovery time, i.e. it should not extend Extension. We already have a few of this kind of extension, e.g. DisplayNameGenerator and MethodOrderer. So I think we'll need a new annotation that goes along with ExclusiveResourceProvider, maybe @ExclusiveResources(MyExclusiveResourceProvider.class)?

@marcphilipp Oh, I hadn't considered that. That sounds like it could break the dependency cycle.

But admittedly I've lost interest in making programmatic exclusive resources a reality, so I'll leave this issue and my draft PR for someone else who in interested in picking things up.

[marcphilipp]

No worries. Thanks for letting us know!