Reduce code verbosity when using loops in yet empty stages

[nikowitt]

Hi Jan,

sorry for bothering again, but due to the fact that we are systematically introducing JGiven in our testing environment, I want to share every feedback to make this great product even better.

Let's check the PrivateTest again:

package com.sobis.suma.tests;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.test.context.ContextConfiguration;

import com.sobis.jaf.JAFConstants;
import com.sobis.jaf.tests.spring.ApplicationJUnit4ClassRunner;
import com.sobis.jaf.tools.JAFLogger;
import com.sobis.suma.tests.PrivateTest.PrivateStage;
import com.tngtech.java.junit.dataprovider.DataProvider;
import com.tngtech.jgiven.Stage;
import com.tngtech.jgiven.junit.SimpleScenarioTest;

@RunWith(ApplicationJUnit4ClassRunner.class)
@ContextConfiguration(locations = { JAFConstants.APPLICATION_CONTEXT })
public class PrivateTest extends SimpleScenarioTest<PrivateStage> {
    public static final JAFLogger LOGGER = new JAFLogger(PrivateTest.class);

    @Test
    @DataProvider({ "1", "2" })
    public void test_report(int cycles) {

        given().something();

        for (int i = 0; i < cycles; i++) {
            if (i > 0) {
                when().and();
            }
            when().something_is_done();
        }

        then().something_is_expected();
    }

    static class PrivateStage extends Stage<PrivateStage> {

        public PrivateStage something_is_done() {
            return self();

        }

        public PrivateStage something_is_expected() {
            return self();

        }

        public PrivateStage something() {
            return self();

        }

    }
}

This test creates the following output:

Test Class: com.sobis.suma.tests.PrivateTest

 Scenario: test report

  Case 1: cycles = 1
   Given something
    When something is done
    Then something is expected

  Case 2: cycles = 2
   Given something
    When something is done
    When something is done
    Then something is expected

When I change my test to

    @Test
    @DataProvider({ "1", "2" })
    public void test_report(int cycles) {

        given().something();

        PrivateStage when = when();
        for (int i = 0; i < cycles; i++) {
            if (i > 0) {
                when.and();
            }
            when.something_is_done();
        }

        then().something_is_expected();
    }

the following output is created:

Test Class: com.sobis.suma.tests.PrivateTest

 Scenario: test report

  Case 1: cycles = 1
   Given something
    When something is done
    Then something is expected

  Case 2: cycles = 2
   Given something
    When something is done
     And something is done
    Then something is expected

The second is implementation is what I expect JGiven to do, but it requires me to have an additional reference "when" before my loop. Is there any way to improve the behaviour so that the first implementation also works?

It would be even greater when it is possible that the "and()" is ignored in the report when a stage does not contain anything yet so I could drop

if (i > 0) {
                when.and();
            }

in my loop, so that the implementation

    @Test
    @DataProvider({ "1", "2" })
    public void test_report(int cycles) {

        given().something();

        for (int i = 0; i < cycles; i++) {
            when().and().something_is_done();
        }

        then().something_is_expected();
    }

results in the second output:

Test Class: com.sobis.suma.tests.PrivateTest

 Scenario: test report

  Case 1: cycles = 1
   Given something
    When something is done
    Then something is expected

  Case 2: cycles = 2
   Given something
    When something is done
     And something is done
    Then something is expected

I know that that is whining on a very high level, but I just wanted to share my experience.

Best regards, Niko

[janschaefer]

first of all: thank you very much for your feedback! This is very valuable, I am happy to get this kind of feedback, so please continue bothering me ;-)

Regarding your question: While I agree that this is not perfectly nice, I guess it is not easy to fix. The problem is that I want to avoid putting such logic into JGiven. Currently, JGiven does not even know that there exist the words 'and' and 'when', this is completely freely defined in the 'Stage' class using the built-in @IntroWord mechanism (this makes JGiven also completely language independent, so you can define, for example, German words like 'Gegeben' and 'und').

