Added parameterized specs for Gherkin syntax (Scenario Outline)

[ashleyfrieze]

Relates to #57 - this is a way of providing the capability generically. If this looks good, I'll add scenarioOutline back to it before we merge it.

[coveralls]

Coverage remained the same at 100.0% when pulling 272daa1 on ashleyfrieze:parameterized into d6d5c6e on greghaskins:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 15722a4 on ashleyfrieze:parameterized into d6d5c6e on greghaskins:master.

[coveralls]

Coverage remained the same at 100.0% when pulling a6ddb9d on ashleyfrieze:parameterized into cdba0a2 on greghaskins:master.

[greghaskins]

I don't think this is strictly necessary yet. You can accomplish the same thing out-of-the box with minimal fuss:

Stream.of("a", "b", "c").forEach((parameter) -> {

  describe("Suite test " + parameter, () -> {

    it("can process " + parameter, () -> {
      Assert.assertNotNull(parameter);
    });

    Stream.of("d", "e", "f").forEach((subItem) -> {
      it("can also process " + subItem, () -> {
        Assert.assertNotEquals(parameter, subItem);
      });
    });

  });
});

We could look at fancier syntactic sugar down the line, but I'd say that's a lower priority than #56 .

[ashleyfrieze]

@greghaskins - there's a reason why I thought this was a better idea than doing it long-hand. It's more declarative of intent.

In the counter example, there's imperative programming to define the inner tests to the parameters and there's no guarantee of the user remembering to put the parameter into the test name. In this example, it's implicit in the syntax that you're defining a template of test to run over a collection.

None of this is worth fighting for for the RSpec style, but it's a definite plus to support Gherkin's scenarioOutline.

Perhaps a better name would swing it - allOf is ugly. withTemplate ?

[coveralls]

Coverage remained the same at 100.0% when pulling 04cfc75 on ashleyfrieze:parameterized into 8cb1f5d on greghaskins:master.

[greghaskins]

I wonder if we could make this even more Gherkin-like, perhaps as a specific part of that syntax. Using the example from this page on the Cucumber wiki, I was playing around with something like:

scenarioOutline("eating", () -> {

  Variable<Integer> start = new Variable<>();
  Variable<Integer> eat = new Variable<>();
  Variable<Integer> left = new Variable<>();

  Variable<CukeEater> me = new Variable<>();

  given("there are " + start + " cucumbers", () -> {
    me.set(new CukeEater(start.get()));
  });

  when("I eat " + eat + " cucumbers", () -> {
    me.get().eatCucumbers(eat.get());
  });

  then("I should have " + left + " cucumbers", () -> {
    assertThat(me.get().remainingCucumbers(), is(left.get()));
  });

  examples()
      .with(start,  12, 20)
      .and(eat,     5,  5)
      .and(left,    7,  15);

});

Not sure exactly the best way to represent the tabular data, but this was one try. It would be good if it could be typesafe; this one uses variables and varargs since they are bound to a specific <T>.

[ashleyfrieze]

Cucumber solves the problem by injecting objects into the steps. Your proposal may not be obvious to a cucumber user but it is food for thought. I think the question is how to hook let like objects within the spec to examples generated outside. My idea was a composite input object and a stream of examples.

[greghaskins]

You're the heavier cucumber user, so you'd know better than me what would be expected. I was just looking at syntax. From the wiki, it looks like examples get nested inside Scenario Outline instead of in front of it, and there's a nice way (the table) of seeing which values went together (row by row).

Ideas?

[ashleyfrieze]

Drop strong types and suffer. Use strong types and it's easy but long winded. Use strong types with string typed tables and reflection and write a lot of code to get it to work.

Which would Greg like...

[greghaskins]

To clarify, I'm proposing that we make this feature a Gherkin-specific thing, instead of trying to generalize it to also cover the Spec syntax. There is no real consensus on how to do parameterized tests in RSpec/Jasmine/etc. anyway. Cucumber, on the other hand, has a well-defined precedent for this.

With my comment above, I was fishing for reasoning behind the way Cucumber structures things the way they do, so we can meet the same goals. From my reading of their docs:

• They call it "Scenario Outline" instead of using the word "template." Not sure why.
• They put the template first, followed by input data for examples. Perhaps because the expected behavior and data relationships are more important to define/understand than the specific example cases? Maybe it's easier to add more cases later? Or it could be just a preference by the Cucumber authors.
• Cucumber provides flexibility in how the steps appear; you can put placeholders anywhere in the step name, for example. I'm guessing this is mostly so things would read nicer as complete sentences.
• The examples are presented in a table with headings. For me, this is to make it easy to see which values go with which variables (columns), and also which combinations of values will be running together (rows). Or maybe it's just to stick with conventions used elsewhere (which were used for some other reason).

