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

Allow Assertions and Assumptions classes to be extended #1484

Closed
lgoldstein opened this issue Jul 1, 2018 · 16 comments
Closed

Allow Assertions and Assumptions classes to be extended #1484

lgoldstein opened this issue Jul 1, 2018 · 16 comments

Comments

@lgoldstein
Copy link

lgoldstein commented Jul 1, 2018

The current class is final and has a private constructor. This means that in order to use its methods without having to qualify them as Assertions.assertEquals/assertTrue/etc... one has to use static imports. In many projects (including ours) this is not allowed due to various concerns (here is neither the place nor the time to discuss them). In JUnit 4, the Assert class was not final, so we could easily extend it and use the various assertXXX methods without having to qualify them

public class FooTest extends Assert {
    @Test
    public void testSomething() {
        ....
        assertEquals("Mismatched X and Y", x, y);
    }
}

Unfortunately, we cannot do this with Assertions - which means we cannot use JUnit 5 since our project does not allow static imports and we would rather not use qualified access on our 1000's of assertions (again, neither the place nor the time to discuss why...) . It would really help (IMO not just us) if Assertions was extensible...

@marcphilipp
Copy link
Member

I don't think it would be wise to allow subclassing Assertions because it would blur how it's supposed to be used.

@junit-team/junit-lambda Thoughts?

@sbrannen
Copy link
Member

sbrannen commented Jul 2, 2018

which means we cannot use JUnit 5

This limitation does not in any way prevent you from using JUnit Jupiter (a.k.a., JUnit 5).

You are free to use other assertion libraries such as AssertJ, Hamcrest, Truth, etc.

You can in fact continue to use org.junit.Assert if you so desire.

See Third-party Assertion Libraries in the User Guide for details.

@sbrannen
Copy link
Member

sbrannen commented Jul 2, 2018

I don't think it would be wise to allow subclassing Assertions because it would blur how it's supposed to be used.

Based on principle, I agree.

Assertions are indeed supposed to be used via static imports.

However, if a significant number of users request that Assertions be extensible we could reconsider.

@sormuras
Copy link
Member

sormuras commented Jul 2, 2018

How about wrapping the assertions you need into your custom extensible type?

package a.b.c;

import org.junit.jupiter.api.Assertions;

class Assert {
  void assertEquals(...) { Assertions.assertEquals(...); }
}

Usage:

class FooTest extends a.b.c.Assert { ...

No static imports, only the assertions you need, customizable with custom assertions and only need to adopt the package of the Assert type you extend.

...not use qualified access on our 1000's of assertions...

If you're low on time, keep your unit tests running "as-is" with the Vintage engine. Write new ones with Jupiter API.

@lgoldstein
Copy link
Author

How about wrapping the assertions you need into your custom extensible type?

Not really a solution since I need to copy-paste all the delegated methods (and there are many of them) - not to mention that if there is a new or modified method on a new version I need to keep updating my class.

If you're low on time, keep your unit tests running "as-is" with the Vintage engine.

Again, not really a solution - the whole idea is to get rid of obsolete code. If all I do is use the Vintage engine why switch to JUnit 5 and not remain with 4 ?

Assertions are indeed supposed to be used via static imports.

Again, without going into a discussion, this is a legitimate decision, but we decided that the cons outweigh the pros of using static imports and therefore enforce (via Checkstyle) the ban on using them.

@sormuras
Copy link
Member

sormuras commented Jul 2, 2018

Do you also extend java.lang.System in order to use static methods like arraycopy and friends?

Again, not really a solution - the whole idea is to get rid of obsolete code.
If all I do is use the Vintage engine why switch to JUnit 5 and not remain with 4 ?

If test code is obsolete, delete it.
If test code is hard to change but fulfills a purpose, keep it. Vintage to the rescue for legacy tests.
If new test code is developed, choose the right tool to achieve your (testing) goals. Be it 4 or 5 or any other framework.

Your design decisions are fine with me. Somehow limited, but fine.

@lgoldstein
Copy link
Author

Do you also extend java.lang.System in order to use static methods like arraycopy and friends?

The example is not equivalent for a variety of reasons...

Be it 4 or 5 or any other framework.

this issue is just convenient, nothing more. If this were the only issue, it would not have prevented us from adopting version 5. For now though 4 is the choice ) - not because of this issue but rather because of several others much more problematic...

