spock icon indicating copy to clipboard operation
spock copied to clipboard

Iterable data providers are not called multiple times for size() anymore

Open AndreasTu opened this issue 1 year ago • 3 comments

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

AndreasTu avatar Oct 18 '24 16:10 AndreasTu

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.86%. Comparing base (2c7db77) to head (e026689). Report is 157 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2027      +/-   ##
============================================
+ Coverage     80.44%   81.86%   +1.41%     
- Complexity     4337     4609     +272     
============================================
  Files           441      448       +7     
  Lines         13534    14445     +911     
  Branches       1707     1829     +122     
============================================
+ Hits          10888    11826     +938     
+ Misses         2008     1942      -66     
- Partials        638      677      +39     

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

codecov[bot] avatar Oct 18 '24 17:10 codecov[bot]

@Vampire But why should we call a size() method on an Iterable, which is not part of the contract for Iterable. On Collections there is the size() method contract, which we can argument "hey then apply a cache", but the call to DefaultGroovyMethods.size(Iterable) is IMHO very surprising, which leads to 3 iterations of the iterable, where the user did not define it.

I think the size() method on an iterable is a Groovy idiosyncrasy, which we should not force onto users.

AndreasTu avatar Oct 18 '24 17:10 AndreasTu

But why should we call a size() method on an Iterable, which is not part of the contract for Iterable.

Spock specs are Groovy code, not Java code. In Groovy size() is part of the contract of Iterable: https://docs.groovy-lang.org/latest/html/groovy-jdk/java/lang/Iterable.html#size()

I think the size() method on an iterable is a Groovy idiosyncrasy, which we should not force onto users.

I'm totally on the opposite actually. We are in Groovy environment, so we should follow Groovy semantics. And for Groovy the method is defined and thus executed.

@leonard84 what is your PoV?

Vampire avatar Oct 18 '24 17:10 Vampire