TNG / junit-dataprovider

A TestNG like dataprovider runner for JUnit with many additional features
Apache License 2.0
246 stars 164 forks source link

Custom parameter format #101

Closed codecholeric closed 6 years ago

codecholeric commented 7 years ago

I don't know, if this is already possible, and I just don't know about it, but I find myself often writing wrapper objects, simply providing a reasonable toString for the test parameter (e.g. the input is a Class<?> but I don't want class some.pkg.Foo in the output, but would rather have the simple class name).

Is there any elegant solution for this case? I know I can control the format of the method name, but the parameter always seems to be presented as toString?

aaschmid commented 7 years ago

Hi @codecholeric,

you can provide a custom StringConverter. Here are the examples:

Does this help? Feedback and ideas for simplifying the mechanism (especially with JUnit5) are very welcome.

Cheers, Andreas

codecholeric commented 7 years ago

Sorry, my initial post might have been a little bit unclear, as far as I understand it, your example is exactly the other way around, you write '2017-01-01' and have a Date parameter, that is printed as date.toString() in the test case output. What I meant, is a DataProvider serving Date, but the test case output of the parameter is customized. I.e.

@DataProvider
public static Object[][] dates() {
    return $$($(new Date()));
}

@UseDataProvider("dates")
public void test(Date date) {
    // ...
}

and the output would for example only be the year of Date, e.g. test 2017). Is there any way, to customize it in this direction? Only the output - the provider should give Date and the test should take Date, just the output of the parameter in the test description should be customized, and not date.toString().

aaschmid commented 7 years ago

Oh, now reading I thought in the wrong direction. You can provide a custom placeholder / formatter. Here are the corresponding examples:

Better and helpful this time? Feedback and ideas for simplifying the mechanism (especially with JUnit Jupiter) are very welcome.

codecholeric commented 7 years ago

The example looks like it could be, what I mean, however I don't fully understand the test. When I run it (in IntelliJ), the parameters methodName1 and methodName2 are not really changed? Shouldn't the test have a case methodName2 = "veryV...ed" that must be equal then? I only see differences in case, but since the test asserts equalsIgnoreCase, this is kind of irrelevant / a test for String#equalsIgnoreCase? Also, when I execute the test in debug mode, I don't end up in StripParameterLengthPlaceholder?

Another question, this doesn't seem threadsafe? In case I run two test classes in parallel, and both want to format the same type in a different way?

As for feedback, when I tried to figure this out, I was kind of wishing for a mechanism like JGiven's @Format (meta-)annotation, so you could just annotate your test input parameter and it would then be represented differently in the test method name (same would work for conversion, too). Have you considered (and rejected?) this approach?

aaschmid commented 7 years ago

Unfortunately, the test does not work anymore in JUnit4 as @BeforeClass integration was removed with v1.10.4 (because of other unwanted sideeffects) and therefore the additional placeholder is not registered. I fixed this now using a static initializer in master branch. Can you check it out again?

And yes, the test case itself is made up just for demonstration as I cannot assert the test naming at that point ... so this is kind of a manual test which I forgot to check recently. Though, it should be thread-safe, because the placeholder are called within a synchronized block using themselves as lock object (I know this is not the best solution I completely change it for JUnit5).

However, I kind of like the idea with the @Format annotation according to JGiven. I will have a look at it.

dalewking commented 6 years ago

I have similar issues but think the given solution is way too complex and does not really work. My case is simply one of the values is a long, but I want it displayed in hex for a particular data provider. I end up sending it in as a hex string and converting to long in the test method.

codecholeric commented 6 years ago

Maybe to help the discussion, I can add a typical type of test case that I'm frequently encountering in my project. Let's suppose we have some ShippingAddress POJO with 10 properties. toString() will print some form that lists all those properties. We want to write a test that checks that an inner country shipping address leads to a different tax rate than an outer country address. Instinctively I would write sth. like

@DataProvider
public static Object[][] addressTestCases()
{
    return $$(
            $(createDefaultAddress(), expectedInnerCountryTaxRate()),
            $(createForeignAddress(), expectedOutOfCountryTaxRate())
    );
}

@Test
@UseDataProvider("addressTestCases")
public void adds_extra_tax_on_shippings_to_foreign_addresses(ShippingAddress shippingAddress, BigDecimal expectedTaxRate)
{
    Order order = defaultOrder().withShippingAddress(shippingAddress).build();

    Tax tax = taxCalculator.calculateTax(order);

    assertThat(tax.getRate()).isEqualTo(expectedTaxRate);
}

In practice I would of course have more data points. When the tests run, my test cases are described as