I'm honestly guessing at the reasoning behind their design decisions and looking for ideas and feedback from heavier Cucumber users.

[ashleyfrieze]

I would guess that 80% of what Cucumber does with Gherkin is just expediency because of the parsing of steps into Regexes. You have to remember that you are expected to write general purpose step glue code which is parameterised by the text hitting regexes within your step definition.

In addition, you can feed data into a single table with a data table under it.

All of this is caused by the fact that Gherkin as a language is plaintext in natural language to be parsed.

I would try to capture the spirit of Gherkin, rather than go down the route of parsing strings, or compromising the implementation to look too much like Cucumber. We have strongly typed Java at our disposal. The aim should be to make this as pragmatic and practical to use. Passing strongly typed objects in is the default, with maybe some convenience methods involving slightly weaker-typed inputs to safe making everything into a new type.

In Cucumber glue code you often end up making strong types for all the things in Cucumber tables... so it's actually EASIER to ditch some Cucumber conventions - similar amounts of work, but less overall!

[greghaskins]

Capturing a couple more ideas here so I have a place to stick them before I forget. Feedback not urgent or expected.

scenarioOutline("eating",
    (start, eat, left) -> {

      Variable<CukeEater> me = new Variable<>();

      given("there are " + start + " cucumbers", () -> {
        me.set(new CukeEater(start));
      });

      when("I eat " + eat + " cucumbers", () -> {
        me.get().eatCucumbers(eat);
      });

      then("I should have " + left + " cucumbers", () -> {
        assertThat(me.get().remainingCucumbers(), is(left));
      });
    },

    example(12, 5, 7),
    example(20, 5, 15)

);

Or, allowing even the scenario name to be parameterized:

outline((start, eat, left) -> {

  scenario("eating " + eat + " cucumbers", () -> {

    Variable<CukeEater> me = new Variable<>();

    given("there are " + start + " cucumbers", () -> {
      me.set(new CukeEater(start));
    });

    when("I eat " + eat + " cucumbers", () -> {
      me.get().eatCucumbers(eat);
    });

    then("I should have " + left + " cucumbers", () -> {
      assertThat(me.get().remainingCucumbers(), is(left));
    });
  });

},
    example(15, 5, 10),
    example(12, 7, 5)
);

[ashleyfrieze]

This is nuts but it works.

ashleyfrieze@7cc5452

			scenarioOutline("Cuke eating", (buy, eat, remaining) -> {
				Variable<CukeEater> cukeEater = new Variable<>();
				given("a default cuke eater", () -> {
					cukeEater.set(new CukeEater());
				});
				and("we buy " + buy + " cukes", () -> {
					cukeEater.get().buy(buy);
				});
				when("we eat " + eat + " cukes", () -> {
					cukeEater.get().eat(eat);
				});
				then("there are " + remaining + " cukes", ()-> {
					assertThat(cukeEater.get().remaining(), is(remaining));
				});
			}, withExamples(
				example(12, 5, "7"),
				example(20, 5, "15")
			));

If fleshed out correctly with implementations for, say, 1-9 parameters, this allows a fully type-safe passing of ad-hoc-defined example-tuples into the body of the scenario outline. The magic is in the withExamples and example calls, which assemble a consistent type-specific set of parameters which can then be pushed into the consuming block in the spec above.

It requires some awfully duplicated back-end code to make something so pretty for the front end...

[greghaskins]

I like withExamples, that reads much nicer. It also sounds like you wouldn't need the parameters in the scenario name itself.

I actually came up with a trick that makes the backend implementation much less terrible than it seems. Basically just four lines per variant, with the rest generic. I'll put some code together in a separate branch/PR.

[ashleyfrieze]

Regarding how it could look in the runner. Here's a real-life example:

and here's how it looks in the runner

I think that we'd probably make scenarioOutline map to describe, Examples: map to describe or context, and make a special compositeSpec for the inner, named after the input example.

That would match the Cuke approach.

[greghaskins]

@ashleyfrieze thanks for the example. That helps clarify what we should do. I think we can use the format you described to make the output very familiar to Cucumber users.

When I get some time, I will post what I'm thinking in terms of implementation. If that works, the API above is pretty solid, I think.

[ashleyfrieze]

Really looking forward to it.

[greghaskins]

@ashleyfrieze Here's a sample of what I'm thinking: https://gist.github.com/greghaskins/6063f71e9ec28831387bb148c0163161

The only pieces that need to duplicate for additional parameter counts are the ParameterizedBlock interfaces and the one-line example() functions.

[ashleyfrieze]

That's fiendishly clever

[ashleyfrieze]

I'm just doing a quick diff of our approaches.