What you more or less want is a more intelligent @IntroWord that depending on the position of the intro word prints different words. It maybe possible to extend the @IntroWord annotation so that you can provide an alternative word in case it is not the first word of the stage. As your use case is a rather common use case, it might be worth implementing this. I will have to think about that a bit ;-)

[nikowitt]

Yes, some mechanism that you've just described would be perfect. Since given(), and(), when() seem to be technically equal and there is no hierarchy, it is probably sufficient to provide an alternative when an @IntroWord is the second word of a stage.

Some other idea is to be able to mark given(), when(), then() (or any other @IntroWord) as a "leading" @IntroWord and then to provide an alternative word for @IntroWord in general when the preceding @IntroWord is a "leading" @IntroWord. This at least would cover my case (provide a blank value as alternative word for and() when the preceding @IntroWord is leading) and is not too much additional logic.

[janschaefer]

Another problem is a bit that sometimes you want to explicitly have another 'given' (or when or then) even though you had one already. For example, if you have multiple independent things and you want to highlight that. For example:

Given a car
   and the car drives at 100 miles per hour
Given another car
   and ...

So building logic into the intro words may result in a report that you might not really want. Maybe you can explain your use case a bit more, because we typically never have loops in our scenarios. Instead of having a loop you could write your scenario as:

given().something();
when().something_happens_$_times( 5 );
then().something_is_expected();

This will also improve the output of your report, because instead of having multiple cases you will get a data table in the output:

Given something
   and something happens <times> times
  then something is expected

 Cases:

  | # | times | Status  |
  +---------------------+
  | 1 | 1     | Success |
  | 2 | 5     | Success |

[nikowitt]

Let me show the test:

    @Test
    @DataProvider({
            "2015-01-01,null,2015-01-01",
            "2015-01-01,null,2013-01-01,2015-01-01,2014-01-01",
            "null      ,1   ,2015-01-01",
            "2014-01-01,2   ,2013-01-01,2015-01-01,2014-01-01",
            "2015-01-01,1   ,2013-01-01,2015-01-01,2014-01-01",
            "2013-01-01,2   ,2013-01-01,2015-01-01"
    })
    public void check_last_award_date(DateTime expectedDate,
                                      Integer quotationPositionToRemoveAwardDateFrom,
                                      DateTime... providedDates)   throws Exception {

        given().
            logged_in_system_user().
            and().a_not_awarded_supplier_with_quotation_count_of(providedDates.length);

        SupplierTestScenario when = when();
        for (int i = 0; i < providedDates.length; i++) {
            if (i > 0) {
                when.and();
            }
            when.quotation_$_of_the_same_company_is_awarded_on(i + 1,providedDates[i]);
        }

        if (quotationPositionToRemoveAwardDateFrom != null) {      
            when.and().quotation_$_award_is_removed(quotationPositionToRemoveAwardDateFrom);
        }

        then().supplier_award_date_matches(expectedDate);

    }

In this test, the loop is determined by the length of the var-arg dates which should be directly displayed in the report to be able to check if the logic (the supplier award date always match the latest award date) is correct, so just shifting the loop down has a negative impact on the readability.

Test Class: com.sobis.tests.businessobject.BOSupplierTest

 Scenario: check last award date

  Case 1: expectedDate = 2015-01-01T00:00:00.000+01:00,
quotationPositionToRemoveAwardDateFrom = null, providedDates =
2015-01-01T00:00:00.000+01:00
   Given logged in system user
     And a not awarded supplier with quotation count of 1
    When quotation 1 of the same company is awarded on 2015-01-01
    Then supplier award date matches 2015-01-01

  Case 2: expectedDate = 2015-01-01T00:00:00.000+01:00,
quotationPositionToRemoveAwardDateFrom = null, providedDates =
2013-01-01T00:00:00.000+01:00, 2015-01-01T00:00:00.000+01:00,
2014-01-01T00:00:00.000+01:00
   Given logged in system user
     And a not awarded supplier with quotation count of 3
    When quotation 1 of the same company is awarded on 2013-01-01
     And quotation 2 of the same company is awarded on 2015-01-01
     And quotation 3 of the same company is awarded on 2014-01-01
    Then supplier award date matches 2015-01-01

  Case 3: expectedDate = null, quotationPositionToRemoveAwardDateFrom =
