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

Conditional extension registration via @ExtendWith #1242

Closed
Lubetkin opened this issue Jan 14, 2018 · 23 comments
Closed

Conditional extension registration via @ExtendWith #1242

Lubetkin opened this issue Jan 14, 2018 · 23 comments

Comments

@Lubetkin
Copy link

Lubetkin commented Jan 14, 2018

I have a use case where I have a custom annotation i.e. @MyAnnotation that is annotated by @ExtendWith(Extension.class).

On a test method, I declare @MyAnnotation and another annotation, @OtherAnnotation.

@MyAnnotation
@OtherAnnotation
void test(){...}

I want to be able to deactivate the extension registered in MyAnnotation (Extension.class) when the second annotation, OtherAnnotation is present, because JUnit Jupiter takes the extension from MyAnnotation and runs it, but I want that MyAnnotation could be used as a self contained annotation in other test cases.

Is there a way to tell if OtherAnnotation is present and not run the Extension.class in MyAnnotation?

@sormuras
Copy link
Member

sormuras commented Jan 14, 2018

Implement a check in MyAnnotation that tests for the presence of OtherAnnotation (on the same annotatable element) and make MyAnnotation perform a "no operation"?

Use ExtensionContext.getElement() to get a handle of the annotated element.

@Lubetkin
Copy link
Author

I want it as an infrastructure for all annotations that being set with MyAnnotation. that was a simple example. Are you saying that each annotation need to implement this "no operation" code?

@marcphilipp
Copy link
Member

Can you provide some more details on your use case? To me, having an annotation change its meaning when another annotation is present sounds confusing.

@Lubetkin
Copy link
Author

Lubetkin commented Jan 14, 2018

@marcphilipp

@myannotationA
@otherannotation
void test1(){...}

@myannotationB
@otherannotation
void test2(){...}

@myannotationA
void test3(){...}

@myannotationB
void test4(){...}

test1 will ignore myannotationA extensions registered in ExtendWith because otherAnnotation is executing those extensions.

test2 the same as test1, will ignore myannotationB extensions registered in ExtendWith because otherAnnotation is executing those extensions.

test3 and test4 will execute the extensions from the above annotations because otherAnnotation is missing.

I want to be able to do it in an infrastructure level and not like was proposed here that every annotation logic will check if otherAnnotation exists or not.

@sbrannen
Copy link
Member

Is there a way to tell if OtherAnnotation is present and not run the Extension.class in MyAnnotation?

No, there is no built-in mechanism for doing something like this.

In other words, JUnit Jupiter does not have support for "conditional extension registration" -- which is how I would phrase your request.

Rather, as @sormuras pointed out, you would have to implement the conditional logic within your extension implementations, effectively performing a no-op if your "higher ranking" annotation is also present.

@sbrannen
Copy link
Member

On a test method, I declare @MyAnnotation and another annotation, @OtherAnnotation.

Why?

Why would you expect users to be able to (or want to) declare @MyAnnotation and @OtherAnnotation simultaneously if @OtherAnnotation always overrides @MyAnnotation?

Why not just instruct users to declare either @MyAnnotation or @OtherAnnotation?

If you truly need conditional configuration, why not just have a single annotation that accepts configuration values (i.e., via annotation attributes) and then have a single extension that does the right thing according to the user's configuration in that single annotation?

@sbrannen sbrannen changed the title Ignore ExtendWith annotation Conditional extension registration via @ExtendWith Jan 14, 2018
@sbrannen
Copy link
Member

FYI: I reworded the title of this issue to reflect the scope of the discussion.

@Lubetkin
Copy link
Author

@sbrannen
Because the OtherAnnotation is a common annotation that is being used by other test modules. And MyAnnotation is per test module.
I have an option to pass a list of extension to that annotation but want to minimize the boilerplate of that list by abstracting it in another annotation and also for reusable annotation to be used without the common one.

@sbrannen
Copy link
Member

Hmmmm.... the discussion here is a bit too abstract for me.

Can you perhaps shed some light on the actual use case you have?

That would make it easier for us to understand and perhaps easier for us to make an alternative proposal.

@Lubetkin
Copy link
Author

I want to pass to the extension state in OtherAnnotation a list of extension that are stated in MyAnnotation to execute as well.
the extension state in OtherAnnotation is a test template and I need the ability to add those extensions to the test template as an additional extensions.
And the ability to use those extensions state in MyAnnotation as self contained annotation as well.

@sormuras
Copy link
Member

You could share some common state via the global store:

