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

Create example for TemporaryFolder extension #4

Closed
sormuras opened this issue Feb 23, 2016 · 31 comments
Closed

Create example for TemporaryFolder extension #4

sormuras opened this issue Feb 23, 2016 · 31 comments

Comments

@sormuras
Copy link
Member

What would an implementation outline look like?

Usage:

@ExtendWith(TemporaryFolder.class)
public class TempTest {
// ...
  @Test
  void temp(@TemporaryFolder.Tag TemporaryFolder temp) throws IOException {
    System.out.println(temp.getRoot());
  }
}

Implementation ("copied" from JUnit 4, stripped ExternalResource dependency):

public class TemporaryFolder implements MethodParameterResolver, AfterAllExtensionPoint {

  @Target(ElementType.PARAMETER)
  @Retention(RetentionPolicy.RUNTIME)
  public @interface Tag { }
// ...
  @Override
  public Object resolve(Parameter parameter, MethodInvocationContext mci, ExtensionContext ec)
      throws ParameterResolutionException {
    try {
      create();
    } catch (IOException e) {
      throw new ParameterResolutionException(e.toString());
    }
    return this;
  }

  @Override
  public boolean supports(Parameter parameter, MethodInvocationContext mci, ExtensionContext ec)
      throws ParameterResolutionException {
    return parameter.isAnnotationPresent(Tag.class);
  }

  @Override
  public void afterAll(ContainerExtensionContext arg0) throws Exception {
    delete();
  }
}
@marcphilipp
Copy link
Member

We actually have implemented a version of this for our tests called TempDirectory which is used in XmlReportsWritingListenerTests. It uses the Path API from Java 7 (NIO).

I think we should provide that as an example -- if that suits your needs?

@sormuras
Copy link
Member Author

Perfect match! Thanks for the hint/link.

Will you bundle some common "well-known JUnit 4 rules" into a ported "basic JUnit 5 extensions" package?

@marcphilipp marcphilipp changed the title Create example for TemporayFolder "Rule" Create example for TemporaryFolder "Rule" Feb 29, 2016
@marcphilipp
Copy link
Member

What else besides TemporaryFolder?

@sormuras
Copy link
Member Author

Hm, only StopWatch left, right? TestName is covered by TestInfo...

@marcphilipp
Copy link
Member

I don't think something like StopWatch is possible at the moment. We'd need an additional extension point as discussed in junit-team/junit5#157.

@sbrannen sbrannen changed the title Create example for TemporaryFolder "Rule" Create example for TemporaryFolder extension Feb 29, 2016
@sbrannen
Copy link
Member

It depends on the functionality of the StopWatch that you need.

If you just want to time tests, that is already possible. Check out the example TimingExtension.

Does that suit your needs?

@sormuras
Copy link
Member Author

Ja, that's all I need. Thanks, again.

But not having a collection of default/common extensions shipping along with JUnit 5, seems a bit strange. Feels like "everybody" will write it's own base extensions. Or did almost every feature provided by Rules move to TestInfo & Co?

@marcphilipp
Copy link
Member

Please note that the rules shipped with JUnit 4 were considered examples at first, too. Nevertheless, I think we should consider providing a few basic extensions (like TempDirectory) out of the box.

@junit-team/junit-lambda What do you guys think?

@jlink
Copy link
Contributor

jlink commented Mar 1, 2016

What candidates do we have?

  • TempDirectory
  • Timeout

Not sure if something like Watchman and ExternalRessource make sense.

2016-03-01 8:58 GMT+01:00 Marc Philipp notifications@github.com:

Please note that the rules shipped with JUnit 4 were considered examples
at first, too. Nevertheless, I think we should consider providing a few
basic extensions (like TempDirectory) out of the box.

@junit-team/junit-lambda
https://github.com/orgs/junit-team/teams/junit-lambda What do you guys
think?


Reply to this email directly or view it on GitHub
#4 (comment)
.

@sbrannen
Copy link
Member

We should definitely provide extensions for common use cases (and not just as part of the junit5-samples repository).

The only questions are:

  • Which extensions?
  • Where should they reside? Alongside the JUnit 5 programming API? In a separate junit-extensions module/JAR?

Let's discuss...

@mmerdes
Copy link

