Skip to content

Introduce SPI for generating display names #162

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

Closed
2 tasks done
nipafx opened this issue Feb 21, 2016 · 24 comments
Closed
2 tasks done

Introduce SPI for generating display names #162

nipafx opened this issue Feb 21, 2016 · 24 comments

Comments

@nipafx
Copy link
Contributor

nipafx commented Feb 21, 2016

In many cases the strings passed to @DisplayName are just a prettified version of the class/method name, e.g.:

@DisplayName("List")
class ListTest {
    @Nested
    @DisplayName("when empty")
    class WhenEmpty {
        @Test
        @DisplayName("can add")
        void canAdd() {
            ...
        }
    }
}

It might be a nice-to-have if this could happen automatically, e.g.:

@DisplayName(prettify = true)
class ListTest {
    @Nested
    class WhenEmpty {
        @Test
        void canAdd() {
            ...
        }
        @Test
        @DisplayName("can't get")
        void cantGet() {
            ...
        }
    }
}

Related Issues

Deliverables

  • Introduce an extension for generating display names.
  • Provide an out-of-the-box implementation of the extension that can be enabled via a JUnit configuration property.
@schauder
Copy link
Contributor

how about making 'prettify' the default action when no explicit name is given?

@sbrannen sbrannen added this to the 5.0 M1 milestone Feb 27, 2016
@sbrannen sbrannen changed the title Prettify test method names Prettify test method display names by default Feb 27, 2016
@sbrannen
Copy link
Member

I like the proposal from @schauder, and I have changed the title of this issue accordingly.

@junit-team/junit-lambda, what does the rest of the team think about making this change?

@jlink
Copy link
Contributor

jlink commented Feb 27, 2016

I think we should discuss all related issues (#145 and #153) together and make consistent changes: name, display name and unique ID have intermingled concerns which we should separate first.

@bechte
Copy link
Contributor

bechte commented Feb 28, 2016

I like the idea and also support going forward into this direction.

We should discuss in detail what prettify exactly means as we somehow set a standard for naming test methods and I know there are different styles in the community:

https://dzone.com/articles/7-popular-unit-test-naming

With this change we might make a statement which style should be used as we support it the best.

We simply should keep that in mind as we move on. Still, I support this change.

@jlink
Copy link
Contributor

jlink commented Feb 28, 2016

I'd rather not have prettification as default because there'll always be some corner cases (eg. acronyms and abbreviations) where any automatic prettification scheme will fail.

@marcphilipp marcphilipp modified the milestones: 5.0 M2, 5.0 M1 Apr 15, 2016
@marcphilipp marcphilipp modified the milestones: 5.0 Backlog, 5.0 M2 Apr 29, 2016
@signed
Copy link
Contributor

signed commented May 7, 2016

I agree with @bechte about the different testing styles and the corner cases mentioned by @jlink. I'd prefer not having this complexity and ‘one size fits all’ naming convention in junit core.

How about providing an ExtensionPoint to provide the display name?
This way there can be a domain/style/language aware version of pretty for everyone that is willing to write an extension. By defining my own @test annotation that activates the extension by default, I can even skip the repetitive @DisplayName annotation for all of my test methods.

@jlacar
Copy link

jlacar commented Sep 20, 2016

Two edges to this sword:

  1. Bad examples: https://www.youtube.com/watch?v=bpKbvb95Bv0 (from 06:17:00)
  2. Good examples: http://blog.codefx.org/libraries/junit-5-basics/#Naming-Tests

YMMV but my experience often shows that more people will find and follow bad examples than the good ones. I fear the most common usage we'll end up seeing is where the display names basically repeat what the method name already says. Goodbye DRY, now we're WET (Writing Everything Twice) all over the place.

The link to the bad examples a JavaOne 2016 presentation stream, the relevant segment was given by folks from Penn State University, starting at the 06:17:00 mark. To think that these are the kind of examples that future authors of JUnit tests will learn from makes me shudder. I also would humbly point out that the JUnit 5 user guide doesn't have very good examples either.

I like the idea of automatically prettifying the names for the report through an ExtensionPoint. Are there really that many corner cases where auto-prettify would get tripped up? My teams' test naming convention follows Neal Ford's style, using underscores to separate words so an extension that auto replaces "_" with " " would have a very high, if not perfect, success rate.

@marcphilipp marcphilipp modified the milestones: 5.1+ Backlog, 5.0 M5 Dec 22, 2016
@sbrannen sbrannen changed the title Prettify test method display names by default Introduce extension for generating display names May 1, 2017
@sbrannen
Copy link
Member

sbrannen commented May 1, 2017

In light of recent team discussions, I have renamed this issue to focus on the introduction of a new extension for generating display names.

@sbrannen
Copy link
Member

sbrannen commented May 1, 2017

FYI: I have added a Deliverables section to this issue's description.

@marcphilipp marcphilipp removed this from the 5.0 M5 milestone May 6, 2017
@sbrannen
Copy link
Member

Regarding the camel-case converter, searching the Internet reveals lots of discussions around this topic -- for example, https://stackoverflow.com/questions/2559759/how-do-i-convert-camelcase-into-human-readable-names-in-java/2560017#2560017

sormuras added a commit that referenced this issue Sep 19, 2018
@sormuras
Copy link
Member

Thanks for the review and feedback, Sam. Pushed a second spike to #1588.

sormuras added a commit that referenced this issue Sep 19, 2018
@sbrannen sbrannen changed the title Introduce extension for generating display names Introduce SPI for generating display names Sep 19, 2018
@sbrannen
Copy link
Member

Of course, we could also come up with a hybrid camel-case + underscore style.

Actually, the more I think about it (and after having looked at the latest updates @sormuras made to his PR), I think we are actually implementing the Decorator pattern with our styles.

This leads to the following, treating style names as functions.

  • default()
  • underscore(default())
  • camelCase(underscore(default()))

So that means that the camel-case display name generator would automatically convert underscores to spaces simply be delegating to the underscore style which in turn delegates to the default style.

sormuras added a commit that referenced this issue Sep 19, 2018
sormuras added a commit that referenced this issue Sep 19, 2018
sormuras added a commit that referenced this issue Sep 20, 2018
sormuras added a commit that referenced this issue Sep 20, 2018
sormuras added a commit that referenced this issue Sep 20, 2018
sormuras added a commit that referenced this issue Sep 20, 2018
sormuras added a commit that referenced this issue Sep 20, 2018
sormuras added a commit that referenced this issue Sep 21, 2018
sormuras added a commit that referenced this issue Sep 21, 2018
sormuras added a commit that referenced this issue Sep 22, 2018
sormuras added a commit that referenced this issue Sep 22, 2018
sormuras added a commit that referenced this issue Sep 25, 2018
sormuras added a commit that referenced this issue Sep 25, 2018
sormuras added a commit that referenced this issue Sep 25, 2018
Addresses #162
sormuras added a commit that referenced this issue Sep 26, 2018
sormuras added a commit that referenced this issue Sep 26, 2018
sormuras added a commit that referenced this issue Sep 26, 2018
sormuras added a commit that referenced this issue Sep 26, 2018
@ghost ghost removed the status: in progress label Sep 26, 2018
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

10 participants