First of all thanks for the contribution @lahirus! There were already many live tests failing in master, so I assume those failures aren't related to your changes. However, before merging this pull requests, some unit tests that assert your additions should be added to the following classes
* *BindLaunchSpecificationToFormParamsTest* * *CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsTest* (simply add a method that tests the `addSecurityGroups` method) * *AWSRunInstancesOptionsTest* * *SpotInstanceApiExpectTest* There are no tests for the `requestSpotInstancesInRegion` (our fault), but having a method that tests it with a `LaunchSpecification` that uses your additions is important. Expect tests are end-to-end (mock-based) tests that verify the entire mechanism is working. If the test works, we know the REST API calls are being properly generated. * *LaunchSpecificationHandlerTest* This test class does not exist, and I understand it is not in the scope of this PR to add all missing tests. However, if you could add a test there to verify the behavior of your concrete additions, that would be great. We rely a lot on unit/live tests before releasing, specially when one supports such an amount of providers. It is important to add tests to every change so we don't break them unintentionally. Could you kindly add the relevant unit tests? Thanks again @lahirus! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/333#issuecomment-40567607