mmerdes commented Mar 11, 2016

Depending on the Number of specializations of org.junit.rules.ExternalResource out there
it might make sense to provide a generic adapter ExternalResource-Subclasses.
This would then enable reuse of the existing implementations
which might ease the transition for some people.
Contrary to the general case of TestRule this approach might be feasible.
I would be interested to give it a try unless y'all think such an adaptor would be the wrong way to go.

@jlink
Copy link
Contributor

jlink commented Mar 11, 2016

This adapter would have to live in a new module with junit4 dependency, right?

@mmerdes
Copy link

mmerdes commented Mar 11, 2016

it would definitely need the junit4 dependency.
not sure if it made sense to put it in the junit4-runner
which already has this dependency...

@jlink
Copy link
Contributor

jlink commented Mar 11, 2016

Alternatively we could have an adapter but users have to change the import of their original resource rule.

@sbrannen
Copy link
Member

Providing some form of adapters is definitely worth investigating.

I have heard questions from various users regarding how one would migrate from rules to extensions.

Along these lines, another useful adapter would be one for the ExpectedException rule. The idea here is that users could still program to the ExpectedException API, so to speak, but the implementation would be extension-based. In order to make the use of such an adapter as transparent as possible, we would have to have a class with the same name but in a different (gen5) package (like @jlink said).

@sbrannen
Copy link
Member

@mmerdes, feel free to do your work in conjunction with junit-team/junit5#169 (or create a new issue if you prefer).

@marcphilipp
Copy link
Member

Well, if we had a general Statement-like execution extension point in place we could provide an implementation that supports rules (not @ClassRule) in general.

@sbrannen
Copy link
Member

@marcphilipp, I think we should avoid introducing a Statement-like execution model in JUnit 5 for reasons discussed elsewhere in greater detail.

@mmerdes
Copy link

mmerdes commented Mar 11, 2016

I limited my proposal to create an adaptor to ExternalResource subclasses (and similar cases) because they do not need access to the Statement abstraction.

@mmerdes
Copy link

mmerdes commented Mar 16, 2016

Please take a look at:
https://github.com/junit-team/junit5/blob/issue169-RuleAdapter-experiments/junit-tests/src/test/java/org/junit/gen5/adapter/ExternalResourceLegacySupportTests.java

I did an experiment to support JUnit4 ExternalResource subclasses like TemporaryFolder with a new Extension.
The idea is to minimize the effort (and fear) to migrate to JUnit5. Hence this extension honors the original JUnit4 annotation.
Disadvantage: direct coupling to JUnit4 (obviously)
Advantage: works out of the box, no porting at all

What do you think of this approach?
It is of course limited to TestRule subclasses which don't mess with Statements.
Some TestRule subclasses could be easily ported to be ExternalResource subclasses
achieving compatibility with both JUnit 4 and 5.

(nb: the implementation is still sketchy and there might be a better module for it as well)

@jlink
Copy link
Contributor

jlink commented Mar 16, 2016

Cool! What I'd suggest is to NOT use the original JUnit4 @Rule annotation but a new one like @JUnit4ExternalResourceRule to make it obvious that not all JUnit4 rules can be used.

An alternative approach could be to provide an adapter extension like:

public class ExternalResourceAdapter<T extends ExternalResource> 
            implements BeforeEachExtensionPoint, AfterEachExtensionPoint, MethodParameterResolver {
    T create(Object[] constructorParameters);
}

public class TemporaryFolderExtension extends ExternalResourceAdapter<TemporaryFolder> {
}

Would be a bit more JUnit5-ish but require more effort from rule providers. Don't know if that's feasible, though.

@Siedlerchr
Copy link

Are there any news on including the TemporaryFolder Extension Rule equivalent in junit5 somewhere as official public usable? That would be really great, because we have a lot of tests which depend on this and before we migrate them all, it would be nice to have an official equivalent to this.

@sbrannen
Copy link
Member

@Siedlerchr, we haven't talked about this in a while, so there's no update.

However, now that you've brought it up again...

@junit-team/junit-lambda, let's discuss this during the next team call.

Tentatively slating for 5.1 M2 to put it on the immediate radar, but the implementation (if we decide to do it) would likely come in 5.2.

@sbrannen
Copy link
Member

