Skip to content

Commit

Permalink
Iterable data providers are not called multiple times for size() anymore
Browse files Browse the repository at this point in the history
Iterables which are no Collections are not called for size() anymore,
which would lead to multiple expensive operations, when the
DefaultGroovyMethods.size() method is used.

Fixes #2022
  • Loading branch information
AndreasTu committed Oct 18, 2024
1 parent ee3f305 commit d335c49
Show file tree
Hide file tree
Showing 4 changed files with 172 additions and 0 deletions.
1 change: 1 addition & 0 deletions docs/release_notes.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ include::include.adoc[]
** This allows the use of these classes in an OSGi environment, where the class imports in the embedded spec are not visible to the Spock OSGi bundle ClassLoader
* Fix mocking issue with the ByteBuddy MockMaker when using multiple classloaders in Java 21 spockIssue:2017[]
* Fix mocking of final classes via `@SpringBean` and `@SpringSpy` spockIssue:1960[]
* Iterable non Collection data providers are not called multiple times for `size()` anymore spockIssue:2022[]

== 2.4-M4 (2024-03-21)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,15 @@ class EmbeddedSpecRunner {
.map {new XFailure(it)}
.collect(Collectors.toList())
}

@Override
String toString() {
return """EmbeddedSpecExecutionResult:

Check warning on line 325 in spock-core/src/main/groovy/spock/util/EmbeddedSpecRunner.groovy

View check run for this annotation

Codecov / codecov/patch

spock-core/src/main/groovy/spock/util/EmbeddedSpecRunner.groovy#L325

Added line #L325 was not covered by tests
testsAbortedCount: $testsAbortedCount
testsFailedCount: $testsFailedCount
testsSkippedCount: $testsSkippedCount
testsSucceededCount: $testsSucceededCount"""
}
}

@TupleConstructor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ protected int estimateNumIterations(@Nullable Object dataProvider) {
return UNKNOWN_ITERATIONS;
}

if (dataProvider instanceof Iterable && !(dataProvider instanceof Collection)) {
//Issue #2022: Do not call size() method on an Iterable, if it is not a Collection
// because the size() call may construct the expensive iterator multiple times.
return UNKNOWN_ITERATIONS;
}

Object rawSize = GroovyRuntimeUtil.invokeMethodQuietly(dataProvider, "size");
if (!(rawSize instanceof Number)) {
return UNKNOWN_ITERATIONS;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/*
* Copyright 2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.spockframework.datapipes

import org.spockframework.EmbeddedSpecification
import spock.lang.Issue
import spock.util.EmbeddedSpecRunner

import static java.util.Objects.requireNonNull

class DataPipesIteratorSpec extends EmbeddedSpecification {
private static final ThreadLocal<Object> currentDataProvider = new ThreadLocal<>();

def before() {
compiler.addClassImport(DataPipesIteratorSpec)
}

def after() {
currentDataProvider.remove()
}

def "Collection data provider will use size method to estimate number of iterations"() {
given:
def dataCollection = dataProvider(new TestDataCollection())

when:
def res = runIterations()

then:
res.testsSucceededCount == 2
dataCollection.iteratorCalls == 1
dataCollection.sizeCalls == 2
}

@Issue("https://github.com/spockframework/spock/issues/2022")
def "Iterable shall not use a size method to estimate number of iterations"() {
given:
def dataIterable = dataProvider(new TestDataIterable())

when:
def res = runIterations()

then:
res.testsSucceededCount == 2
dataIterable.iteratorCalls == 1
dataIterable.sizeCalls == 0
}

@Issue("https://github.com/spockframework/spock/issues/2022")
def "Iterator shall be only called once"() {
given:
def dataIterator = dataProvider(new TestDataIterator())

when:
def res = runIterations()

then:
res.testsSucceededCount == 2
dataIterator.hasNextCalls == 3
dataIterator.nextCalls == 1
}

private EmbeddedSpecRunner.SummarizedEngineExecutionResults runIterations() {
runner.runSpecBody """
def "run"(Object input){
expect:
input != null
where:
//noinspection UnnecessaryQualifiedReference
input << org.spockframework.datapipes.DataPipesIteratorSpec.getDataProvider()
}
"""
}

private static <T> T dataProvider(T dataProvider) {
currentDataProvider.set(requireNonNull(dataProvider))
return dataProvider
}

static Object getDataProvider() {
def data = currentDataProvider.get()
assert data != null
return data
}

private static class TestDataCollection extends AbstractCollection {
int iteratorCalls = 0
int sizeCalls = 0

@Override
Iterator iterator() {
iteratorCalls++
return ["Value"].iterator()
}

@Override
int size() {
sizeCalls++
return 1
}
}

private static class TestDataIterable implements Iterable {
int iteratorCalls = 0
int sizeCalls = 0

int size() {
sizeCalls++
return 1
}

@Override
Iterator iterator() {
iteratorCalls++
return ["Value"].iterator()
}
}

private static class TestDataIterator implements Iterator {
int hasNextCalls = 0
int nextCalls = 0

@Override
boolean hasNext() {
hasNextCalls++
if (nextCalls == 0) {
return true
}
return false
}

@Override
Object next() {
nextCalls++
if (nextCalls == 1) {
return "Value"
}
throw new NoSuchElementException()
}
}
}

0 comments on commit d335c49

Please sign in to comment.