@jbduncan
Copy link
Contributor

jbduncan commented Jul 2, 2018

It sounds to me that perhaps the best solution, at least in the interim, would be to upgrade to JUnit 5 but to continue using JUnit 4's Assert class.

@lgoldstein WDYT?

@lgoldstein
Copy link
Author

Like I said, why would we do that if we remain bound to the JUnit 4 API(s) ? What about the extra work it would take to retro-fit the code to JUnit 5 once it catches up and provides the features that are missing from it (and exist already in JUnit 4) ...

Maybe next project - after JUnit 5 matures some more...

@marcphilipp
Copy link
Member

@lgoldstein have you considered using Assertions.assertEquals() etc. without static imports?

We're always interested in feedback on missing features. Are this and #878 the only things you're missing or is there something else?

@jbduncan
Copy link
Contributor

jbduncan commented Jul 2, 2018

Like I said, why would we do that if we remain bound to the JUnit 4 API(s) ? What about the extra work it would take to retro-fit the code to JUnit 5 once it catches up and provides the features that are missing from it (and exist already in JUnit 4) ...

Hmm. Apologies if I've missed something,, but the way I personally see it, from what you've explained so far, it sounds to me that you sincerely want to upgrade to JUnit 5 but you're frustrated because you can't do so fully due to various issues.

I can't talk about your other issues, but if it were me, I'd happily take a sort of incremental approach where for new tests, I'd use JUnit 5's annotations and JUnit 4's Assert, whilst continuing to run my old completely-JUnit-4-based tests on the Vintage engine. I'd then wait to see if the JUnit 5 developers decide to make Assertions instantiable, and then take a similar incremental approach to gradually migrating all uses of Assert to Assertions, before finally tackling migrating the old completely-JUnit-4-based tests to JUnit 5.

The way I see it, the effort that would be needed to migrate the new JUnit 5 tests from the old Assert to the new Assertions would be rather small compared to migrating your existing JUnit 4 tests to JUnit 5.

Again, apologies if I've missed something. I'm genuinely curious to know if my proposed solution would work for you, and if not, why so. :)

@sbrannen
Copy link
Member

sbrannen commented Jul 3, 2018

Team Decision:

Allow Assertions and Assumptions to be extended via a protected constructor.

@sbrannen sbrannen self-assigned this Jul 3, 2018
@sbrannen sbrannen added this to the 5.3 RC1 milestone Jul 3, 2018
@sbrannen sbrannen changed the title Feature request: Assertions class should not be final and allow other classes to extend it Allow Assertions class to be extended Jul 3, 2018
@sbrannen sbrannen changed the title Allow Assertions class to be extended Allow Assertions and Assumptions classes to be extended Jul 3, 2018
@sbrannen
Copy link
Member

sbrannen commented Jul 3, 2018

in progress

@ghost ghost removed the status: in progress label Jul 3, 2018
@sbrannen
Copy link
Member

sbrannen commented Jul 3, 2018

This has been addressed in master in dc89365 and will be available in JUnit 5.3 RC1.

@sbrannen
Copy link
Member

sbrannen commented Jul 3, 2018

It will of course also be available in the next SNAPSHOT build.

@lgoldstein
Copy link
Author

Unfortunately, we do not use snapshots and cannot afford to wait until such time as 5.3 is released - but thanks for listening and discussing the issue.

dotCipher pushed a commit to dotCipher/junit5 that referenced this issue Aug 11, 2018
…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
dotCipher pushed a commit to dotCipher/junit5 that referenced this issue Sep 18, 2018
…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
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

5 participants