Oops... just realized this is the "samples" repo. 😮

@sbrannen
Copy link
Member

FYI: I created an issue to address this in JUnit Jupiter...

junit-team/junit5#1247

@mniedero
Copy link

Indeed, it would be great if there is a replacement of JUnit 4 TemporaryFolder within JUnit 5.

Here, I've re-engineered lot of JUnit tests for following reasons:
o) make them platform independent (Win/Linux).
We develop on Windows but Jenkins CI is running on Linux (RHEL 7 like)
o) have a testbed that is celeaned-up/destroyed automatically after success. This is essential on Jenkins CI because I'm interesseted to minimize time needed for maintenance as much as possible.
o) lot of my tests are like integration test: They read and write files. But, writing files into folder structure containing source code is strictly forbidden.

Well, I can write my own TemporaryFolder code like ten years (or more) ago. Since a Temporaryfolder rule is available with JUnit 4 this is no longer feasible.
For the moment, missing TemporaryFolder Extension is biggest hurdle for migration from Junit 4 to JUnit 5. No, it has become a no-go.

@autonomousapps
Copy link

I have converted the old code presented above into something that works and is written in Kotlin, for anyone, like me, looking for this:

/**
 * Replacement for JUnit4 `@Rule TemporaryFolder`.
 */
class TempDirectory : AfterEachCallback, ParameterResolver {

    override fun supportsParameter(parameterContext: ParameterContext, extensionContext: ExtensionContext): Boolean {
        return parameterContext.parameter.getAnnotation(Root::class.java) != null &&
            Path::class.java == parameterContext.parameter.type
    }

    override fun resolveParameter(parameterContext: ParameterContext, context: ExtensionContext): Any {
        return getLocalStore(context).getOrComputeIfAbsent<String, Any>(KEY) { _ -> createTempDirectory(context) }
    }

    override fun afterEach(context: ExtensionContext) {
        val tempDirectory = getLocalStore(context).get(KEY) as? Path
        if (tempDirectory != null) {
            delete(tempDirectory)
        }
    }

    private fun getLocalStore(context: ExtensionContext): ExtensionContext.Store {
        return context.getStore(localNamespace(context))
    }

    private fun localNamespace(context: ExtensionContext): ExtensionContext.Namespace {
        return ExtensionContext.Namespace.create(TempDirectory::class.java, context)
    }

    private fun createTempDirectory(context: ExtensionContext): Path {
        try {
            return Files.createTempDirectory(context.displayName)
        } catch (e: IOException) {
            throw ParameterResolutionException("Could not create temp directory", e)
        }

    }

    private fun delete(tempDirectory: Path) {
        Files.walkFileTree(tempDirectory, object : SimpleFileVisitor<Path>() {

            override fun visitFile(file: Path?, attrs: BasicFileAttributes?): FileVisitResult {
                return deleteAndContinue(file)
            }

            override fun postVisitDirectory(dir: Path?, exc: IOException?): FileVisitResult {
                return deleteAndContinue(dir)
            }

            private fun deleteAndContinue(path: Path?): FileVisitResult {
                Files.delete(path)
                return FileVisitResult.CONTINUE
            }
        })
    }

    companion object {
        private const val KEY = "tempDirectory"
    }
}

@Target(AnnotationTarget.VALUE_PARAMETER)
@Retention(AnnotationRetention.RUNTIME)
@MustBeDocumented
annotation class Root

At least, it seems to work (compiles and tests run). If I've made a mistake, I'd be happy to be corrected!

@jbduncan
Copy link
Contributor

@autonomousapps Good news: I understand that @marcphilipp is introducing a TempDirectory extension in JUnit Pioneer (the semi-official JUnit 5 extensions pack), see junit-pioneer/junit-pioneer#69.

Thus your Kotlin example should not be strictly needed very soon. :)

@autonomousapps
Copy link

Great! Good thing it only took me a few minutes to adapt from the earlier sample :) And I'm always happy to learn new things.

@marcphilipp
Copy link
Member

In fact I think we should close this issue in favor of junit-pioneer/junit-pioneer#69. Thanks for the reminder! 🙂

@sormuras
Copy link
Member Author

It only took you (now: us) two years and a couple of month!

🥂

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

No branches or pull requests

9 participants