Add assertThrows overloads that take a ThrowingSupplier #1401

cushon · 2018-05-02T03:07:35Z

If the given ThrowingSupplier fails to throw the expected exception and
instead returns a value, the string representation of the value is
included in the failure message to aide debugging.

Fixes #1394

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

codecov · 2018-05-02T03:16:14Z

Codecov Report

Merging #1401 into master will decrease coverage by 0.06%.
The diff coverage is 88.88%.

@@             Coverage Diff              @@
##             master    #1401      +/-   ##
============================================
- Coverage     92.09%   92.03%   -0.07%     
- Complexity     3258     3263       +5     
============================================
  Files           301      301              
  Lines          7846     7859      +13     
  Branches        662      663       +1     
============================================
+ Hits           7226     7233       +7     
- Misses          452      456       +4     
- Partials        168      170       +2

Impacted Files	Coverage Δ	Complexity Δ
...rc/main/java/org/junit/jupiter/api/Assertions.java	`95.47% <100%> (-0.37%)`	`123 <3> (+2)`
.../main/java/org/junit/jupiter/api/AssertThrows.java	`91.66% <86.66%> (-8.34%)`	`9 <9> (+4)`
...jupiter/engine/execution/ExtensionValuesStore.java	`87.14% <0%> (-4.29%)`	`21% <0%> (-1%)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e28654c...7bd0a35. Read the comment docs.

sbrannen · 2018-05-02T15:57:03Z

junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertThrows.java

@@ -66,8 +87,16 @@ private AssertThrows() {
 		}

 		String message = buildPrefix(nullSafeGet(messageOrSupplier))
-				+ String.format("Expected %s to be thrown, but nothing was thrown.", getCanonicalName(expectedType));
+				+ String.format("Expected %s to be thrown, but nothing was thrown.", getCanonicalName(expectedType))
+				+ (result != null ? String.format(" (returned %s)", result) : "");


As mentioned in #1394...

In any case, we cannot implement it as you have it currently in your PoC since a method may actually return null.

In other words, we would have to maintain the current behavior for Executable and have different behavior for ThrowingSupplier.

You can probably get away with returning a special Object instance defined in a constant instead of null.

One additional minor thing: the concatenated message will be Expected %s to be thrown, but nothing was thrown. (returned %s). I'd prefer the . to be in the end, i.e. after the closing parenthesis.

In any case, we cannot implement it as you have it currently in your PoC since a method may actually return null.

Thanks for the reminder! I had started with a different approach than the sentinel value, but I'm happy to do that instead, let me know which you prefer.

I'd prefer the . to be in the end, i.e. after the closing parenthesis.

Done.

The current change looks good, too. No need to rework it again. Thanks for reminding me of the term "sentinel value", though. I'll try to remember that for next time. 🙂

marcphilipp

In addition to the inline comment, it would be great if you could add an example to the User Guide and mention it in the Release Notes. In case you don't have time to do so, we can take it from here – just let us know! 🙂

marcphilipp · 2018-05-02T18:35:20Z

junit-jupiter-api/src/main/java/org/junit/jupiter/api/AssertThrows.java

@@ -66,8 +87,16 @@ private AssertThrows() {
 		}

 		String message = buildPrefix(nullSafeGet(messageOrSupplier))
-				+ String.format("Expected %s to be thrown, but nothing was thrown.", getCanonicalName(expectedType));
+				+ String.format("Expected %s to be thrown, but nothing was thrown.", getCanonicalName(expectedType))
+				+ (result != null ? String.format(" (returned %s)", result) : "");


You can probably get away with returning a special Object instance defined in a constant instead of null.

One additional minor thing: the concatenated message will be Expected %s to be thrown, but nothing was thrown. (returned %s). I'd prefer the . to be in the end, i.e. after the closing parenthesis.

cushon · 2018-05-02T19:01:56Z

it would be great if you could add an example to the User Guide and mention it in the Release Notes.

Sure, for the release notes do I include an edit to https://github.com/junit-team/junit5/blob/master/documentation/src/docs/asciidoc/release-notes/release-notes-5.3.0-M1.adoc in this PR? And do you have a suggestion for where to put the example in the user guide? I see a couple of examples of assertThrows, but none that show what the failure message would be for the assertion.

marcphilipp · 2018-05-02T19:19:16Z

Sure, for the release notes do I include an edit to https://github.com/junit-team/junit5/blob/master/documentation/src/docs/asciidoc/release-notes/release-notes-5.3.0-M1.adoc in this PR?

Yep.

And do you have a suggestion for where to put the example in the user guide? I see a couple of examples of assertThrows, but none that show what the failure message would be for the assertion.

Right, it's probably too low level for the User Guide. Let's omit it from there. Instead, we should add a sentence why the new overloaded methods exist to their Javadoc.

cushon · 2018-05-02T19:31:46Z

I updated the release notes and added some detail to the javadoc for the new overloads. Any suggestions for improvements to the text are welcome :)

sormuras · 2018-05-02T19:40:00Z

junit-jupiter-api/src/main/java/org/junit/jupiter/api/Assertions.java

+	 * <p>If you do not want to perform additional checks on the exception instance,
+	 * simply ignore the return value.
+	 *
+	 * <p>If the given {@link ThrowingSupplier<?>} returns a result instead of throwing the expected exception,


Checkstyle choked on the < character. If I'm not mistaken you may omit the <?>.

Thanks, done.

marcphilipp

Besides the Checkstyle issue, this looks very good! 👍

If the given ThrowingSupplier fails to throw the expected exception and instead returns a value, the string representation of the value is included in the failure message to aide debugging.

sormuras · 2018-05-02T20:31:45Z

Thanks @cushon!

Prior to this commit, if a lambda expression provided to assertThrows() returned an array instead of throwing an exception, the array included in the failure message was not human-readable. This commit addresses this by delegating to StringUtils.nullSafeToString() in order to properly print arrays as well. Issue: #1401

@SInCE

…arate package with basic event timing Refactored to -tck format with package name changes + added concept of TestExecution + TerminationInfo SpotlessApply for checkstyle fixes Corrected README Refactored to junit-platform-test and pulled in testArtifact classes from junit-platform-engine Starting javadocs Corrected imports and styling Consolidated some of the API calls for execution recorder + javadocs / API docs ./gradlew spotlessApply Switched to using Instant for execution timings and minor refactoring Fix javadoc NPE by removing {@link} references to generics. Relevant ticket here: https://bugs.openjdk.java.net/browse/JDK-8188248 Bumped API version to tagged PR version of platform 1.3.x Fixed platform-tooling-support-tests for junit-platform-test module Refactor to junit-platform-testkit from junit-platform-test Fixed top level build.gradle reference for junit-platform-testkit name Fixed a few typos Fixed expected package in jar-describe-module test Fixed another package typo for jar-describe-module test Suppress warnings in tests Rename test class to DefaultParallelExecutionConfigurationStrategyTests - Added the "s" to "Test" Disable broken tests in DefaultParallelExecutionConfigurationStrategyTests This commit disables tests in DefaultParallelExecutionConfigurationStrategyTests that are broken due to issues with Mockito on Java 10. Issue: junit-team#1478 Fix DefaultParallelExecutionConfigurationStrategyTests Move test task config to testing.gradle - Remove duplication - Rename test classes to adhere to naming convention Simplify test include config in Gradle build Upgrade to Mockito 2.19.0 Upgrade to Maven Surefire 2.22.0 Upgrade to Univocity 2.6.4 Upgrade to Spotless 3.13.0 Upgrade to Dependency Versions plugin 0.20.0 Release 5.3 M1 Back to snapshots for further development Add missing date for 5.3 M1 release Upgrade to Gradle build scan 1.14 Upgrade to Fast Classpath Scanner 3.1.0 Upgrade to JRuby 9.2.0.0 Upgrade to JMH Gradle plugin 0.4.6 Upgrade to Nemerosa Versioning plugin 2.7.1 Upgrade to git-publish plugin 1.0.1 Upgrade to Kotlin 1.2.50 Upgrade to ktlint 0.24.0 Introduce log4j config for platform-tooling-support-tests project Avoid Eclipse build path cycles This commit modifies the test dependencies for the junit-jupiter-engine project so that there are no build path cycles when importing all of the projects into Eclipse. Switch to different mirror for Ant downloads Use latest Ant version Upgrade to Ant 1.10.4 Ant now supports printing the test run summary via the built-in junitlauncher task. This supersedes the need for the SummaryPrintingListener class, which is now removed from the example project. Revert JRuby upgrade due to incompatibility with asciidoctorj This reverts commit 1c3899e. Do not publish to local Maven repo unless necessary This commit reintroduces the change in 84e2abe and adds a comment to explain the rationale. Issue: junit-team#1429 Polish JavaDoc for @ResourceLock Polish JavaDoc for @ResourceLocks Remove default value for @ResourceLocks.value() Added back API functions Refactored method usage in most tests to adhere to original API Add missing @since/@API to Store.getOrComputeIfAbsent(Class) This commit adds the missing @SInCE and @API declarations to ExtensionContext.Store.getOrComputeIfAbsent(Class<V>) that were accidentally omitted in the JUnit 5.1 release. Issue: junit-team#1110 Polishing Polishing Prune Release Notes for 5.3 Closes: junit-team#1480 Introduce skeleton for 5.3 RC1 release notes Introduce "Release Notes" template Improve error message for 0 arg sets for a @ParameterizedTest Issue: junit-team#1477 Fix broken test Use simple class name as display name in JUnit Vintage Prior to this commit, there was a discrepancy between how Jupiter and Vintage generated display names for test classes. Specifically, Jupiter provided short class names as display names; whereas, Vintage provided fully qualified class names. This commit addresses this by using the simple name of the test class as the display in Vintage as well. Issue: junit-team#970 Document change in Release Notes Issue: junit-team#970 Ensure project is built successfully before publishing Polish HierarchicalTestEngine and collaborators Introduce TestInstanceFactory extension API This commit introduces a new TestInstanceFactory extension API that allows for custom creation of test instances. - TestInstanceFactory must be applied at the class level. - If multiple TestInstanceFactory extensions are registered on a single test class, an exception is thrown. - TestInstanceFactory may also be used with @nested test classes, in which case only one TestInstanceFactory is registered on each level. Issue: junit-team#672 Polish TestInstanceFactory implementation Issue: junit-team#672 Move TestInstanceFactory entry to 5.3 RC1 Release Notes Issue: junit-team#672 Polish TestInstanceFactory section in User Guide Issue: junit-team#672 Instantiate converters and aggregators only once per parameterized test Prior to this commit `ArgumentConverter` and `ArgumentsAggregator` classes were instantiated once per invocation of the `@ParameterizedTest`. With this commit, instances will be created during the first invocation of the `@ParameterizedTest` method, stored and reused on subsequent invocations. If the same converter of aggregator classes are used on other `@ParameterizedTest` methods a new instance will be created on first invocation and reused for subsequent invocations of that method. Of the two proposed options to implement this, either eager instantiation or reuse I chose reuse. The rationale was if the instantiation of a Converter of Aggregator failed with an exception this error could be caught with the existing safe mechanism and also for the failure to be very close to the impacting test invocation. Issue: junit-team#1397 Polishing Document junit-team#1397 in Release Notes Resolves junit-team#1397. Closes junit-team#1433. Make TestInstancePostProcessor a @FunctionalInterface Polish TestInstanceFactoryTests Issue: junit-team#672 Ensure TestInstanceFactory can be registered as a lambda expression Issue: junit-team#672 Test exception handling for TestInstanceFactory errors Issue: junit-team#672 Ensure TestInstanceFactory creates object of correct type Issue: junit-team#672 Resolve TestInstanceFactory at class level Prior to this commit, TestInstanceFactory extensions were resolved lazily on-demand. This led to the undesired side effect that configuration errors were reported for each test method when executing with per-method lifecycle semantics. In addition, this also meant that an attempt was made to invoke all test methods even though the test class could not be instantiated. This commit addresses these issues by resolving the TestInstanceFactory during the "prepare" phase for each ClassTestDescriptor. This commit also introduces the following methods in ExtensionRegistry in order to make the above change possible. - getParent() - getLocalExtensions(Class) Issue: junit-team#672 Handle exception thrown by TestInstanceFactory This commit ensures that an exception thrown by a TestInstanceFactory is wrapped in a TestInstantiationException, unless the exception is already a TestInstantiationException in which case it is simply rethrown. Issue: junit-team#672 Allow Assertions and Assumptions classes to be extended Prior to this commit, it was not possible to extend the Assertions and Assumptions classes since they both were final and had private constructors. This commit removes this restriction in order to support unusual circumstances in which it may be "forbidden" to use static imports. Note, however, that a note has been added to the class-level JavaDoc for these classes in order to voice the opinion of the JUnit Team that it is a best practice to use static imports for methods defined in the Assertions and Assumptions classes. Resolves: junit-team#1484 Upgrade Gradle to 4.9-rc-1 Introduce ParameterizedTestMethodContext `ParameterizedTestMethodContext` encapsulates access to the test method parameters' annotations to ensure it is only done once per `@ParameterizedTest` which significantly improves performance for methods with more than a few declared parameters. Resolves junit-team#1483. Upgrade to Gradle build-scan 1.15 Make AnnotationConsumer a @FunctionalInterface Fix and polish JavaDoc for AnnotationConsumer Polish JavaDoc Polish JavaDoc Upgrade to Kotlin 1.2.51 Use single quotes where feasible in Gradle build scripts Print arrays in failure msg for assertThrows(ThrowingSupplier) Prior to this commit, if a lambda expression provided to assertThrows() returned an array instead of throwing an exception, the array included in the failure message was not human-readable. This commit addresses this by delegating to StringUtils.nullSafeToString() in order to properly print arrays as well. Issue: junit-team#1401 Add note regarding CommonParserSettings.setNumberOfRowsToSkip() Improve exception handling for CSV errors This commit introduces custom exception handling for errors that occur while processing the @CsvSource and @CsvFileSource annotations for parameterized tests. In addition to custom error messages, this commit also introduces a dedicated CsvParsingException. Attempt to fix build on Windows Use archive.apache.org to download Ant binaries Prior to this commit a mirror was used to get the current version, 1.10.4, of the Ant binaries. When the next version, like 1.10.5, is released the mirrors usually switch to provide only the new version. This will fail our build. Now, using archive.apache.org to download Ant binaries, the build will continue to work: the archive provides the current and also former releases. Note: Tool.MAVEN already uses the same source. Refine DoD wrt. test coverage Team Decision: Make it more clear to contributors that not just happy paths should be covered by tests Safely parse FilePosition from query string Prior to this commit, FilePosition.fromQuery() would fail (and return an empty Optional) if any query parameter had a non-integer value. This commit fixes this issue by only attempting to parse a query parameter's value as an integer if the parameter's name is "line" or "column". In addition, the parsing algorithm now stops as soon as it has encountered entries for the line and column. Fixes: junit-team#1489 Implement first-one-wins algorithm in FilePosition.fromQuery() Remove unnecessary @API declaration Support classpath resource for custom TestSource in dynamic tests Issue junit-team#1178 introduced support for creating a UriSource (specifically a FileSource, DirectorySource, or DefaultUriSource) from a URI supplied for a dynamic container or dynamic test. This commit further extends that feature by introducing support for creating a ClasspathResourceSource from a URI supplied for a dynamic container or dynamic test if the URI contains the "classpath" scheme. Otherwise, the behavior introduced in junit-team#1178 is used. Issue: junit-team#1467 Add missing Release Notes entry for 5.3 M1 Issue: junit-team#1178 Polish Release Notes for 5.3 M1 Polishing Exceptions in `@AfterEach` methods fail aborted tests Issue: junit-team#1313 Exceptions in `@AfterAll` methods fail aborted containers Issue: junit-team#1313 Make ThrowableCollector configurable This commit generalizes `ThrowableCollector` to take a predicate that is used to decide whether a `Throwable` is aborted or failed execution. The Jupiter engines uses a specialized implementation that treats OTA's `TestAbortedExceptions` as aborting and everything else as failing: `OpenTest4JAwareThrowableCollector`. In addition, this commit introduces `ThrowableCollector.Factory` and lets `HierarchicalTestEngines` create them in order to allow the engine to decide how to configure its `ThrowableCollectors`. For backwards compatibility, the default implementation returns a factory that always creates instances of `OpenTest4JAwareThrowableCollector`. Issue: junit-team#1313 Use diamond notation where possible Polishing Rename keepAlive to keepAliveSeconds Prior to this commit, the Javadoc for `getKeepAlive()` in `ParallelExecutionConfiguration` mistakenly documented that the timeout's unit was milliseconds when in fact `ForkJoinPoolHierarchicalTestExecutorService` used seconds. Now, the property has been renamed to `getKeepSecondsAlive()` and the documentation has been adapted accordingly. Compile tests using Java 10 Simplify configuration of Java compile tasks Spotless! Document result of ArgumentsProvider throwing TestAbortedException Issue: junit-team#1477 Enable all recommended compiler warnings Prior to this commit, we maintained an explicit list of compiler warnings via `-Xlint:<key>` arguments. The list of possible keys grew to 26, as of javac shipped with JDK 10, and we already missed to activate some new warnings. Now all recommended compiler warnings are enabled via the `-Xlint` argument. Disable "method overrides" warnings for test sources Avoid redundant type argument warning on JDK 9+ This commit introduces a concrete implementation of the internal ResultAwareThrowingSupplierAdapter API in order to avoid a warning that results from the use of an anonymous inner class with "redundant type information" that is flagged on JDK 9 and above. Since the code base is still released using JDK 8 as the target environment, we cannot simply use the diamond notation for the anonymous inner class declaration. Upgrade to Gradle 4.9-rc-2 Reuse config from testing.gradle Polishing Limit queue size of dynamic tests Prior to this commit, concurrently executed dynamic tests (e.g. from a Jupiter `@ParameterizedTest`) were forked immediately after being reported even though all worker threads of the `ForkJoinPool` were already busy. Now, we limit the amount of queued work by using `ForkJoinTask.getSurplusQueuedTaskCount()`. We only fork execution if, according to this heuristic, the task can be executed right away. Otherwise, we execute the task in the current thread which effectively blocks further dynamic tests from being reported and queued. Issue: junit-team#1491 Add tests for additional supported @MethodSource return types A discussion in junit-team#1492 made us aware that @MethodSource factory methods can return 2D object arrays as well as other types that were neither tested nor documented. This commit introduces tests for additionally supported return types for @MethodSource factory methods. Issue: junit-team#1492 Starting refactor to minimize code diff noise based on CR Correcting some formatting problems Minimize the codediff by providing existing method calls, preserving order of method declarations, and indentation

@SInCE

…arate package with basic event timing Refactored to -tck format with package name changes + added concept of TestExecution + TerminationInfo SpotlessApply for checkstyle fixes Corrected README Refactored to junit-platform-test and pulled in testArtifact classes from junit-platform-engine Starting javadocs Corrected imports and styling Consolidated some of the API calls for execution recorder + javadocs / API docs ./gradlew spotlessApply Switched to using Instant for execution timings and minor refactoring Fix javadoc NPE by removing {@link} references to generics. Relevant ticket here: https://bugs.openjdk.java.net/browse/JDK-8188248 Bumped API version to tagged PR version of platform 1.3.x Fixed platform-tooling-support-tests for junit-platform-test module Refactor to junit-platform-testkit from junit-platform-test Fixed top level build.gradle reference for junit-platform-testkit name Fixed a few typos Fixed expected package in jar-describe-module test Fixed another package typo for jar-describe-module test Suppress warnings in tests Rename test class to DefaultParallelExecutionConfigurationStrategyTests - Added the "s" to "Test" Disable broken tests in DefaultParallelExecutionConfigurationStrategyTests This commit disables tests in DefaultParallelExecutionConfigurationStrategyTests that are broken due to issues with Mockito on Java 10. Issue: junit-team#1478 Fix DefaultParallelExecutionConfigurationStrategyTests Move test task config to testing.gradle - Remove duplication - Rename test classes to adhere to naming convention Simplify test include config in Gradle build Upgrade to Mockito 2.19.0 Upgrade to Maven Surefire 2.22.0 Upgrade to Univocity 2.6.4 Upgrade to Spotless 3.13.0 Upgrade to Dependency Versions plugin 0.20.0 Release 5.3 M1 Back to snapshots for further development Add missing date for 5.3 M1 release Upgrade to Gradle build scan 1.14 Upgrade to Fast Classpath Scanner 3.1.0 Upgrade to JRuby 9.2.0.0 Upgrade to JMH Gradle plugin 0.4.6 Upgrade to Nemerosa Versioning plugin 2.7.1 Upgrade to git-publish plugin 1.0.1 Upgrade to Kotlin 1.2.50 Upgrade to ktlint 0.24.0 Introduce log4j config for platform-tooling-support-tests project Avoid Eclipse build path cycles This commit modifies the test dependencies for the junit-jupiter-engine project so that there are no build path cycles when importing all of the projects into Eclipse. Switch to different mirror for Ant downloads Use latest Ant version Upgrade to Ant 1.10.4 Ant now supports printing the test run summary via the built-in junitlauncher task. This supersedes the need for the SummaryPrintingListener class, which is now removed from the example project. Revert JRuby upgrade due to incompatibility with asciidoctorj This reverts commit 1c3899e. Do not publish to local Maven repo unless necessary This commit reintroduces the change in 84e2abe and adds a comment to explain the rationale. Issue: junit-team#1429 Polish JavaDoc for @ResourceLock Polish JavaDoc for @ResourceLocks Remove default value for @ResourceLocks.value() Added back API functions Refactored method usage in most tests to adhere to original API Add missing @since/@API to Store.getOrComputeIfAbsent(Class) This commit adds the missing @SInCE and @API declarations to ExtensionContext.Store.getOrComputeIfAbsent(Class<V>) that were accidentally omitted in the JUnit 5.1 release. Issue: junit-team#1110 Polishing Polishing Prune Release Notes for 5.3 Closes: junit-team#1480 Introduce skeleton for 5.3 RC1 release notes Introduce "Release Notes" template Improve error message for 0 arg sets for a @ParameterizedTest Issue: junit-team#1477 Fix broken test Use simple class name as display name in JUnit Vintage Prior to this commit, there was a discrepancy between how Jupiter and Vintage generated display names for test classes. Specifically, Jupiter provided short class names as display names; whereas, Vintage provided fully qualified class names. This commit addresses this by using the simple name of the test class as the display in Vintage as well. Issue: junit-team#970 Document change in Release Notes Issue: junit-team#970 Ensure project is built successfully before publishing Polish HierarchicalTestEngine and collaborators Introduce TestInstanceFactory extension API This commit introduces a new TestInstanceFactory extension API that allows for custom creation of test instances. - TestInstanceFactory must be applied at the class level. - If multiple TestInstanceFactory extensions are registered on a single test class, an exception is thrown. - TestInstanceFactory may also be used with @nested test classes, in which case only one TestInstanceFactory is registered on each level. Issue: junit-team#672 Polish TestInstanceFactory implementation Issue: junit-team#672 Move TestInstanceFactory entry to 5.3 RC1 Release Notes Issue: junit-team#672 Polish TestInstanceFactory section in User Guide Issue: junit-team#672 Instantiate converters and aggregators only once per parameterized test Prior to this commit `ArgumentConverter` and `ArgumentsAggregator` classes were instantiated once per invocation of the `@ParameterizedTest`. With this commit, instances will be created during the first invocation of the `@ParameterizedTest` method, stored and reused on subsequent invocations. If the same converter of aggregator classes are used on other `@ParameterizedTest` methods a new instance will be created on first invocation and reused for subsequent invocations of that method. Of the two proposed options to implement this, either eager instantiation or reuse I chose reuse. The rationale was if the instantiation of a Converter of Aggregator failed with an exception this error could be caught with the existing safe mechanism and also for the failure to be very close to the impacting test invocation. Issue: junit-team#1397 Polishing Document junit-team#1397 in Release Notes Resolves junit-team#1397. Closes junit-team#1433. Make TestInstancePostProcessor a @FunctionalInterface Polish TestInstanceFactoryTests Issue: junit-team#672 Ensure TestInstanceFactory can be registered as a lambda expression Issue: junit-team#672 Test exception handling for TestInstanceFactory errors Issue: junit-team#672 Ensure TestInstanceFactory creates object of correct type Issue: junit-team#672 Resolve TestInstanceFactory at class level Prior to this commit, TestInstanceFactory extensions were resolved lazily on-demand. This led to the undesired side effect that configuration errors were reported for each test method when executing with per-method lifecycle semantics. In addition, this also meant that an attempt was made to invoke all test methods even though the test class could not be instantiated. This commit addresses these issues by resolving the TestInstanceFactory during the "prepare" phase for each ClassTestDescriptor. This commit also introduces the following methods in ExtensionRegistry in order to make the above change possible. - getParent() - getLocalExtensions(Class) Issue: junit-team#672 Handle exception thrown by TestInstanceFactory This commit ensures that an exception thrown by a TestInstanceFactory is wrapped in a TestInstantiationException, unless the exception is already a TestInstantiationException in which case it is simply rethrown. Issue: junit-team#672 Allow Assertions and Assumptions classes to be extended Prior to this commit, it was not possible to extend the Assertions and Assumptions classes since they both were final and had private constructors. This commit removes this restriction in order to support unusual circumstances in which it may be "forbidden" to use static imports. Note, however, that a note has been added to the class-level JavaDoc for these classes in order to voice the opinion of the JUnit Team that it is a best practice to use static imports for methods defined in the Assertions and Assumptions classes. Resolves: junit-team#1484 Upgrade Gradle to 4.9-rc-1 Introduce ParameterizedTestMethodContext `ParameterizedTestMethodContext` encapsulates access to the test method parameters' annotations to ensure it is only done once per `@ParameterizedTest` which significantly improves performance for methods with more than a few declared parameters. Resolves junit-team#1483. Upgrade to Gradle build-scan 1.15 Make AnnotationConsumer a @FunctionalInterface Fix and polish JavaDoc for AnnotationConsumer Polish JavaDoc Polish JavaDoc Upgrade to Kotlin 1.2.51 Use single quotes where feasible in Gradle build scripts Print arrays in failure msg for assertThrows(ThrowingSupplier) Prior to this commit, if a lambda expression provided to assertThrows() returned an array instead of throwing an exception, the array included in the failure message was not human-readable. This commit addresses this by delegating to StringUtils.nullSafeToString() in order to properly print arrays as well. Issue: junit-team#1401 Add note regarding CommonParserSettings.setNumberOfRowsToSkip() Improve exception handling for CSV errors This commit introduces custom exception handling for errors that occur while processing the @CsvSource and @CsvFileSource annotations for parameterized tests. In addition to custom error messages, this commit also introduces a dedicated CsvParsingException. Attempt to fix build on Windows Use archive.apache.org to download Ant binaries Prior to this commit a mirror was used to get the current version, 1.10.4, of the Ant binaries. When the next version, like 1.10.5, is released the mirrors usually switch to provide only the new version. This will fail our build. Now, using archive.apache.org to download Ant binaries, the build will continue to work: the archive provides the current and also former releases. Note: Tool.MAVEN already uses the same source. Refine DoD wrt. test coverage Team Decision: Make it more clear to contributors that not just happy paths should be covered by tests Safely parse FilePosition from query string Prior to this commit, FilePosition.fromQuery() would fail (and return an empty Optional) if any query parameter had a non-integer value. This commit fixes this issue by only attempting to parse a query parameter's value as an integer if the parameter's name is "line" or "column". In addition, the parsing algorithm now stops as soon as it has encountered entries for the line and column. Fixes: junit-team#1489 Implement first-one-wins algorithm in FilePosition.fromQuery() Remove unnecessary @API declaration Support classpath resource for custom TestSource in dynamic tests Issue junit-team#1178 introduced support for creating a UriSource (specifically a FileSource, DirectorySource, or DefaultUriSource) from a URI supplied for a dynamic container or dynamic test. This commit further extends that feature by introducing support for creating a ClasspathResourceSource from a URI supplied for a dynamic container or dynamic test if the URI contains the "classpath" scheme. Otherwise, the behavior introduced in junit-team#1178 is used. Issue: junit-team#1467 Add missing Release Notes entry for 5.3 M1 Issue: junit-team#1178 Polish Release Notes for 5.3 M1 Polishing Exceptions in `@AfterEach` methods fail aborted tests Issue: junit-team#1313 Exceptions in `@AfterAll` methods fail aborted containers Issue: junit-team#1313 Make ThrowableCollector configurable This commit generalizes `ThrowableCollector` to take a predicate that is used to decide whether a `Throwable` is aborted or failed execution. The Jupiter engines uses a specialized implementation that treats OTA's `TestAbortedExceptions` as aborting and everything else as failing: `OpenTest4JAwareThrowableCollector`. In addition, this commit introduces `ThrowableCollector.Factory` and lets `HierarchicalTestEngines` create them in order to allow the engine to decide how to configure its `ThrowableCollectors`. For backwards compatibility, the default implementation returns a factory that always creates instances of `OpenTest4JAwareThrowableCollector`. Issue: junit-team#1313 Use diamond notation where possible Polishing Rename keepAlive to keepAliveSeconds Prior to this commit, the Javadoc for `getKeepAlive()` in `ParallelExecutionConfiguration` mistakenly documented that the timeout's unit was milliseconds when in fact `ForkJoinPoolHierarchicalTestExecutorService` used seconds. Now, the property has been renamed to `getKeepSecondsAlive()` and the documentation has been adapted accordingly. Compile tests using Java 10 Simplify configuration of Java compile tasks Spotless! Document result of ArgumentsProvider throwing TestAbortedException Issue: junit-team#1477 Enable all recommended compiler warnings Prior to this commit, we maintained an explicit list of compiler warnings via `-Xlint:<key>` arguments. The list of possible keys grew to 26, as of javac shipped with JDK 10, and we already missed to activate some new warnings. Now all recommended compiler warnings are enabled via the `-Xlint` argument. Disable "method overrides" warnings for test sources Avoid redundant type argument warning on JDK 9+ This commit introduces a concrete implementation of the internal ResultAwareThrowingSupplierAdapter API in order to avoid a warning that results from the use of an anonymous inner class with "redundant type information" that is flagged on JDK 9 and above. Since the code base is still released using JDK 8 as the target environment, we cannot simply use the diamond notation for the anonymous inner class declaration. Upgrade to Gradle 4.9-rc-2 Reuse config from testing.gradle Polishing Limit queue size of dynamic tests Prior to this commit, concurrently executed dynamic tests (e.g. from a Jupiter `@ParameterizedTest`) were forked immediately after being reported even though all worker threads of the `ForkJoinPool` were already busy. Now, we limit the amount of queued work by using `ForkJoinTask.getSurplusQueuedTaskCount()`. We only fork execution if, according to this heuristic, the task can be executed right away. Otherwise, we execute the task in the current thread which effectively blocks further dynamic tests from being reported and queued. Issue: junit-team#1491 Add tests for additional supported @MethodSource return types A discussion in junit-team#1492 made us aware that @MethodSource factory methods can return 2D object arrays as well as other types that were neither tested nor documented. This commit introduces tests for additionally supported return types for @MethodSource factory methods. Issue: junit-team#1492 Starting refactor to minimize code diff noise based on CR Correcting some formatting problems Minimize the codediff by providing existing method calls, preserving order of method declarations, and indentation

Prior to this commit, if a lambda expression provided to assertThrows() returned an array instead of throwing an exception, the array included in the failure message was not human-readable. This commit addresses this by delegating to StringUtils.nullSafeToString() in order to properly print arrays as well. Issue: junit-team#1401

sbrannen requested changes May 2, 2018

View reviewed changes

marcphilipp requested changes May 2, 2018

View reviewed changes

marcphilipp added this to the 5.3 M1 milestone May 2, 2018

cushon force-pushed the assertThrows branch 3 times, most recently from 93e0865 to ab88945 Compare May 2, 2018 18:56

cushon force-pushed the assertThrows branch from ab88945 to 92de612 Compare May 2, 2018 19:30

sormuras reviewed May 2, 2018

View reviewed changes

marcphilipp approved these changes May 2, 2018

View reviewed changes

Add assertThrows overloads that take a ThrowingSupplier

7bd0a35

If the given ThrowingSupplier fails to throw the expected exception and instead returns a value, the string representation of the value is included in the failure message to aide debugging.

cushon force-pushed the assertThrows branch from 92de612 to 7bd0a35 Compare May 2, 2018 20:19

sormuras approved these changes May 2, 2018

View reviewed changes

sormuras merged commit a396a07 into junit-team:master May 2, 2018

sbrannen assigned cushon Jun 21, 2018

marcphilipp mentioned this pull request Jun 24, 2018

Add assertThrows overloads with ThrowingSupplier junit-team/junit4#1537

Closed

sbrannen mentioned this pull request Sep 5, 2018

Regression: assertThrows() ThrowingSupplier variants break existing code #1576

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add assertThrows overloads that take a ThrowingSupplier #1401

Add assertThrows overloads that take a ThrowingSupplier #1401

cushon commented May 2, 2018

codecov bot commented May 2, 2018 •

edited

Loading

sbrannen May 2, 2018

marcphilipp May 2, 2018

cushon May 2, 2018

marcphilipp May 2, 2018

marcphilipp left a comment

marcphilipp May 2, 2018

cushon commented May 2, 2018

marcphilipp commented May 2, 2018

cushon commented May 2, 2018

sormuras May 2, 2018

cushon May 2, 2018

marcphilipp left a comment

sormuras commented May 2, 2018

Add assertThrows overloads that take a ThrowingSupplier #1401

Add assertThrows overloads that take a ThrowingSupplier #1401

Conversation

cushon commented May 2, 2018

codecov bot commented May 2, 2018 • edited Loading

Codecov Report

sbrannen May 2, 2018

Choose a reason for hiding this comment

marcphilipp May 2, 2018

Choose a reason for hiding this comment

cushon May 2, 2018

Choose a reason for hiding this comment

marcphilipp May 2, 2018

Choose a reason for hiding this comment

marcphilipp left a comment

Choose a reason for hiding this comment

marcphilipp May 2, 2018

Choose a reason for hiding this comment

cushon commented May 2, 2018

marcphilipp commented May 2, 2018

cushon commented May 2, 2018

sormuras May 2, 2018

Choose a reason for hiding this comment

cushon May 2, 2018

Choose a reason for hiding this comment

marcphilipp left a comment

Choose a reason for hiding this comment

sormuras commented May 2, 2018

codecov bot commented May 2, 2018 •

edited

Loading