@marcphilipp
Copy link
Member

Extensions registered on test template methods will already be registered for each invocation. There's no need to add them to the test template invocation context as additional extensions.

@Lubetkin Am I missing something?

@marcphilipp
Copy link
Member

Closing due to lack of activity. Please reopen if the issue persists.

@Adam-Szczeblewski-EPAM
Copy link

Adam-Szczeblewski-EPAM commented Jan 19, 2021

@marcphilipp

I came across this looking for "conditional extension registration", which you mentioned is not supported. Maybe we could turn it into a feature request.

Here's my use case. Please note use of nonexisting @RegisterExtensionIfSystemProperty, consider it a solution proposal.
Hope the below code is self-explanatory, but will be happy to answer questions of course.
I would like to express the following (although it doesn't even compile, please consider as a high-level idea):

//my public API, implemented by my Spring RestController. I can also create a feign client with it, for remote access.
interface TodoerApi {
	@PostMapping("/{user}/todo")
	IdResponse createTodo(@PathVariable String user, @RequestBody NewTodoRequest req);

	@GetMapping("/{user}/todo")
	List<Todo> getTodos(@PathVariable String user);

	@DeleteMapping("/{user}/todo/{id}")
	void markDone(@PathVariable String user, @PathVariable long id);
}

// ----

@BlackBoxTest
class AppTests {
	@Inject TodoerApi api; 
	
	//tests omitted
}

// ----

@RegisterExtensionIfSystemProperty("integration-test-strategy", matches="ZERO_IO", extension=SpringBootTestWithoutTomcat)
@RegisterExtensionIfSystemProperty("integration-test-strategy", matches="IN_PROCESS", extension=SpringBootTestWithFeignClient)
@RegisterExtensionIfSystemProperty("integration-test-strategy", matches="DOCKER_IMAGE", extension=DockerImageStarter)
@interface BlcakBoxTest {}

@SpringBootTest(webEnvironment = NONE) // in this configuration, my RestController will be injected into AppTests.api, making tests very fast
@interface SpringBootTestWithoutTomcat {} 

@SpringBootTest(webEnvironment = RANDOM_PORT)
@Import(SpringConfigWithFeign.class) 	// in this configuration, a feign client is created as a primary bean of TodoerApi type and injected into AppTests.api
@interface SpringBootTestWithFeignClient {} 

@ExtendWith(DockerImageStarterExtension.class) //my custom extension which starts a pre-built docker image with my app and injects preconfigureed feign client into AppTests.api 
@interface DockerImageStarter {}

Then I can create various build pipelines using mvn -Dintegration-test-strategy=XXX test

Given that I cannot implement @BlackBoxTest annotation, I'm currently falling back on the below approach.
But it gives me warnings on IGNORED TESTS in the build process (only one of Unit/Integreation/Blackbox is executing), which I don;t like:

abstract class AppTests {
	@EnabledIfSystemProperty(named = "integration-test-strategy", matches = "ZERO_IO") 
	@SpringBootTestWithoutTomcat 
	static class UnitTests extends AppTests {}
	
	@EnabledIfSystemProperty(named = "integration-test-strategy", matches = "IN_PROCESS") 
	@SpringBootTestWithFeignClient 
	static class IntegrationTests extends AppTests {}
	
	@EnabledIfSystemProperty(named = "integration-test-strategy", matches = "DOCKER_IMAGE") 
	@DockerImageStarter 
	static class BlackBoxTests extends AppTests {}

	@Inject TodoerApi api; 
	
	//tests omitted
}

Maybe this would fuel adding "conditional extension registration" capability to the platform?

@sbrannen
Copy link
Member

@Adam-Szczeblewski-EPAM, your proposal unfortunately would not work.

SpringBootTestWithoutTomcat, SpringBootTestWithFeignClient, and DockerImageStarter are composed annotations and therefore cannot be registered as extensions with JUnit Jupiter.

Thus, even if we did add support for an annotation like @RegisterExtensionIfSystemProperty, such a feature would not help with your use case.

Each of the test classes in your use case relies on a single extension from an external project (namely the SpringExtension), and the external projects (i.e., Spring Framework and Spring Boot) react to the presence of annotations present on the test class. Specifying Spring annotations on a composed annotation that is referenced via in attribute in the proposed @RegisterExtensionIfSystemProperty would make those annotations invisible to Spring Framework and Spring Boot.

Thus, your current approach is likely the best approach to achieve your goal.

@Adam-Szczeblewski-EPAM
Copy link

Adam-Szczeblewski-EPAM commented Jan 19, 2021

@sbrannen , thanks for your input!

I'm also thinking to try out something like:

@ExtendWithIfSystemProperty(named = "integration-test-strategy", matches="ZERO_IO", extension=MySpringExtension.class) 
@interface BlackBoxTest {}

where MySpringExtension would be my subclass extending SpringExtension - which I hopefully can programatically force to use webEnvironment of my choice. That way I would not depend on composed annotations.

I know, I'll also need to take care of SpringBootTestContextBootstrapper and 10000 other things (defeating the purpose, which is to simplify the test infrastructure code), but I'm curious of your take on feasibility of this.

@sbrannen
Copy link
Member

My gut feeling is that that approach would not be feasible.

Your @BlackBoxTest annotation would result in your MySpringExtension being conditionally registered with JUnit Jupiter, but it would not prevent the SpringExtension from being registered by various Spring Boot annotations for test slices (e.g., @WebMvcTest). So it would be impossible to use Boot's test slices with such an approach.

In addition, the SpringExtension is effectively just a thin integration layer with JUnit Jupiter that delegates to Spring's TestContextManager which further delegates to multiple internal components within Spring Test and Spring Boot Test. So just subclassing SpringExtension would not get you very far in terms of addressing those 10,000 other things you alluded to.

@Adam-Szczeblewski-EPAM
Copy link

Adam-Szczeblewski-EPAM commented Jan 26, 2021

Well, the intention is for BlackBox test to be mutually exclusive with test slices - blackbox test should not claim having access to any internals of the app, is might be as well executing against app running in a separate process (e.g. docker image).

Nevertheless, I agree that subclassing SpringExtension is too much fuss, and not worth the effort.

BTW, for anyone who gets here trying to solve a similar problem: I ended up with using junit categories (@Tag) instead of @EnabledIfSystemProperty. All inner static IntegrationTest classes are tagged with @Tag("integration"), so I can have CI/CD pipelines like so:

first: compile and run unit tests (note the exclamation mark):
mvn clean package -Dgroups=!integration
later: run integration tests only:
mvn test -Dgroups=integration

@sbrannen
Copy link
Member

@Adam-Szczeblewski-EPAM ,

Tagging is also a good solution for that problem. Glad you sorted it out!

@pjkvyezz
Copy link

sorry to raise this old issue but I have another use case:

I have an extension that launches BrowserStack Local (a binary that allows you to test private sites on browser stack). I only want this extension to load if the the system property browserStackEnabled is true.

what's a good way to do this if not via conditional loading of this extension?

@sbrannen
Copy link
Member

what's a good way to do this if not via conditional loading of this extension?

You could register the extension via @RegisterExtension on a field, and the field assignment could use conditional logic to check the system property and then assign the real extension or a dummy (no-op) extension.

@pjkvyezz
Copy link

pjkvyezz commented Aug 20, 2021

thanks @sbrannen . for anyone else that comes across this the snippets below explain how to do it.

//BaseTestWatcher.java
package com.foo;

import org.junit.jupiter.api.extension.TestWatcher;

/**
 * This class is necessary because JUnit 5 does not allow conditional registration of extensions.
 * This class should not do anything and is essentially a dummy extension.
 */
public class BaseTestWatcher implements TestWatcher {
}
// BrowserStackTestWatcher.java
package com.foo;

import org.junit.jupiter.api.extension.TestWatcher;

public class BrowserStackTestWatcher extends BaseTestWatcher {
  // you extension logic
}
package com.foo;

public class BaseTest {

  @RegisterExtension
  BaseTestWatcher testWatcher;

  public BaseTest() {
        if (PropertyManager.getPropertyAsBoolean("browserstack.enabled")) {
            // only register the real extension when we want to
            testWatcher = new BrowserStackTestWatcher(); 
        } else {
             // Dummy extension is registered here.
            testWatcher = new BaseTestWatcher(); 
        }
  }
} 

@sbrannen
Copy link
Member

sbrannen commented Aug 26, 2021

thanks @sbrannen .

You're very welcome, and I'm glad that helped you!

for anyone else that comes across this the snippets below explain how to do it.

I was actually thinking about doing it as follows without the need for a constructor.

public class BaseTest {

	@RegisterExtension
	Extension testWatcher = Boolean.getBoolean("browserstack.enabled") ?
				new BrowserStackTestWatcher() :
				new Extension() {};

} 

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

No branches or pull requests

6 participants