QuickTip (Spock): Don't use .each in a then block!!!

This happens time and time again. QA finds a bug and I swear I wrote a test for that condition. So, I go back to try to debug that test.  I see the original test I wrote and the condition that should fail.  It looks something like this:

 when:  
 List<SomeType> someResults = service.someAction(someParam)  
 
 then:  
 someResults // assert the result is not null, empty list, etc  
 someResults.each {  
   it.someFieldName == 'someValue'  
   it.someOtherFieldName == 1  
 }  

In this case, someOtherFieldName is empty but the test still passes... wtf?! right?

Well, this is actually the expected behavior.

Because each doesn't actually return anything from the closure but just returns the collection itself, Spock thinks it is fine.  It doesn't care about the lines within the each block and whether they fail or not.

So we have a few choices here:
  • use .every (only applies to the last line)
 when:  
 List<SomeType> someResults = service.someAction(someParam)  
 
 then:  
 someResults  
 someResults.every {  
   it.someFieldName == 'someValue'  
   it.someOtherFieldName == 1  
 }  

'Every' asserts that the last condition inside the closure holds true. So in this case, it never asserts that someFieldName is equal to 'someValue'. Therefore, it is better to use explicit assertions.
  • add assertions for each line inside the each
 when:
 List<SomeType> someResults = service.someAction(someParam)

 then:
 someResults  
 someResults.each {  
   assert it.someFieldName == 'someValue'  
   assert it.someOtherFieldName == 1  
 }  

This is a bit more tedious and prone to errors if you skip one line so make sure you don't forget! That's why Spock has a special method to handle this case.
  • use Spock's with method
 when:
 List<SomeType> someResults = service.someAction(someParam)

 then:  
 with(someResults) {  
   someFieldName == 'someValue'  
   someOtherFieldName == 1  
 } 

There is one gotcha here though. Be careful when assigning local variables with the same name as the field name. For example,

 given:
 String someFieldName = 'someOtherValue' 

 when:
 List<SomeType> someResults = service.someAction(someParam)

 then:  
 with(someResults) {  
   someFieldName == 'someValue'  
   someOtherFieldName == 1  
 }

The local variable will take precedence and this example would fail even if someResults.someFieldName is equal to 'someValue. If you absolutely must name the local variable the same as the fieldName, you can use it.someFieldName to target the one on the result.

 given:
 String someFieldName = 'someOtherValue'

 when:
 List<SomeType> someResults = service.someAction(someParam)

 then:  
 with(someResults) {  
   it.someFieldName == 'someValue'  
   it.someOtherFieldName == 1
 }

Because of this behavior, some people don't like using with.

Conclusion

Pick the approach that is best for your team. Personally, I prefer using with or adding asserts to every line. No matter which one you pick, know the limitations and make sure you don't make the same mistake over and over again.

If you have another approach you prefer, add a comment below.

Comments