junit4: Undeprecate ExpectedException rule

@daicoden

I’m pretty sure assertThrows returns the throwable, so you could just capture it and make assertions like you would for any other object:

StatusRuntimeException thrown = assertThrows(StatusRuntimeException.class, () -> {
   GrpcDispatch.makeRequest(service::enableJob, JobReference.newBuilder().setName(UNKNOWN_JOB).build());
});

assertEquals(Status.INVALID_ARGUMENT, thrown.getStatus());

I also don’t understand why this deprecation should have happened in JUnit 4. I’m migrating to JUnit 5 using the junit-vintage-engine to support both versions temporarily. But this pulls in JUnit 4.13 which for some reason decided that it was a good idea to deprecate things (that have been supported for years) in what is likely one of its last releases.

There also seems to be no reason for the deprecation other than to point out an alternative – this could have been achieved by at least offering an alternative (make the constructor public, or offer another static method – message communicated).

The ExpectedException code as it is now will continue working (and will keep working even in later releases) as it simply uses basic features of JUnit offered in an (at the time) convenient way.

I would be in favor of deprecating just ExpectedException.none().

Doing # 2 would simplify future migration to JUnit 5.

There should be a version overlap when the old and the new feature coexist without either being deprecated.

Why? The intent of the deprecation is to make people aware of the new, in our opinion better API.

I also agree that we should only deprecate ExpectedException.none() to make it less invasive.

@stefanbirkner Ok with you?

I wonder if @Test(expected = ...) should also be deprecated since we have assertThrows now. Or at least the reference to ExpectedException should be removed from the attribute’s Javadoc.

I’m not sure why this likely being the last JUnit 4.x release is related to whether or not we deprecate things. JUnit rarely deletes any APIs, so deprecation hasn’t been an indication that an API is going away. It’s an indication that either that the API might not be supported and/or better APIs exist. Both are true in this case (we’ve rejected proposed changes to this rule because we think it’s better to just have people use assertThrows())

I suggest Spring slowly migrate away from ExpectedException. A few days of work spread out over a few months to get to safer and easier to understand tests seems like a win. The benefit is other people less familiar with recent changes to JUnit will know to avoid ExpectedException

ErrorProne has a check for this that will provide a recommended fix, so I don’t think it would take several days to resolve this issue.