Add support for cross-multiplying two or more data providers (#1062)

[Vampire]

fixes #1062

[Vampire]

Another option for cross_product: would for example be cross_multiply: or combinations: (though I don't like that one) or cartesian_product: (too long and I have to think how to spell it :-D)

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 87.37624% with 51 lines in your changes are missing coverage. Please review.

Project coverage is 81.73%. Comparing base (2c7db77) to head (4513a1f). Report is 81 commits behind head on master.

:exclamation: Current head 4513a1f differs from pull request most recent head 12f5263. Consider uploading reports for the commit 12f5263 to get more accurate results

Files	Patch %	Lines
...rg/spockframework/runtime/DataIteratorFactory.java	82.70%	29 Missing and 17 partials :warning:
...ock/util/SourceToAstNodeAndSourceTranspiler.groovy	0.00%	0 Missing and 1 partial :warning:
...rg/spockframework/compiler/WhereBlockRewriter.java	98.98%	1 Missing :warning:
.../spockframework/compiler/model/BlockParseInfo.java	75.00%	1 Missing :warning:
...kframework/runtime/extension/IIterationRunner.java	0.00%	1 Missing :warning:
.../org/spockframework/runtime/model/FeatureInfo.java	75.00%	0 Missing and 1 partial :warning:

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1298      +/-   ##
============================================
+ Coverage     80.44%   81.73%   +1.28%     
- Complexity     4337     4550     +213     
============================================
  Files           441      445       +4     
  Lines         13534    14242     +708     
  Branches       1707     1802      +95     
============================================
+ Hits          10888    11640     +752     
+ Misses         2008     1938      -70     
- Partials        638      664      +26     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[leonard84]

@spockframework/supporter please give feedback for this feature

[Vampire]

It now also works between data tables with different separator types

[masooh]

This is a nice improvements and easier to read as shown in https://github.com/spockframework/spock/issues/1062#issuecomment-593946189.

Whats difficult is the name of the label: Spock test should be read like sentences and this does not work any more:

def "all chess fields have size 2"() {
    expect: 'combination having size 2'
    (col + row).size() == 2

    where: 'column'
    col |
    'a' | _
    'b' | _
    ...

    combine: '???'
    row |
    1 | _
    2 | _
    ...
}

In this case a label of is_combined_with: would be better, but I'm not sure if that's always the case, and apart from that: It would be very long.

[masooh]

Does this feature support combinations of Data Tables, Data Pipes, and Variable Assignments?

...
where:
a | _
3 | _
7 | _
0 | _

b << [5, 0, 0]

c = a > b ? a : b

combine:
aa | _
3 | _
7 | _
0 | _

bb << [5, 0, 0]

cc = aa > bb ? aa : bb

Can I have variable assignments which can access both where and combine: x = b * cc?

[Vampire]

Maybe

    expect: 'combination having size 2'
    where: 'column is a or b'
    combine: 'with row 1 or 2'

? Or do you have a better idea? Or would

    expect: 'combination having size 2'
    where: 'column is a or b'
    combined: 'with row 1 or 2'

be better? combined_with or combinedWith would be the first two-word stanza we have and then probably the next flamewar about camelCase vs. snake_case starts. :-D

[masooh]

No I don't want to go for two words, I just wanted to mention it for brainstorming.

IMO it reads better with combined:.

[Vampire]

In its current shape only data tables can be combined with each other and it only combines the table before it with the table after it.

Maybe it will be extended to be able to multiply a data table with a data pipe and a data pipe with a data pipe. You will probably not be able to multiply with a data variable, that wouldn't make much sense, because 10*1 still stays 10.

So your example will probably never be supported as it would define a table, then a data pipe, then try to combine the data variale with the next table , then have another data pipe and another data variable.

[masooh]

I just was curious what would be possible. Thanks for clarification.

Here some more specific thoughts:

• Combination of data variables is not necessary
• data variables should be possible like before and have access to the resulting columns of the combination
• Combination of table and data pipe would be nice
• Combination of data pipe and another data pipe could be done, but has no real advantage over groovy combinations

[Vampire]

Combination of data variables is not necessary

Fully agreed, that makes no sense

data variables should be possible like before and have access to the resulting columns of the combination

Definitely, this is the case and should stay like that

Combination of table and data pipe would be nice

That is not so trivial, currently the combination is done at compile time, just multiplying out the cases and in the end the generated source looks like if you had written the big table in the first place. With data pipes you cannot do that as they are arbitrary iterables. But we are thinking about ways to maybe make it possible.

Combination of data pipe and another data pipe could be done, but has no real advantage over groovy combinations

I don't agree here.

[x, y] << [
  [[1, 2, 3]].transpose(),
  [[4, 5, 6]].transpose()
].combinations()*.flatten()

vs.

x << [1, 2, 3]
combined:
y << [4, 5, 6]

[kriegaex]

Far from wishing to interfere with internal processes in the group of developers, please bear with me when I am asking if you guys think you could conclude the review for this valuable feature anytime soon. This could have been in 2.0, maybe it makes it into 2.1. I think Björn has done some impressive work here.

[Vampire]

It's hanging at me currently. I need to do some changes and don't have the time for it right now.

[kriegaex]

Oh, I see. Thanks for the update. 😀

[Vampire]

Yay, great news @spockframework/supporter, I finally got around to rework this PR. Now you should be able to combine arbitrary data providers and only lost the ability to access columns of previous data tables. Feel free to have a look at the new state and leave a comment. :-)

[AndreasTu]

@Vampire You can close the @Nullable and Throwable discussions, if you like. I am not allowed to resolve/close any of these discussions.

[Vampire]

Oh, ok, I thought as creator of them you should be allowed to resolve them

[Vampire]

Thanks for the great review. :-)

[leonard84]

It has evolved nicely. I'm still thinking about how we could make it extensible in a meaningful way. I think we've discussed making it possible to change the operation with an extension or even inline combine: customCombinerFactory(). Do you think it is feasible to have a IDataCombiner interface that gets DataVariableCombination with DataVariableCombinationOperand. I'm not dead set on having this, but if we could make this feature more general that would be a good thing.

Thoughts?

[Vampire]

I'm not fully sure what you mean by customizing the behavior and right now do not remember a discussion about it. What customizations do you envision? Maybe if you list some use-cases, it would make more sense to me and I could come up with something or I can say why I think it would not make sense. :-D How else would you combine two data providers? Could the use-cases you envision maybe better be done by an IDataDriver instead?

The only thing I right now remember again that you mentioned is maybe having an optional filter: block after a combine: block to filter out combination results that are not wanted, instead of using an @IgnoreIf which would make those unwanted combinations skipped tests instead of not being present at all.

[Vampire]

Yay, finally after three years. :-D 🥂 🎉 🕺