I have a simple Java class with single method so far which fetches data from database and then does some updates, it looks like this:
@Slf4j
@Service
public class PaymentServiceImpl implements PaymentService {
private final PaymentsMapper paymentsMapper;
private final MyProperties myProperties;
public PaymentServiceImpl(PaymentsMapper paymentsMapper,
MyProperties myProperties) {
this.paymentsMapper = paymentsMapper;
this.myProperties = myProperties;
}
@Override
@Transactional
public void doSomething() {
List<String> ids = paymentsMapper.getPaymentIds(
myProperties.getPayments().getOperator(),
myProperties.getPayments().getPeriod().getDuration().getSeconds());
long updated = 0;
for (String id : ids ) {
updated += paymentsMapper.updatedPaymentsWithId(id);
}
}
}
For the record, MyProperties class is a @ConfigurationProperties
class that fetches properties from the application.properties
, it looks like this:
@Data
@Configuration("myProperties")
@ConfigurationProperties(prefix = "my")
@PropertySource("classpath:application.properties")
public class MyProperties {
private Payments payments;
@Getter
@Setter
public static class Payments {
private String operator;
private Period period;
@Getter @Setter
public static class Period{
private Duration duration;
}
}
}
Now I am trying to write a simple test for such method, I have come up with this:
class PaymentServiceImplTest extends Specification {
@Shared
PaymentsMapper paymentsMapper = Mock(PaymentsMapper)
@Shared
MyProperties properties = new MyProperties()
@Shared
PaymentServiceImpl paymentService = new PaymentServiceImpl(paymentsMapper, properties)
def setupSpec() {
properties.setPayments(new MyProperties.Payments())
properties.getPayments().setOperator('OP1')
properties.getPayments().setPeriod(new MyProperties.Payments.Period())
properties.getPayments().getPeriod().setDuration(Duration.ofSeconds(3600))
}
def 'update pending acceptation payment ids'() {
given:
paymentsMapper.getPaymentIds(_ as String, _ as long) >> Arrays.asList('1', '2', '3')
when:
paymentService.doSomething()
then:
3 * paymentsMapper.updatedPaymentsWithId(_ as String)
}
}
but trying to run the test I am getting a Null Pointer Exception:
java.lang.NullPointerException
at com.example.PaymentServiceImpl.doSomething(PaymentServiceImpl.java:33)
at com.example.service.PaymentServiceImplTest.update pending acceptation payment ids(PaymentServiceImplTest.groovy:33)
Could someone tell me why is that? Why am I getting NPE there?
My pom.xml dependencies for Spock are as follows:
<dependency>
<groupId>org.spockframework</groupId>
<artifactId>spock-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.spockframework</groupId>
<artifactId>spock-spring</artifactId>
<scope>test</scope>
</dependency>
You have several problems here:
You use @Shared
variables. You should only do that when absolutely necessary, e.g. if you need to instantiate expensive objects like DB connections. Otherwise, there is danger that context from feature method A bleeds over into B, because the shared object was modified. Then features suddenly become sensitive to execution order, which they should not be.
Your mock is also shared, but you are trying to specify stubbed results and interactions from within a feature method. This is not going to work as you might expect, if you do that in multiple feature methods. In that case, you should create a new instance for each feature, which also means that a shared variable does not make sense anymore. The only case in which it might make sense is if the exact same mock instance is used without any change in all feature methods. But then an interaction like 3 * mock.doSomething()
would continue counting across features. Also, a mock is always cheap, so why make a mock shared in the first place?
Interaction paymentsMapper.getPaymentIds(_ as String, _ as long)
will not match in your case, hence the null
default return value. The reason is the second parameter's runtime type in Groovy is Long
. So you need to change the parameter list to any of (_ as String, _ as Long)
or simpler (_ as String, _)
(_, _)
, (*_)
, depending on how specific your match needs to be.
So you could do either of the following:
Do not use @Shared
for your fields and rename setupSpec
to setup
. Very simple, the specification will not run noticeably slower because of this.
If you insist in shared variables, make sure to either set up the two mock interactions only once in the setupSpec
method or inline in the mock definition, i.e. something like
@Shared
PaymentsMapper paymentsMapper = Mock(PaymentsMapper) {
getPaymentIds(_ as String, _ as Long) >> Arrays.asList('1', '2', '3')
3 * updatedPaymentsWithId(_ as String)
}
// ...
def 'update pending acceptation payment ids'() {
expect:
paymentService.doSomething()
}
But then the mock interactions are outside of the feature method, which might make readers wonder what the feature method actually does. So, technically this works, but an easily readable test looks different IMO.
You also might have the idea to instantiate and configure the shared mock in each feature method. But then the one-time assignment to @Shared PaymentServiceImpl paymentService
would use another instance or null
. Uh-oh! Do you see the problem with shared mocks? I guess, it is not worth your premature optimisation, because this is what I believe it is.