Introduce exception handling support for lifecycle callback methods #1509

ghost · 2018-07-26T15:51:46Z

Overview

PR for Issue #1454
Make it possible to also handle exceptions thrown by BeforeEach/AfterEach methods via the TestExecutionExceptionHandler.
The new handler methods are default methods to make them optional and to not break the FunctionalInterface.
This PR is a proposal to solve the issue. It's not complete. The Javadoc is not adjusted and the exception handling for the BeforeAll/AfterAll methods is missing.

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

Definition of Done

There are no TODOs left in the code
Method preconditions are checked and documented in the method's Javadoc
Coding conventions (e.g. for logging) have been followed
Change is covered by automated tests including corner cases, errors, and exception handling
Public API has Javadoc and @API annotations
Change is documented in the User Guide and Release Notes
All continuous integration builds pass

ghost · 2018-07-31T10:39:50Z

Added exception handling for BeforeAll/AfterAll methods

sbrannen

Please add tests for the new exception handling callbacks.

sbrannen · 2018-07-31T11:13:17Z

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassTestDescriptor.java

-		for (Method method : this.beforeAllMethods) {
-			throwableCollector.execute(
-				() -> executableInvoker.invoke(method, testInstance, extensionContext, registry));
+		for (Method method: this.beforeAllMethods) {


Please reintroduce the space between method and :.

sbrannen · 2018-07-31T11:15:34Z

The build is currently broken due to formatting issues.

Run gradlew spotlessApply to fix these violations.

ghost · 2018-07-31T13:42:38Z

But where to put the tests (TestExecutionExceptionHandlerTests or BeforeAndAfterEachTests or a new class) without blowing up an existing class or repeating too much code in a new class?

sbrannen · 2018-07-31T14:25:41Z

I'd probably just introduce a set of test classes parallel to the existing TestExecutionExceptionHandlerTests.

As for adhering to the DRY principle, just do your best. We can always refactor later. The main objective for starters is just having tests in place.

sbrannen · 2018-07-31T14:26:09Z

and yeah.... please don't "blow up" anything. 😉

ghost · 2018-08-07T12:54:52Z

Tests added

ghost · 2018-08-08T18:16:23Z

Finally removed the formatting issues - these checks are really pedantic...
Don't know why there is still a failure.

jbduncan · 2018-08-08T18:19:37Z

@phoenix384 Were you not able to simply run gradlew spotlessApply to fix all the formatting problems? 🤔

sbrannen · 2018-08-09T11:44:00Z

@phoenix384, you really shouldn't try to tackle formatting issues one-by-one.

Instead, as @jbduncan mentioned, you should simply execute gradlew spotlessApply once before committing your changes.

Alternatively, you can set up the formatting in your IDE.

This is all documented in the CONTRIBUTING documentation for JUnit 5.

sbrannen · 2018-08-09T11:47:15Z

The last failure appears to have something to do with Jacoco:

https://travis-ci.org/junit-team/junit5/jobs/413706488#L1467-L1486

Maybe that's just an intermittent issue on the CI server.

Does gradle build work consistently on your machine?

marcphilipp · 2018-09-14T11:23:11Z

...jupiter-api/src/main/java/org/junit/jupiter/api/extension/TestExecutionExceptionHandler.java

 	 */
 	void handleTestExecutionException(ExtensionContext context, Throwable throwable) throws Throwable;

+	default void handleExceptionInBeforeAllMethod(ExtensionContext context, Throwable throwable) throws Throwable {


Team Decision: Please rename all newly introduced methods to better align with the naming convention of the previously existing method, e.g. handleBeforeAllMethodExecutionException etc.

ghost · 2019-02-08T13:59:56Z

5.4 is released and this is still not merged. Is there a reason?

sbrannen · 2019-02-08T14:15:38Z

Hi @phoenix384,

5.4 is released and this is still not merged. Is there a reason?

Yes, requested changes have not yet been made (e.g., #1509 (comment)), and the following tasks under "Definition of Done" have not been addressed.

Public API has Javadoc and @API annotations
Change is documented in the User Guide and Release Notes

Please rebase on master and address those 3 tasks, and we'll make sure to review it again and get it into JUnit 5.5.

Thanks!

marcphilipp · 2019-05-03T10:47:11Z

Closing in favor of #1868.

phoenix384 added 6 commits July 26, 2018 17:29

Update TestExecutionExceptionHandler.java

3d90700

Update TestMethodTestDescriptor.java

8a432db

Update TestMethodTestDescriptor.java

0971059

Update TestMethodTestDescriptor.java

1123259

Update TestExecutionExceptionHandler.java

5d485ea

Update TestMethodTestDescriptor.java

586f097

ghost mentioned this pull request Jul 28, 2018

Introduce extension APIs to handle exceptions from lifecycle callback methods #1454

Closed

phoenix384 added 3 commits July 31, 2018 12:27

Update TestMethodTestDescriptor.java

6c50204

Update TestExecutionExceptionHandler.java

ae047f4

Update ClassTestDescriptor.java

2bb07cb

sbrannen added type: enhancement theme: extensions component: Jupiter labels Jul 31, 2018

sbrannen added this to the 5.3 RC1 milestone Jul 31, 2018

sbrannen requested changes Jul 31, 2018

View reviewed changes

Update ClassTestDescriptor.java

7e16c23

marcphilipp modified the milestones: 5.3 RC1, 5.4 M1 Aug 3, 2018

phoenix384 added 2 commits August 7, 2018 14:51

Added Tests

4f548b4

Added tests

5176418

phoenix384 added 4 commits August 8, 2018 17:21

Fix formatting

bb13e61

Fix formatting

7f2185f

Fix formatting

89ed782

Fix formatting

cea8010

phoenix384 added 5 commits August 8, 2018 18:47

Fix formatting

7f26488

Fix formatting

1adcc3c

Fix formatting

6babbfa

Fix formatting

1584f4a

Fix formatting

c0f2678

marcphilipp requested changes Sep 14, 2018

View reviewed changes

marcphilipp modified the milestones: 5.4 M1, 5.4 M2 Nov 23, 2018

sormuras modified the milestones: 5.4 RC1, 5.4 GA Jan 11, 2019

marcphilipp modified the milestones: 5.4 GA, 5.5 M1 Jan 25, 2019

sbrannen changed the title ~~Exception handling for BeforeEach/AfterEach methods (issue 1454)~~ Introduce exception handling support for lifecycle callback methods Feb 8, 2019

marcphilipp modified the milestones: 5.5 M1, 5.5 M2 Mar 8, 2019

sbrannen added type: new feature and removed type: enhancement labels Apr 10, 2019

marcphilipp modified the milestones: 5.5 M2, 5.5 Backlog Apr 18, 2019

pietrygamat mentioned this pull request Apr 22, 2019

Introduce extension APIs to handle exceptions from lifecycle callback methods #1868

Closed

7 tasks

marcphilipp removed this from the 5.5 Backlog milestone May 3, 2019

marcphilipp closed this May 3, 2019

Uh oh!

Introduce exception handling support for lifecycle callback methods #1509

Introduce exception handling support for lifecycle callback methods #1509

Uh oh!

Conversation

ghost commented Jul 26, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Definition of Done

Uh oh!

ghost commented Jul 31, 2018

Uh oh!

sbrannen left a comment

Choose a reason for hiding this comment

Uh oh!

sbrannen Jul 31, 2018

Choose a reason for hiding this comment

Uh oh!

sbrannen commented Jul 31, 2018

Uh oh!

ghost commented Jul 31, 2018

Uh oh!

sbrannen commented Jul 31, 2018

Uh oh!

sbrannen commented Jul 31, 2018

Uh oh!

ghost commented Aug 7, 2018

Uh oh!

ghost commented Aug 8, 2018

Uh oh!

jbduncan commented Aug 8, 2018

Uh oh!

sbrannen commented Aug 9, 2018

Uh oh!

sbrannen commented Aug 9, 2018

Uh oh!

marcphilipp Sep 14, 2018

Choose a reason for hiding this comment

Uh oh!

ghost commented Feb 8, 2019

Uh oh!

sbrannen commented Feb 8, 2019

Uh oh!

marcphilipp commented May 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ghost commented Jul 26, 2018 •

edited by ghost

Loading