>@nacx not sure what unit tests I could write that's not already covered. I've >not actually written any new functionality just checked for a possible NPE.
The [AllocateAndAddFloatingIpToNodeExpectTest](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNodeExpectTest.java) currently [unit tests the catch clause](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNodeExpectTest.java#L100), which is reached when the servers returns a 400 response (the [NovaErrorhandler](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/handlers/NovaErrorHandler.java#L83-L90) is the responsible of automatically translating that 400 response into the `InsufficientResourcesException`). That test covers only the case when the exception is thrown, but not when the API returns `null`. To properly verify that the class behaves properly even if the API returns `null`, a new test should be added. It should be pretty straightforward: just copy the existing test for the 400 response, but configure the response to be a 404. That will translate into a `null` return value. The rest of the test could be the same, as with your change, a `null` value should reach the same execution path than the `InsufficientResourcesException`. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/425#issuecomment-47668739
