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

Introduce JUnit Jupiter extension analogous to JUnitRestDocumentation #296

Closed
sbrannen opened this issue Sep 1, 2016 · 15 comments
Closed

Comments

@sbrannen
Copy link
Member

sbrannen commented Sep 1, 2016

As mentioned in the Spring REST docs: How to migrate Rule to JUnit 5 discussion on Stack Overflow, it is possible to migrate existing Spring REST Docs tests based on JUnit 4 to JUnit Jupiter (JUnit 5); however, there is currently no built-in support analogous to the JUnitRestDocumentation TestRule for JUnit 4.

I propose that REST Docs should introduce a JUnit Jupiter extension analogous to the JUnitRestDocumentation TestRule. This extension should implement the BeforeEachCallback and AfterEachCallback APIs from JUnit Jupiter. Alternatively, the extension could implement the BeforeTestExecutionCallback and AfterTestExecutionCallback APIs, depending on the exact needs of Spring REST Docs.

@sbrannen
Copy link
Member Author

sbrannen commented Sep 1, 2016

If Spring REST Docs is not able or willing to provide built-in integration with milestone releases of JUnit Jupiter, another option would be to provide an example implementation in the wiki as an interim solution for early adopters of JUnit 5.

@sbrannen
Copy link
Member Author

sbrannen commented Sep 1, 2016

@wilkinsona, I'd be glad to consult you on the implementation. 😉

@wilkinsona
Copy link
Member

Rather than being able to implement TestRule and pass arguments into the rule via its constructor, the replacement will require a custom extension and annotation with the latter being meta-annotated with @ExtendWith and having an attribute for the outputDirectory. The extension then has to jump through a few hoops to retrieve the attribute. Nothing insurmountable, but this feels like a step backwards from Unit 4 from an implementers perspective.

Arguably, assuming my analysis above is accurate, it's also a step backwards for users. This arrangement will mean that the output directory has to be a literal value and can't be something that's determined at runtime. More generally, it also appears to limit the types that can be passed into an extension to those that are legal for an annotation attribute.

@sbrannen
Copy link
Member Author

sbrannen commented Sep 2, 2016

@wilkinsona,

You rock, man!

Seriously... I could hug you right now.

Now... before you get scared, let me explain my elation. 😉

Thanks to your in-depth analysis and spot-on constructive criticism, I have had an epiphany of sorts!

What if I told you, you'll be able to replace this in JUnit 4 tests:

@Rule
public JUnitRestDocumentation restDocumentation =
        new JUnitRestDocumentation("target/generated-snippets");

... with the following in JUnit Jupiter tests:

@ExtendWith
RestDocumentationExtension restDocumentation =
        new RestDocumentationExtension("target/generated-snippets");

Now, as you correctly deduced, the above is currently not possible in JUnit Jupiter since extensions can presently only be registered on classes or methods.

But... I am going to do my best to ensure that JUnit Jupiter also supports the JUnit 4 registration style for rules declared as fields, even if the implementation style differs.

What would you say to that?

Cheers,

Sam

@sbrannen
Copy link
Member Author

sbrannen commented Sep 2, 2016

FYI: I overhauled and partially repurposed junit-team/junit5#416 to address the concerns raised here.

@otrosien
Copy link

otrosien commented Sep 2, 2016

Just my 2cts regarding this....

From a user's perspective it felt wrong having to (usually repeatedly) specify the output directory within the tests, like it is right now. Why should all of my tests care about this property? To me an annotation value that can be controlled from my (maven/gradle) build would be a better-suited place, making it some kind of shared concern of all my REST tests. After all it's my build tool that picks up these artefacts later on.

  • Maybe we only need to produce documentation artefacts if we're running the test inside the build tool?

@sbrannen
Copy link
Member Author

sbrannen commented Sep 2, 2016

I'm not proposing that RestDocumentationExtension can only be declared as a field. Rather, I think it's a nice option in case the user needs to programmatically determine the output directory.

However, having said that, since Spring REST Docs uses Spring... it's certainly conceivable that the RestDocumentationExtension could also support a custom annotation that accepts the output directory as an expression. Such an expression could be a hard-coded value, a property placeholder like ${rest.docs.output.dir}, or a full-blown SpEL expression.

@wilkinsona
Copy link
Member

From a user's perspective it felt wrong having to (usually repeatedly) specify the output directory within the tests, like it is right now

I, too, dislike the repetition.

making it some kind of shared concern of all my REST tests

That would be ideal. An alternative is to use a base class for all REST tests that defines the rule and the output directory.

After all it's my build tool that picks up these artefacts later on.

True, but the output when a test is run in the IDE needs to be written somewhere. I want that to be the same place as is used when the tests are run via Maven or Gradle.

I've struggled with this for a while, but I have some ideas now on how it can be improved and use a convention-based approach. However, it will still be necessary to allow the convention to be overridden which is where the discussion above will come in useful.

@wilkinsona
Copy link
Member

Thanks, @sbrannen. I'll look forward to my hug the next time I see you.

That looks much better from REST Docs' perspective. The tests will require access to the instance of the extension so would need to declare the field anyway. I think that having the @ExtendWith annotation on the field so that the two related items are right next to each other will makes things much clearer.

@sbrannen
Copy link
Member Author

sbrannen commented Sep 3, 2016

The tests will require access to the instance of the extension so would need to declare the field anyway.

Well, with JUnit Jupiter that's actually no longer a requirement like it is with JUnit 4. The instance of the RestDocumentationContextProvider created by the extension could actually be provided as a parameter to a constructor or a @BeforeAll, @BeforeEach, or @Test method if the extension implements ParameterResolver. So keep that in mind.

Having said that, however, having the RestDocumentationContextProvider instance in a field might result in the cleanest programming model for the end user.

I think that having the @ExtendWith annotation on the field so that the two related items are right next to each other will makes things much clearer.

I agree.

@sbrannen
Copy link
Member Author

sbrannen commented Sep 8, 2016

FYI: support for programmatic extension registration in JUnit Jupiter will now be addressed in junit-team/junit5#497.

@ericjturley
Copy link

Is there a workaround for this currently?
I'm about to start experimenting with an Extension or a TestExecutionListener that will delegate to a ManualRestDocumentation instance.
Because that's where I'm at now... so .... someone stop me.

@wilkinsona
Copy link
Member

An extension that delegates to ManualRestDocumentation is the right approach, IMO. If you look at JUnitRestDocumentation you'll should see that that's exactly what it does too.

@ericjturley
Copy link

Thanks.
I started with a TestExecutionListener. But I didn't see a good way to provide the test case a reference to the ManualRestDocumentation instance from it.

The Extension worked better in that the ExtensionContext store allowed it to keep that reference and then provide it via ParameterResolver.

I've posted it back to the question. If you see that I'm doing something absurd, please point it out.

@wilkinsona
Copy link
Member

@ericjturley. Nothing absurd there at all. Thanks for sharing what you've done. I'm looking forward to junit-team/junit5#497 in JUnit 5 M4 as it'll allow an extension to be used as a field.

@wilkinsona wilkinsona added this to the 2.0.0.M1 milestone Apr 24, 2017
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

4 participants