Ashley	Greg
ExampleSource (withExamples)	Stream<Example>
Example1, Example2, Example3 etc	Example (a variable sized example parameterized by ThreeArgBlock etc
DataConsumer1,2,3 etc	and then OneArgBlock, TwoArgBlock etc
scenarioOutline(DataConsumer1, 2 3) etc	a single scenarioOutline specialised by parameterized block type

Yours is much neater and I'll put together a merge request for both scenarioOutline and describe using the technique.

https://www.relishapp.com/rspec/rspec-core/docs/example-groups/shared-examples

suggests describe("name", BLOCK, includeExamples(...)) might be natural. I'm going to suggest that withExamples would be fine here too.

https://github.com/tomykaira/rspec-parameterized has the idea of where as a parameter set.

[greghaskins]

Awesome. FWIW, I'm not married to Stream<Example>, it was just the quickest way to try something out. I'm also not attached to the names OneArgBlock, TwoArgBlock, etc. If you've got a better idea or stronger opinion, that's great.

Let's leave this PR focused only on Gherkin, and discuss the design for spec-style tests over in #80. That one will probably be small after this, with most of the work just on getting the API right.

[ashleyfrieze]

ashleyfrieze@c549c20 is my latest - has a problem with the example name... but closer to what we're talking about.???

[ashleyfrieze]

@greghaskins I missed your guidance while I was hacking away. See my change. Still not in this PR yet... the Stream of Examples is actually a useful implementation detail. I tried something more complex and it would just generate more code.

I had a go at retrofitting to describe for illustration. It looks good.

Let me know what you want for this PR...

The names of the internal types are not especially interesting. To most people.

How much you need to static import might be a useful discussion.

[ashleyfrieze]

[ashleyfrieze]

CONFUSING OTHER CHANGES NOW REMOVED

Here lies the latest greatest version of the paramaterized stuff with support for up to 8 parameters.

[coveralls]

Coverage remained the same at 100.0% when pulling 1b8152c on ashleyfrieze:parameterized into 20827ba on greghaskins:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 49a80b2 on ashleyfrieze:parameterized into 20827ba on greghaskins:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 9f0171b on ashleyfrieze:parameterized into 659a38c on greghaskins:master.

[greghaskins]

@ashleyfrieze has this PR actually been updated with the latest design? I'm looking at the files that changed and still seeing the simple allOf that we started with.

[ashleyfrieze]

has this PR actually been updated with the latest design?

Sorry! I had two local branches pushing to the same remote, which this PR was using. I rebased the wrong (old) one. I've sorted that out now. Sorry for the confusion - blame Christmas and the way it ran the garbage collector in my brain!

[coveralls]

Coverage remained the same at 100.0% when pulling ea21ff0 on ashleyfrieze:parameterized into 659a38c on greghaskins:master.

[greghaskins]

t1, t2, and t3 don't feel natural to me as parameter names. The convention is usually arg0, arg1, etc. for generic unnamed parameters, but I wonder if param1, param2, etc. would be better. Thoughts?

For the one-arg case, it should just be <T> since there's only one type.

[ashleyfrieze]

I recommend against this. There's literally no advantage in t1 vs arg1 or param1 to the reader. It's just an index. However, one reason to keep it short is that your style rules fight with longer names here. I had to lengthen the max-line to stop one rule reformatting the code one way and another saying it was too long.

So, let's keep it to a single character description/name. And let's just agree if we want 0-based or 1-based indexing on the name. I don't mind either way.

[greghaskins]

That's fair. IMO, 1-based is most intuitive. Perhaps it's just the t that makes me think types and not parameters. Maybe p1, p2, et. al? Really doesn't matter though, as you said.

[ashleyfrieze]

I've just made it all 0-based and arg based. So... basically, if you'd like to change it you're welcome to... I was trying to please you. :\

[greghaskins]

I'd prefer to discuss the interface for spec-style parameterized tests over in #80. Let's make this PR just for the Gherkin scenarioOutline and I think we're basically set. – i.e. remove describeParameterized.

[greghaskins]

Should there be spaces around the joining vertical bars? I don't know if that's in line with Cucumber proper, but your screenshot is hard to read for my eyes. | 1 | 2 | 3 | vs |1|2|3|.

[ashleyfrieze]

Easily changed :)

[ashleyfrieze]

@greghaskins I've just implemented what you asked for pretty much as you asked it. There was a lot of code coverage depending on describeParameterized so I made that a helper method in the class that was testing the variants of Example - this defers decision making on the Spectrum version of scenarioOutline.

[coveralls]

Coverage remained the same at 100.0% when pulling cfb8a6b on ashleyfrieze:parameterized into 457bafc on greghaskins:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 9bb2d5e on ashleyfrieze:parameterized into 173713f on greghaskins:master.