Introduce Display Name Generator SPI #1588

sormuras · 2018-09-16T14:44:15Z

Overview

Addresses #162

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

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

...er-engine/src/main/java/org/junit/jupiter/engine/descriptor/DefaultDisplayNameGenerator.java

sbrannen

I requested that unrelated changes be pushed to master.

documentation/src/test/java/example/RepeatedTestsDemo.java

...upiter-engine/src/test/java/org/junit/jupiter/api/extension/ExtensionComposabilityTests.java

sbrannen

I've submitted a separate review for the meat of this PR.

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

...er-engine/src/main/java/org/junit/jupiter/engine/descriptor/DefaultDisplayNameGenerator.java

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

codecov · 2018-09-19T05:00:47Z

Codecov Report

Merging #1588 into master will increase coverage by 0.05%.
The diff coverage is 98.07%.

@@             Coverage Diff              @@
##             master    #1588      +/-   ##
============================================
+ Coverage     92.03%   92.09%   +0.05%     
- Complexity     3548     3561      +13     
============================================
  Files           323      325       +2     
  Lines          8474     8510      +36     
  Branches        737      742       +5     
============================================
+ Hits           7799     7837      +38     
  Misses          502      502              
+ Partials        173      171       -2

Impacted Files	Coverage Δ	Complexity Δ
...va/org/junit/jupiter/api/DisplayNameGenerator.java	`100% <100%> (ø)`	`1 <1> (?)`
...r/engine/descriptor/NestedClassTestDescriptor.java	`100% <100%> (ø)`	`4 <1> (ø)`	⬇️
...piter/engine/descriptor/JupiterTestDescriptor.java	`86.44% <100%> (-1.06%)`	`23 <1> (-2)`
...r/engine/descriptor/MethodBasedTestDescriptor.java	`100% <100%> (ø)`	`9 <1> (-1)`	⬇️
...jupiter/engine/descriptor/ClassTestDescriptor.java	`96.79% <100%> (-0.09%)`	`62 <2> (-1)`
...it/jupiter/engine/descriptor/DisplayNameUtils.java	`96.96% <96.96%> (ø)`	`15 <15> (?)`
...jupiter/engine/execution/ExtensionValuesStore.java	`91.42% <0%> (+4.28%)`	`22% <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 5e83bfd...ee139f7. Read the comment docs.

sbrannen · 2018-09-19T11:05:11Z

@sormuras, what was the resolution of the jacoco vs. synthetic discussion?

#1588 (comment)

sbrannen

I've requested minor changes and posed a few questions as well.

documentation/src/test/java/example/DisplayNameGeneratorDemo.java

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

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

...iter-engine/src/main/java/org/junit/jupiter/engine/descriptor/MethodBasedTestDescriptor.java

...upiter-engine/src/test/java/org/junit/jupiter/api/extension/ExtensionComposabilityTests.java

sormuras · 2018-09-19T11:13:03Z

what was the resolution for the jacoco vs. synthetic discussion?

Mh, my comment is buried somewhere or was lost before being posted. Anyway, the underlying issue were the default methods introduced in the DisplayNameGenerator interface. They triggered JaCoCo to introduce the new real method. This new method was not a bridge, and not synthetic.

As I removed the default methods from the inteface, the JaCoCo method vanished as well.

sbrannen · 2018-09-19T11:19:35Z

As I removed the default methods from the inteface, the JaCoCo method vanished as well.

Aha. Interesting!

But that makes sense now.

Thanks for the explanation.

sbrannen · 2018-09-19T11:21:52Z

In any case, those JaCoCo methods will return due to the default interface methods in the upcoming TestWatcher API (#1393 (comment)). 😉

sormuras · 2018-09-19T11:23:01Z

Create an issue in advance? Have a solution shelved here...

sbrannen · 2018-09-19T11:24:33Z

Create an issue in advance? Have a solution shelved here...

Sure, feel free.

But please let @geo-m know so that he's informed. 😉

sbrannen · 2018-09-19T12:29:36Z

Open Issues: (not covered in a review)

Should @DisplayNameGeneration be @Inherited (in inheritance hierarchies)?
Should @DisplayNameGeneration be inherited (in @Nested hierarchies)?
What package(s) do we want the new annotation, interface, and implementations to reside in?

sbrannen

I added another mini review.

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

sormuras · 2018-09-22T04:28:56Z

All comments addressed, starting with the documentation.

sormuras · 2018-09-22T06:32:25Z

Removed the WIP prefix from the issue title. @sbrannen shall I merge to master and you polish the documentation there?

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

sbrannen

Minor change requested

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

sormuras · 2018-09-23T16:08:45Z

@marcphilipp anything else? If not, I'll continue with documentation.

andreybleme · 2018-09-25T16:20:19Z

Hi @sbrannen and @sormuras, what's avoiding this change to be merged at master? Just asking because it blocks #1595, which I'll start working.

sormuras · 2018-09-25T16:30:55Z

A final review by @marcphilipp tonight or ... soon. ;-)

marcphilipp

Looking good! 👍 One added some minor comments. Please add an entry to the release notes as well.

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

documentation/src/docs/asciidoc/user-guide/writing-tests.adoc

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

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

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

junit-jupiter-engine/src/test/java/org/junit/jupiter/api/DisplayNameGenerationTests.java

Addresses #162

Move pre-defined generators to DisplayNameGenerator interface.

documentation/src/docs/asciidoc/user-guide/writing-tests.adoc

Closes #162

ghost assigned sormuras Sep 16, 2018

ghost added the status: in progress label Sep 16, 2018

sormuras force-pushed the issues/162-display-name-generator branch 3 times, most recently from f5687c6 to 43fe669 Compare September 16, 2018 17:31

sormuras commented Sep 17, 2018

View reviewed changes

...-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterTestDescriptor.java Outdated Show resolved Hide resolved

sormuras commented Sep 17, 2018

View reviewed changes

...er-engine/src/main/java/org/junit/jupiter/engine/descriptor/DefaultDisplayNameGenerator.java Outdated Show resolved Hide resolved

sbrannen requested changes Sep 17, 2018

View reviewed changes

This was referenced Sep 17, 2018

Introduce SPI for generating display names #162

Closed

Introduce TestWatcher extension API for processing test execution results #1393

Closed

sormuras changed the title ~~[WIP] Introduce Display Name Generator extension~~ [WIP] Introduce Display Name Generator SPI Sep 17, 2018

sormuras force-pushed the issues/162-display-name-generator branch from 7542436 to 91231ac Compare September 19, 2018 05:00

sormuras force-pushed the issues/162-display-name-generator branch from 91231ac to aa0d3bb Compare September 19, 2018 06:07

sbrannen requested changes Sep 19, 2018

View reviewed changes

sbrannen added theme: extensions status: team discussion component: Jupiter type: new feature labels Sep 19, 2018

sormuras force-pushed the issues/162-display-name-generator branch from aa0d3bb to 7aa3d61 Compare September 19, 2018 11:45

sormuras force-pushed the issues/162-display-name-generator branch from 7aa3d61 to 1572ccc Compare September 19, 2018 14:11

sbrannen requested changes Sep 19, 2018

View reviewed changes

sormuras force-pushed the issues/162-display-name-generator branch from 8f18466 to 2799c3e Compare September 22, 2018 04:28

sormuras changed the title ~~[WIP] Introduce Display Name Generator SPI~~ Introduce Display Name Generator SPI Sep 22, 2018

marcphilipp reviewed Sep 22, 2018

View reviewed changes

junit-jupiter-api/src/main/java/org/junit/jupiter/api/DisplayNameGeneration.java Outdated Show resolved Hide resolved

sormuras force-pushed the issues/162-display-name-generator branch from 559a666 to 6e7f2a3 Compare September 22, 2018 20:37

This was referenced Sep 23, 2018

Add CamelCase Display Name Generator #1595

Closed

Create IndicativeSentences Display Name Generator #1596

Closed

sbrannen approved these changes Sep 23, 2018

View reviewed changes

sbrannen requested changes Sep 23, 2018

View reviewed changes

junit-jupiter-api/src/main/java/org/junit/jupiter/api/DisplayNameGeneration.java Outdated Show resolved Hide resolved

sormuras force-pushed the issues/162-display-name-generator branch from ac4a355 to 2dc2965 Compare September 23, 2018 16:00

sbrannen approved these changes Sep 23, 2018

View reviewed changes

sormuras force-pushed the issues/162-display-name-generator branch from 2dc2965 to be2d0e6 Compare September 25, 2018 10:55

marcphilipp requested changes Sep 25, 2018

View reviewed changes

sormuras force-pushed the issues/162-display-name-generator branch from be2d0e6 to c75ae2c Compare September 25, 2018 19:28

sormuras added 2 commits September 26, 2018 06:17

Introduce DisplayName generation SPI

4f47618

Addresses #162

Document display name generation

5950e42

Move pre-defined generators to DisplayNameGenerator interface.

sormuras force-pushed the issues/162-display-name-generator branch from c75ae2c to 3ad6cf2 Compare September 26, 2018 04:19

marcphilipp requested changes Sep 26, 2018

View reviewed changes

documentation/src/docs/asciidoc/user-guide/writing-tests.adoc Outdated Show resolved Hide resolved

Introduce DisplayNameUtils helper

ee139f7

Closes #162

sormuras force-pushed the issues/162-display-name-generator branch from 3ad6cf2 to ee139f7 Compare September 26, 2018 07:30

marcphilipp approved these changes Sep 26, 2018

View reviewed changes

sormuras merged commit 4db8e8e into master Sep 26, 2018

ghost removed the status: in progress label Sep 26, 2018

sormuras deleted the issues/162-display-name-generator branch September 26, 2018 08:05

sbrannen added the theme: reporting label Aug 5, 2019

d3vel0per mentioned this pull request Sep 14, 2021

Add tophelpers command 🏆 Together-Java/TJ-Bot#117

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce Display Name Generator SPI #1588

Introduce Display Name Generator SPI #1588

sormuras commented Sep 16, 2018 •

edited

Loading

sbrannen left a comment

sbrannen left a comment

codecov bot commented Sep 19, 2018 •

edited

Loading

sbrannen commented Sep 19, 2018 •

edited

Loading

sbrannen left a comment

sormuras commented Sep 19, 2018

sbrannen commented Sep 19, 2018

sbrannen commented Sep 19, 2018

sormuras commented Sep 19, 2018 •

edited

Loading

sbrannen commented Sep 19, 2018

sbrannen commented Sep 19, 2018 •

edited

Loading

sbrannen left a comment

sormuras commented Sep 22, 2018

sormuras commented Sep 22, 2018

sbrannen left a comment

sormuras commented Sep 23, 2018

andreybleme commented Sep 25, 2018

sormuras commented Sep 25, 2018

marcphilipp left a comment

Introduce Display Name Generator SPI #1588

Introduce Display Name Generator SPI #1588

Conversation

sormuras commented Sep 16, 2018 • edited Loading

Overview

Definition of Done

sbrannen left a comment

Choose a reason for hiding this comment

sbrannen left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 19, 2018 • edited Loading

Codecov Report

sbrannen commented Sep 19, 2018 • edited Loading

sbrannen left a comment

Choose a reason for hiding this comment

sormuras commented Sep 19, 2018

sbrannen commented Sep 19, 2018

sbrannen commented Sep 19, 2018

sormuras commented Sep 19, 2018 • edited Loading

sbrannen commented Sep 19, 2018

sbrannen commented Sep 19, 2018 • edited Loading

sbrannen left a comment

Choose a reason for hiding this comment

sormuras commented Sep 22, 2018

sormuras commented Sep 22, 2018

sbrannen left a comment

Choose a reason for hiding this comment

sormuras commented Sep 23, 2018

andreybleme commented Sep 25, 2018

sormuras commented Sep 25, 2018

marcphilipp left a comment

Choose a reason for hiding this comment

sormuras commented Sep 16, 2018 •

edited

Loading

codecov bot commented Sep 19, 2018 •

edited

Loading

sbrannen commented Sep 19, 2018 •

edited

Loading

sormuras commented Sep 19, 2018 •

edited

Loading

sbrannen commented Sep 19, 2018 •

edited

Loading