1, providedDates = 2015-01-01T00:00:00.000+01:00
   Given logged in system user
     And a not awarded supplier with quotation count of 1
    When quotation 1 of the same company is awarded on 2015-01-01
     And quotation 1 award is removed
    Then supplier award date matches null

  Case 4: expectedDate = 2014-01-01T00:00:00.000+01:00,
quotationPositionToRemoveAwardDateFrom = 2, providedDates =
2013-01-01T00:00:00.000+01:00, 2015-01-01T00:00:00.000+01:00,
2014-01-01T00:00:00.000+01:00
   Given logged in system user
     And a not awarded supplier with quotation count of 3
    When quotation 1 of the same company is awarded on 2013-01-01
     And quotation 2 of the same company is awarded on 2015-01-01
     And quotation 3 of the same company is awarded on 2014-01-01
     And quotation 2 award is removed
    Then supplier award date matches 2014-01-01

  Case 5: expectedDate = 2015-01-01T00:00:00.000+01:00,
quotationPositionToRemoveAwardDateFrom = 1, providedDates =
2013-01-01T00:00:00.000+01:00, 2015-01-01T00:00:00.000+01:00,
2014-01-01T00:00:00.000+01:00
   Given logged in system user
     And a not awarded supplier with quotation count of 3
    When quotation 1 of the same company is awarded on 2013-01-01
     And quotation 2 of the same company is awarded on 2015-01-01
     And quotation 3 of the same company is awarded on 2014-01-01
     And quotation 1 award is removed
    Then supplier award date matches 2015-01-01

  Case 6: expectedDate = 2013-01-01T00:00:00.000+01:00,
quotationPositionToRemoveAwardDateFrom = 2, providedDates =
2013-01-01T00:00:00.000+01:00, 2015-01-01T00:00:00.000+01:00
   Given logged in system user
     And a not awarded supplier with quotation count of 2
    When quotation 1 of the same company is awarded on 2013-01-01
     And quotation 2 of the same company is awarded on 2015-01-01
     And quotation 2 award is removed
    Then supplier award date matches 2013-01-01

[janschaefer]

Ok. I also think that data tables are hard to get in this example. However, you might want to use table parameters. See http://jgiven.org/docs/parameterizedsteps/ and http://jgiven.org/javadoc/com/tngtech/jgiven/annotation/Table.html.

Using that you could have one method the_awarded_quotations_of_the_same_company_are( quotations )

Btw.: you might want to annotated the logged_in_system_user method with @Hidden so that it does not appear in the report (http://jgiven.org/javadoc/com/tngtech/jgiven/annotation/Hidden.html)

[nikowitt]

Thanks for the advice, I'll play a little bit with the @Table as soon as I can spare some time. The @Hidden parameter is already frequently used in our JGiven wrapper classes :)

[nikowitt]

I'm just shifting the update to a new post as it might have gone lost.

What could be interesting is to be able add some automatic counter row/column to the table. This way, @Table'd data could be referenced to more easily later (in my example it's basically tabling the quotation award dates without supplying an index as a variable).

[janschaefer]

I agree, I had the same idea, but forgot to create an issue for that :-). Basic idea would be to give the table annotation an attribute to enable these indices.

[janschaefer]

I created #116 to track that.

[nikowitt]

Another idea which might help in this case is to provide a counter that shows the number of appended words/methods of a stage or to implement some isEmpty() flag per stage. This does not cover all cases (e.g the one you mentioned above), but it reduces condition checks in the test code itself.

[janschaefer]

I think the most simplest implementation would be to provide an attribute to @IntroWord where you could specify the text for the first occurrence and/or of a word and the following ones. Something like:

@IntroWord( primary = "given", secondary="and" )
public SELF givenOrAnd() {
    return self();
}

[nikowitt]

Sounds good, as long as this can be provided to the static given(), when(), then() - but probably I can just extend from a JGiven scenario class, "override" the static methods to just use another @IntroWord as the givenOrAnd() is most likely the default in our cases.