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

WIP: 343 - Document compatibility with JUnit 4 TestRules #660

Conversation

smoyer64
Copy link
Contributor

@smoyer64 smoyer64 commented Feb 17, 2017

Overview

This PR includes the tasks required to complete issue #343 with the same title as this PR.

Deliverables

  • Add section introduction paragraph(s)
  • DisableOnDebug Not supported for M4 (or the foreseeable future)
    • Add text to users guide
  • ExpectedException
    • Create jupiter demo
    • Create vintage demo
    • Add text and examples to users guide
  • ExternalResource
    • TemporaryFolder
      • Create jupiter demo
      • Create vintage demo
      • Add text and examples to users guide
  • RuleChain
    • Create jupiter demo
    • Create vintage demo
    • Add text and examples to users guide
  • Stopwatch
    • Create jupiter demo
    • Produce jupiter output
    • Create vintage demo
    • Produce vintage output
    • Add text and examples to users guide
  • TestWatcher
    • TestName
      • Create jupiter demo
      • Create vintage demo
      • Add text and examples to users guide
  • Timeout
    • Create jupiter demo
    • Create vintage demo
    • Add text and examples to users guide
  • Verifier

I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

- Adds output for the StopwatchVintageDemo
- Adds output for the StopwatchJupiterDemo
- Fixes the method signature for Jupiter's succeeding test
- Adds an introduction paragraph for the section
- Adds the DisableOnDebug section text
- Adds the Stopwatch section text
@smoyer64
Copy link
Contributor Author

Please review the (first) section related to replacing the Stopwatch rule and comment. If it's acceptable I'll focus on writing the remainder of these sections as I'd like to finish for inclusion in M4.

I'm also at a loss for why Travis failed this time - any direction would be appreciated!

@gaganis
Copy link
Contributor

gaganis commented Mar 23, 2017

@smoyer64 This looks like a network availability problem with maven repos:

* What went wrong:
Could not resolve all dependencies for configuration ':junit-platform-surefire-provider:compileClasspath'.
> Could not resolve org.apache.maven.surefire:surefire-api:2.19.1.
  Required by:
      project :junit-platform-surefire-provider
      project :junit-platform-surefire-provider > org.apache.maven.surefire:common-java5:2.19.1
   > Could not resolve org.apache.maven.surefire:surefire-api:2.19.1.
      > Could not get resource 'https://repo1.maven.org/maven2/org/apache/maven/surefire/surefire-api/2.19.1/surefire-api-2.19.1.pom'.
         > Could not HEAD 'https://repo1.maven.org/maven2/org/apache/maven/surefire/surefire-api/2.19.1/surefire-api-2.19.1.pom'.
            > Connect to repo1.maven.org:443 [repo1.maven.org/151.101.44.209] failed: Connection timed out (Connection timed out)

This would warrant a rerun of the travis check, though I am not sure if or how this is possible.

@smoyer64
Copy link
Contributor Author

@gaganis - That's what it looked like to me too but I'm not very familiar with Travis or Gradle. I can obviously trigger a rebuild by making a junk commit (which will get squashed with everything else once I'm done with this PR) but I don't know another way either. Thanks for the second set of eyes!

@marcphilipp
Copy link
Member

I've retriggered the Travis build and is back to green now.


public class StopwatchVintageDemo {

private static final Logger logger = Logger.getLogger("");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use Logger.getLogger(StopwatchVintageDemo.class.getName())?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code in the vintage demonstrations was copied directly from the JUnit 4 javadocs - in this case it's at: http://junit.org/junit4/javadoc/latest/org/junit/rules/Stopwatch.html. I made changes to the JUnit 4 sample code when it wouldn't run as shown, and in this case combined both the examples in the javadocs into one file. Do you think it's worth the effort to turn the vintage examples into production-ready code? I have to admit that I completely missed that the code was logging to the root logger ... and it does feel a bit dirty!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't aware that the code originated form Vintage Javadoc. However, I don't want to promote bad development practices, even though it's just example code.

@jbduncan
Copy link
Contributor

@smoyer64 I've noticed that there's no visible work done on demos for TemporaryFolder yet.

Do you have any plans regarding that section? Can you give me a gist of what you intend to write up for the TemporaryFolder jupiter demo? I'm very curious to know what it would look like (mainly because I cannot think of a solution myself, except for using jimfs)!

@smoyer64
Copy link
Contributor Author

I'll try to get the ExternalFolder example committed this weekend - I was basicially going to wrap the JUnit 4 code's folder creation in a JUnit 5 extension as the first implementation. I'm sure that in the long term there will be other needs! See https://github.com/junit-team/junit4/blob/master/src/main/java/org/junit/rules/TemporaryFolder.java.

@marcphilipp
Copy link
Member

Here's a little extension we're using internally for our tests:
https://github.com/junit-team/junit5/blob/master/platform-tests/src/test/java/org/junit/jupiter/extensions/TempDirectory.java

Maybe it could make its way into Pioneer?

@smoyer64
Copy link
Contributor Author

@marcphilipp - We definitely want an extension that provides an upgrade path from JUnit 4's TemporaryFolder in Pioneer. Looking at TempDirectory, maybe it just needs some of the recursion provided in the JUnit 4 rule? What's everyone else's opinion?

@jbduncan
Copy link
Contributor

jbduncan commented May 18, 2017

I don't know about recursion, but I would encourage TempDirectory to have options to use an in-memory filesystem (think Jimfs) and the Path and java.nio.file.Files API.

@sormuras
Copy link
Member

sormuras commented May 18, 2017

[...] in-memory filesystem (think Jimfs)

A TempDirectory feature hosted in JUnit Jupiter should not introduce (shade) another dependency. An user should either redirect "$TEMP" to an in-memory drive (ramdisk) or provide one on-the-fly with Jimfs or others tools.

@marcphilipp
Copy link
Member

What kind of recursion are you referring to?

@jbduncan
Copy link
Contributor

@sormuras That makes sense. I think allowing a user to set a custom java.nio.file.FileSystem (which Jimfs implements) would be the way to go.

@smoyer64
Copy link
Contributor Author

private boolean recursiveDelete(File file) takes care of deleting a hierarchy of files and folders rooted at the folder originally created by the TemporaryFolder rule. When I read through that file I get the impression that code was added for edge cases when things didn't quite work the way they were expected. When I look at the history (https://github.com/junit-team/junit4/commits/master/src/main/java/org/junit/rules/TemporaryFolder.java), I see how much time has gone into making it work the way users expect (plus a few enhancements).

My initial example is somewhere between 50 and 60 lines but if we're going to put it into Pioneer, my inclination is to replace the public API of the JUnit 4 code with the appropriate extension interfaces. Translating the JUnit 4 tests for this code should prove educational too.

@marcphilipp
Copy link
Member

TempDirectory also deletes all files "recursively", but without using recursion: 😉
https://github.com/junit-team/junit5/blob/master/platform-tests/src/test/java/org/junit/jupiter/extensions/TempDirectory.java#L93-L111

As far as the "API" of the extension I don't think it needs operations like newFolder() or newFile(). I think the Java 7 Files class covers that quite nicely.

@sbrannen
Copy link
Member

sbrannen commented Jan 9, 2018

Hi @smoyer64,

Any chance you'll finish this documentation effort in time for the 5.1 GA release (e.g., within the next few weeks)?

@marcphilipp
Copy link
Member

Closing due to lack of activity.

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

Successfully merging this pull request may close these issues.

6 participants