From 6c584d3ddc06af15387f673a3d6c1a608d6d8c3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Kautler?= Date: Mon, 13 Jan 2025 11:26:08 +0100 Subject: [PATCH] Fix cross-multiplication of multi-assignment data providers (#2078) Fixes #2074 --- .../runtime/DataIteratorFactory.java | 103 +++++++++++------- .../parameterization/DataProviders.groovy | 32 ++++++ 2 files changed, 95 insertions(+), 40 deletions(-) diff --git a/spock-core/src/main/java/org/spockframework/runtime/DataIteratorFactory.java b/spock-core/src/main/java/org/spockframework/runtime/DataIteratorFactory.java index 235b439dd8..f7e4e31032 100644 --- a/spock-core/src/main/java/org/spockframework/runtime/DataIteratorFactory.java +++ b/spock-core/src/main/java/org/spockframework/runtime/DataIteratorFactory.java @@ -7,6 +7,7 @@ import java.util.*; import org.jetbrains.annotations.NotNull; +import org.spockframework.util.Assert; import org.spockframework.util.Nullable; import spock.config.RunnerConfiguration; @@ -431,23 +432,31 @@ private IDataIterator[] createDataProviderIterators() { nextDataVariableMultiplication = null; } + List dataProviderInfos = context.getCurrentFeature().getDataProviders(); List dataIterators = new ArrayList<>(dataProviders.length); - for (int i = 0; i < dataProviders.length; i++) { - String nextDataVariableName = dataVariableNames.get(i); + for (int dataProviderIndex = 0, dataVariableNameIndex = 0; dataProviderIndex < dataProviders.length; dataProviderIndex++, dataVariableNameIndex++) { + String nextDataVariableName = dataVariableNames.get(dataVariableNameIndex); if ((nextDataVariableMultiplication != null) && (nextDataVariableMultiplication.getDataVariables()[0].equals(nextDataVariableName))) { // a cross multiplication starts - dataIterators.add(createDataProviderMultiplier(nextDataVariableMultiplication, i)); - // skip processed variables - i += nextDataVariableMultiplication.getDataVariables().length - 1; + dataIterators.add(createDataProviderMultiplier(nextDataVariableMultiplication, dataProviderIndex)); + // skip processed providers and variables + int remainingVariables = nextDataVariableMultiplication.getDataVariables().length; + dataVariableNameIndex += remainingVariables - 1; + while (remainingVariables > 0) { + remainingVariables -= dataProviderInfos.get(dataProviderIndex).getDataVariables().size(); + dataProviderIndex++; + } + dataProviderIndex--; + Assert.that(remainingVariables == 0); // wait for next cross multiplication nextDataVariableMultiplication = dataVariableMultiplications.hasNext() ? dataVariableMultiplications.next() : null; } else { // not a cross multiplication, just use a data provider iterator dataIterators.add(new DataProviderIterator( supervisor, context, nextDataVariableName, - context.getCurrentFeature().getDataProviders().get(i), dataProviders[i])); + dataProviderInfos.get(dataProviderIndex), dataProviders[dataProviderIndex])); } } return dataIterators.toArray(new IDataIterator[0]); @@ -457,16 +466,13 @@ private IDataIterator[] createDataProviderIterators() { * Creates a multiplier that is backed by data providers and on-the-fly multiplies them as they are processed. * * @param dataVariableMultiplication the multiplication for which to create the multiplier - * @param i the index of the first data variable for the given multiplication + * @param dataProviderOffset the index of the first data provider for the given multiplication * @return the data provider multiplier */ - private DataProviderMultiplier createDataProviderMultiplier(DataVariableMultiplication dataVariableMultiplication, int i) { + private DataProviderMultiplier createDataProviderMultiplier(DataVariableMultiplication dataVariableMultiplication, int dataProviderOffset) { DataVariableMultiplicationFactor multiplier = dataVariableMultiplication.getMultiplier(); DataVariableMultiplicationFactor multiplicand = dataVariableMultiplication.getMultiplicand(); - int multiplierDataVariablesLength = multiplier.getDataVariables().length; - int multiplicandDataVariablesLength = multiplicand.getDataVariables().length; - if (multiplier instanceof DataVariableMultiplication) { // recursively dive into the multiplication depth-first // if you combined a with b with c with d, the multiplication is represented as @@ -478,22 +484,15 @@ private DataProviderMultiplier createDataProviderMultiplier(DataVariableMultipli // here we first build the data provider multiplier for the multiplier // then we collect the data provider infos and data providers for the multiplicand // and then create the data provider multiplier for them - DataProviderMultiplier multiplierProvider = createDataProviderMultiplier((DataVariableMultiplication) multiplier, i); + DataProviderMultiplier multiplierProvider = createDataProviderMultiplier((DataVariableMultiplication) multiplier, dataProviderOffset); List multiplicandProviderInfos = new ArrayList<>(); - Object[] multiplicandProviders = new Object[multiplicandDataVariablesLength]; - - List dataProviderInfos = context.getCurrentFeature().getDataProviders(); - int j = multiplierDataVariablesLength; - int j2 = multiplierDataVariablesLength + multiplicandDataVariablesLength; - int k = 0; - for (; j < j2; j++, k++) { - multiplicandProviderInfos.add(dataProviderInfos.get(i + j)); - multiplicandProviders[k] = dataProviders[i + j]; - } + List multiplicandProviders = new ArrayList<>(); + collectDataProviders(dataProviderOffset + multiplierProvider.getProcessedDataProviders(), multiplicand, multiplicandProviderInfos, multiplicandProviders); return new DataProviderMultiplier(supervisor, context, Arrays.asList(dataVariableMultiplication.getDataVariables()), - multiplicandProviderInfos, multiplierProvider, multiplicandProviders); + multiplicandProviderInfos, multiplierProvider, + multiplicandProviders.toArray(new Object[0])); } else { // this path handles the innermost multiplication (a * b) // @@ -501,26 +500,32 @@ private DataProviderMultiplier createDataProviderMultiplier(DataVariableMultipli // and then creates a data provider multiplier for them List multiplierProviderInfos = new ArrayList<>(); List multiplicandProviderInfos = new ArrayList<>(); - Object[] multiplierProviders = new Object[multiplierDataVariablesLength]; - Object[] multiplicandProviders = new Object[multiplicandDataVariablesLength]; - - List dataProviderInfos = context.getCurrentFeature().getDataProviders(); - int j = 0; - int j2 = multiplierDataVariablesLength; - for (; j < j2; j++) { - multiplierProviderInfos.add(dataProviderInfos.get(i + j)); - multiplierProviders[j] = dataProviders[i + j]; - } - int k = 0; - for (j2 += multiplicandDataVariablesLength; j < j2; j++, k++) { - multiplicandProviderInfos.add(dataProviderInfos.get(i + j)); - multiplicandProviders[k] = dataProviders[i + j]; - } + List multiplierProviders = new ArrayList<>(); + List multiplicandProviders = new ArrayList<>(); + collectDataProviders(dataProviderOffset, multiplier, multiplierProviderInfos, multiplierProviders); + collectDataProviders(dataProviderOffset + multiplierProviderInfos.size(), multiplicand, multiplicandProviderInfos, multiplicandProviders); return new DataProviderMultiplier(supervisor, context, Arrays.asList(dataVariableMultiplication.getDataVariables()), - multiplierProviderInfos, multiplicandProviderInfos, multiplierProviders, multiplicandProviders); + multiplierProviderInfos, multiplicandProviderInfos, + multiplierProviders.toArray(new Object[0]), + multiplicandProviders.toArray(new Object[0])); + } + } + + private void collectDataProviders(int dataProviderOffset, DataVariableMultiplicationFactor factor, + List factorProviderInfos, List factorProviders) { + List dataProviderInfos = context.getCurrentFeature().getDataProviders(); + int factorDataVariables = factor.getDataVariables().length; + int remainingDataVariables = factorDataVariables; + while (remainingDataVariables > 0) { + int dataProviderIndex = dataProviderOffset + factorDataVariables - remainingDataVariables; + DataProviderInfo dataProviderInfo = dataProviderInfos.get(dataProviderIndex); + factorProviderInfos.add(dataProviderInfo); + factorProviders.add(dataProviders[dataProviderIndex]); + remainingDataVariables -= dataProviderInfo.getDataVariables().size(); } + Assert.that(remainingDataVariables == 0); } @NotNull @@ -667,7 +672,7 @@ private static class DataProviderMultiplier extends BaseDataIterator { /** * The data providers that are used as multiplicand * in this multiplication, if it represents in an - * {@code ((a * b) * c)} multiplication the {@code (ab * b)} + * {@code ((a * b) * c)} multiplication the {@code (ab * c)} * or the {@code (a * b)} part or otherwise {@code null}. * Outside the constructor it is only used for the final cleanup, */ @@ -722,6 +727,11 @@ private static class DataProviderMultiplier extends BaseDataIterator { */ private final int estimatedNumIterations; + /** + * The amount how many data providers are processed by this data provider multiplier. + */ + private final int processedDataProviders; + /** * Creates a new data provider multiplier that handles two sets of plain data providers as factors. * If the sizes of the providers in at least one of the sets can be estimated, @@ -751,6 +761,8 @@ public DataProviderMultiplier(IRunSupervisor supervisor, SpockExecutionContext c ? UNKNOWN_ITERATIONS : estimatedMultiplierIterations * estimatedMultiplicandIterations; + processedDataProviders = multiplierProviders.length + multiplicandProviders.length; + if ((estimatedMultiplierIterations != UNKNOWN_ITERATIONS) && ((estimatedMultiplicandIterations == UNKNOWN_ITERATIONS) || (estimatedMultiplicandIterations > estimatedMultiplierIterations))) { // multiplier is not unknown and multiplicand is unknown or larger, @@ -798,6 +810,8 @@ public DataProviderMultiplier(IRunSupervisor supervisor, SpockExecutionContext c ? UNKNOWN_ITERATIONS : estimatedMultiplierIterations * estimatedMultiplicandIterations; + processedDataProviders = multiplierProvider.getProcessedDataProviders() + multiplicandProviders.length; + if ((estimatedMultiplierIterations != UNKNOWN_ITERATIONS) && ((estimatedMultiplicandIterations == UNKNOWN_ITERATIONS) || (estimatedMultiplicandIterations > estimatedMultiplierIterations))) { // multiplier is not unknown and multiplicand is unknown or larger, @@ -929,6 +943,15 @@ public List getDataVariableNames() { return dataVariableNames; } + /** + * Returns how many data providers are processed by this data provider multiplier. + * + * @return how many data providers are processed by this data provider multiplier + */ + public int getProcessedDataProviders() { + return processedDataProviders; + } + private Iterator[] createIterators(Object[] dataProviders, List dataProviderInfos) { if (context.getErrorInfoCollector().hasErrors()) { return null; diff --git a/spock-specs/src/test/groovy/org/spockframework/smoke/parameterization/DataProviders.groovy b/spock-specs/src/test/groovy/org/spockframework/smoke/parameterization/DataProviders.groovy index e27abaaf18..c19625341f 100644 --- a/spock-specs/src/test/groovy/org/spockframework/smoke/parameterization/DataProviders.groovy +++ b/spock-specs/src/test/groovy/org/spockframework/smoke/parameterization/DataProviders.groovy @@ -297,6 +297,38 @@ where: a << b ] } + @Issue('https://github.com/spockframework/spock/issues/2074') + def 'multi-assignments can be combined'() { + when: + def results = runner.runSpecBody ''' + def 'a feature (#a #b #c #d #e #f)'() { + expect: + true + + where: + [a, b] << [['a1', 'b1'], ['a2', 'b2']] + combined: + [c, d] << [['c1', 'd1'], ['c2', 'd2']] + combined: + [e, f] << [['e1', 'f1'], ['e2', 'f2']] + } + ''' + + then: + results.testsStartedCount == 1 + (2 * 2 * 2) + results.testEvents().started().list().testDescriptor.displayName == [ + 'a feature (#a #b #c #d #e #f)', + 'a feature (a1 b1 c1 d1 e1 f1)', + 'a feature (a1 b1 c1 d1 e2 f2)', + 'a feature (a1 b1 c2 d2 e1 f1)', + 'a feature (a1 b1 c2 d2 e2 f2)', + 'a feature (a2 b2 c1 d1 e1 f1)', + 'a feature (a2 b2 c1 d1 e2 f2)', + 'a feature (a2 b2 c2 d2 e1 f1)', + 'a feature (a2 b2 c2 d2 e2 f2)' + ] + } + static class MyIterator implements Iterator { def elems = [1, 2, 3]