Data table cells can reference previous ones

[l0rinc]

For data driven tests it could be important for a parameter to depend
on a previously calculated value.

Until now this wasn't possible, because the whole where block was calculated,
before the test method was called (via its generated parameters).
Also, the data providers calculate the whole column,
before moving on to the next one, making the references seem unobvious.

The fix was to provide the data provider methods with the list of already
calculated columns.
And because a cell should only refer to cells in the same row,
the variable references were changed to array indexes (see the getAt part).

This commit should play well with partial parameter definitions.

Partially fixes: #509

[PascalSchumacher]

+1

[l0rinc]

Why is it a breaking change exactly (and why is that a problem)?

[leonard84]

This change brakes the current behavior of spocks public interface. The problem with breaking changes is that, perfectly good tests will start failing or won't even compile. So those changes shouldn't be done lightly and if they are done, they need to be communicated as such.

-  def "parameters in different order"(y, x) {
-    expect:
-    x == y
-
-    where:
-    x << [1, 2]
-    y << [1, 2]
-  }

[leonard84]

What is the problem here?

[l0rinc]

perfectly good tests will start failing or won't even compile

The test you quoted is wrong, because it uses a commutative operator (i.e. the x and y values are switched in reality).

They didn't work until now either, they just seemed to (feel free to change change the values the types or the operator).

I don't think this is a breaking change

[leonard84]

@paplorinc you are right the test was wrong, I removed the label again.

Why is 'data table elements can reference each other' ignored, it looks like it should work.

[l0rinc]

No such property: b

The problem seems to be here, i.e. data tables cannot access previous variable declarations.
This was the case before the fix also.

[leonard84]

Can this be fixed? Otherwise the user will have to know which combinations work and which don't.
Also it might be a good idea to have more simple tests for each of the combinations used in this test, some exist already but there are still missing combinations. Especially the minimal failing where clause.

[l0rinc]

I agree perfectly, but:

1. this seemed not strictly related (i.e. should be fixable separately)
2. this change is complicated enough already
3. ...and it's in review for 2 months

If someone reviews and merges them within a few weeks, I would gladly provide a PR for this also :)

[leonard84]

@robfletcher and I were discussing this PR an decided to include it in the upcoming 1.1 Release, which is planned to happen when #17 is implemented and merged (hopefully within two weeks).

I'm ok with fixing it in a separate commit, as long as it is mentioned in the documentation (which is still missing 😉 ). The documentation should include examples for what is and what is not yet possible, and what will never work (e.g., referencing values from a previous execution). Could you please provide this documentation in another PR.

Regarding 3) yeah it's unfortunate, but we do this in our spare time and as you said in 2) it's a complicated change and it takes time and requires more mental work than a simple pr so you can't just do it on the side. Anyway thanks a lot for the PR and I'm looking forward to your next ones. And look on the bright sight, my PR #17 is from September 2013! and its been in limbo until recently ;) so two month vs. two years is not that bad.

[l0rinc]

Thanks @leonard84, @PascalSchumacher, and @robfletcher :)!

two month vs. two years is not that bad.

I wasn't complaining, just stating that I won't do more PRs until I know whether they're needed :)

Rebased my other related PR Implemented partial parameter definitions, can someone please review it?

[l0rinc]

@PascalSchumacher, @leonard84, @robfletcher, @pniederw, could someone please explain what the difference between

where: a << 1

and

where: a = 1

is supposed to be?

Since it's not even documented afaik, could we merge the behavior of the two and keep the = for single values only (before and after data tables)?

The goal would be to make this test pass: https://github.com/paplorinc/spock/blob/a421de5972f0b7fc2a73c85fbfcc5e04c2428c67/spock-specs/src/test/groovy/org/spockframework/smoke/parameterization/DataTables.groovy#L324

[l0rinc]

I tried implementing that variables be usable before and after data tables/pipes, but I could only make one or the other work - either:

expect: CONST > 3 && b < 3
where:  CONST = Math.PI
        b << [CONST / 2, CONST / 3]

or

expect: a + 1 == b
where:  a << [1, 2]
        b = a + 1

(but not both, it wasn't designed that way and I have already rewritten too much...)

Could someone please respond to my questions, I cannot progress if I have to reverse engineer everything...

[PascalSchumacher]

@paplorinc I would like to help you, but I can't. I'm just a spock user, with no knowledge of spock internals. Sorry!

I guess @pniederw is the only one who could be able to help you, but he did his work on spock so long ago, that even he may not remember.

[l0rinc]

Created a refactoring commit, as fixing it ended up taking too much effort, let's do it in smaller steps: #554