Verizon / remotely

An elegant RPC system for reasonable people
http://verizon.github.io/remotely/
Apache License 2.0
306 stars 39 forks source link

Relying on the order of ScalaCheck property evaluation is unsafe #117

Open larsrh opened 7 years ago

larsrh commented 7 years ago

RemoteSpec relies on the order of ScalaCheck property evaluation in order to clean up the pool in the end. Here's what happens after upgrading to ScalaCheck 1.13.x:

[info] + Remote.roundtrip[List[Int]]: OK, passed 100 tests.
[info] Elapsed time: 0.630 sec 
[info] + Remote.check-serializers: OK, proved property.
[info] Elapsed time: 0.000 sec 
[info] + Remote.add3: OK, passed 100 tests.
[info] Elapsed time: 0.214 sec 
[info] + Remote.roundtrip: OK, passed 100 tests.
[info] Elapsed time: 0.307 sec 
[info] + Remote.check-declarations: OK, proved property.
[info] Elapsed time: 0.000 sec 
[info] + Remote.cleanup: OK, proved property.
[info] Elapsed time: 0.002 sec 
[info] ! Remote.roundtrip[Double]: Exception raised on property evaluation.
[info] > ARG_0: List()
[info] > ARG_1: Map()
[info] > Exception: java.lang.IllegalStateException: Pool not open
[info] org.apache.commons.pool2.impl.BaseGenericObjectPool.assertOpen(BaseGenericObjectPool.java:672)
[info] org.apache.commons.pool2.impl.GenericObjectPool.borrowObject(GenericObjectPool.java:412)
[info] org.apache.commons.pool2.impl.GenericObjectPool.borrowObject(GenericObjectPool.java:363)
[info] remotely.transport.netty.NettyTransport$$anonfun$1.apply(Client.scala:36)
[info] remotely.transport.netty.NettyTransport$$anonfun$1.apply(Client.scala:35)
(more stack trace ...)

This blocks an upgrade to ScalaCheck 1.13.x. It can be easily fixed by converting it into a ScalaTest suite. See larsrh/remotely@30b0ae6fdefe3f3cf4c76162f037041cb801c3a2 for an implementation.