Re: Re: JUnit 4.12-beta-1 is released

Hello, Henrik. Thanks for your feedback. My responses are inlined below

On Mon, Aug 4, 2014 at 3:40 AM, [email protected] [junit] <
[email protected]> wrote:

>
>
> Hello
>
> I am sorry for trashing the party but I have a couple of concerns that are
> very likely to slow down the adoption of JUnit-4.12 among the providers of
> 3rd-party tools ...
>
>  --- 1) Don't remove JUnit4ClassRunner! (in package
> org.junit.internal.runners)
>
This was a hard choice. JUnit4ClassRunner was starting to become a
maintenance burden. When evaluating our options, we realized that
JUnit4ClassRunner has been deprecated for six years, and debated whether we
should delete it.

Things to note

- JUnit4ClassRunner was deprecated six years ago with a comment that we
would delete it in the next JUnit version
- We are now releasing 4.12, which is seven versions after we said we would
delete the class
- BlockJUnit4ClassRunner the replacement for JUnit4ClassRunner) has been
available since 4.5
- JUnit4ClassRunner isn't designed to be extended
- JUnit4ClassRunner doesn't support many features that are core parts of
JUnit (particularly Rules)

Given all these issues, we decided to delete these classes. Third party
runners that still use JUnit4ClassRunner should migrate to
BlockJUnit4ClassRunner and target 4.5.

Yes, some teams might have to wait for 3rd-party runners to upgrade to 4.5
APIs, and that might slow adoption for some projects. But the end result
will be more third-party runners that will support Rules (and will
hopefully automatically get other new features we might add).

There may be a very small percentage of projects that want to upgrade their
3rd-party runner to get bug fixes but the next version of that runner
requires 4.5 and the project hasn't upgraded JUnit since before 4.5 was
released. Hopefully that is a small number of projects.

In any case, the deleted classes are self-contained. Since JUnit is open
source, 3rd-party runners are welcome to copy the deleted classes into
their own package tree as long as they do it in a way that's compatible
with our license.

>
> I have developed a number of 3rd-party runnners myself and I want the
> JUnit community to know that as a 3rd-party runner developer you want the
> latest version of your runner to work with any version of JUnit! -
> Otherwise the end-users (test-developers) will be less likely to start
> using your runner because they probably don't want to deal with the
> version-conflict hell that might raise between JUnit and your 3rd-party
> runner.
>
> A couple of good examples are the 3rd-party runners MockitoJUnitRunner
> (from Mockito - http://mockito.org) and CallbackParamsRunner (from
> CallbackParams - http://callbackparams.org)
>
MockitoJUnitRunner extends BlockJUnit4ClassRunner, and appears to be
equivalent to MockitoJUnitRunner (which extends the now-deleted runner)

CallbackParams appears unaffected. CallbackParamsRunner extends Runner and
EnumRunner extends ParentRunner.

> Now I do think that both of these runners will run just fine with
> JUnit-4.12-beta-1 but their builds would fail the day they decide to
> upgrade their JUnit dependency
>

Yes, at some point the developers of these runners will have to choose
between using new APIs that were released in 4.12 and later vs having their
runners support JUnit 4.4 and earlier.

>
>  --- 2) Please remove pull-request #625 from the release! (The annotation
> @RunListener.ThreadSafe)
>
> I think this pull-request represents a very poor design choice that - at
> best - might help performance only for JUnit runners that executes their
> tests with concurrent threads. I would like to point out that most smaller
> unit-tests do not!! - but they still suffer from the same RunListener
> slowness that this issue is supposed to address ...
>
I think you might be misunderstanding the issues that this series of
commits was addressing.

Prior to 4.12, a good portion of the JUnit code was not thread safe, so the
result of running tests in parallel was completely undefined by the JVM.

A few developers went trough a lot of effort to find and fix the
thread-safety issues. Unfortunately, the RunListener API did not specify
that implementations need to be thread-safe, and even some implementations
of RunListener in JUnit were not thread safe.

What we decided to do was to treat existing RunListener implementations as
not thread safe, which means we had to do synchronization. RunListener
implementations that can depend on JUnit 4.12 APIs can use
the @RunListener.ThreadSafe annotation to indicate that their listener is
thread-safe, so doesn't need synchronization.

The alternative would be to assume that all listeners are thread safe. If
that assumption was
wrong, then JUnit could report incorrect results from test runs. This
didn't seem like a good option, so we decided that it is better to
sacrifice some speed for better correctness guarantees.

Note that for modern JVMs, the cost of an uncontested lock is extremely
low, so I don't believe this should have a noticeable impact on tests that
are not run in parallel. Please let us know if you observe performance
problems from these changes.

-- Kevin