adds_extra_tax_on_shippings_to_foreign_addresses(0: ShippingAddress{firstName: "firstNameFoo", lastName: "lastNameFoo", street: "streetFoo", number: 78, zip: ....

So in particular, the information what was relevant for my test case, is lost. Sure, when I debug it, I can figure out "oh, it's case 5, and the AssertionError is x, thus the problem must be ...", but when I skim those tests in our CI environment, it would be much nicer, to read adds_...(0: Inner country shipping address, 1: ...)

Alternatively I could break it down to primitive values, but in this case this would kind of look like

@DataProvider
public static Object[][] addressTestCases()
{
    return $$(
            $(false, expectedInnerCountryTaxRate()),
            $(true, expectedOutOfCountryTaxRate())
    );
}

@Test
@UseDataProvider("addressTestCases")
public void adds_extra_tax_on_shippings_to_foreign_addresses(boolean isForeign, BigDecimal expectedTaxRate)
{
    ShippingAddress shippingAddress = isForeign? createForeignAddress() : createDefaultAddress();
    Order order = defaultOrder().withShippingAddress(shippingAddress).build();

    Tax tax = taxCalculator.calculateTax(order);

    assertThat(tax.getRate()).isEqualTo(expectedTaxRate);
}

and then print

adds_extra_tax_on_shippings_to_foreign_addresses(0: false, 1: 0.19)

which is almost worse. So what I normally end up with, is writing a TestCase pojo, that has the parameter and a reasonable toString(). Since we have written quite many of those TestCase wrappers, that principally just mean wrap it in the DataProvider, unwrap it in the test, I was wondering, if there is a better solution.

I was thinking, if this use case is too specific for my project, and the implementation too complex, I might just end up writing some generic one/two/three parameter test case

TestCase<A, B>(
        A firstParam, Function<A, String> firstToString, 
        B secondParam, Function<B, String> secondToString)

Then I could reuse toString() conversion as well. Just the unwrapping of TestCase in every test seems like unnecessary bloat.

aaschmid commented 6 years ago

Thanks for your comments,

@dalewking and @codecholeric here are the brainstormed options I could think about:

Do you have any futher ideas? What would be your preference?

aaschmid commented 6 years ago

These are examples how the usage could look like:

codecholeric commented 6 years ago

Personally I like the second and third approach the most, with no specific preference. Both approaches would solve my problems, as far as I can see. The decision probably depends on the desired degree of separation from an API point of view, approach 3 suggests more, that it is an independent feature, while approach 2 binds it stronger to the core functionality... E.g. approach 3 could also be generalized in the future (e.g. a generic hook where approach 3 is only one implementation), while approach 2 defines very clearly, what is intended, with little room for extension. What is the better approach, I don't know... (Guess "it depends" :wink:) The first approach, in particular the combination with format = "[%i: %aa[0..-1]]" feels cryptic and not natural to me, but that might just be personal preference :wink: The 4th approach would work, too, for my case, but if there is no big difference, then the other approaches seem more powerful. If a TestNameFormatter has access to the method within the implementation (and by that annotations on the method), one could implement sth. like approach 4 oneself, if desired, while the other way round, it would hardly be possible.

aaschmid commented 6 years ago

I implemented approach 2 ("specify a test name formatter with @DataProvider") for junit-jupiter in commits above, simply because it fits better with existing APIs and strategy.

@codecholeric Is that sufficient for you or do you also need it in junit4 module?

Note: I want to get rid of format = "..." option in @DataProvider in the future but giving formatter precedence should be perfect for now.

dalewking commented 6 years ago

I needed it in junit 4 but today is my last day on that job so not so much now.

On Fri, Mar 30, 2018, 5:03 PM Andreas Schmid notifications@github.com wrote:

I implemented approach 2 ("specify a test name formatter with @DataProvider") for junit-jupiter in commits above, simply because it fits better with existing APIs and strategy.

@codecholeric https://github.com/codecholeric Is that sufficient for you or do you also need it in junit4 module?

Note: I want to get rid of format = "..." option in @DataProvider in the future but giving formatter precedence should be perfect for now.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TNG/junit-dataprovider/issues/101#issuecomment-377622887, or mute the thread https://github.com/notifications/unsubscribe-auth/AAVHzlwP0QfaDTFK1bkuh9r9pHH1S0gAks5tjp2ugaJpZM4QPQm6 .

aaschmid commented 6 years ago

Hi @dalewking, maybe your colleagues can use it, so I also provided the solution for JUnit4.

codecholeric commented 6 years ago

I need it for JUnit 4 for our current project, too, so thanks a lot :smiley: I'll try this as soon as 2.3 is out...

aaschmid commented 6 years ago

Hehe, depends only on my time tonight :-)

codecholeric commented 6 years ago

Nice, thanks :smiley: It might take a little, until I can give you feedback from our project, because it's not an easy process to upgrade OS library versions :wink: