You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by jasminSPC <no...@github.com> on 2015/03/03 12:05:22 UTC
[jclouds-labs] Profitbricks Drives API (#146)
In this PR I also made changes to Location enum which was missing one enumerator US_LAS_DEV ("us/lasdev") and also I had to fix ImageListResponseHandlerTest.java so it would pass.
//cc @baldwinSPC
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/146
-- Commit Summary --
* Profitbricks Drives API
-- File Changes --
M profitbricks/src/main/java/org/jclouds/profitbricks/ProfitBricksApi.java (4)
A profitbricks/src/main/java/org/jclouds/profitbricks/binder/drive/AddRomDriveToServerRequestBinder.java (45)
A profitbricks/src/main/java/org/jclouds/profitbricks/domain/Drive.java (70)
M profitbricks/src/main/java/org/jclouds/profitbricks/domain/Location.java (1)
A profitbricks/src/main/java/org/jclouds/profitbricks/features/DrivesApi.java (51)
A profitbricks/src/test/java/org/jclouds/profitbricks/binder/drive/AddRomDriveToServerRequestBinderTest.java (51)
A profitbricks/src/test/java/org/jclouds/profitbricks/features/DrivesApiLiveTest.java (72)
A profitbricks/src/test/java/org/jclouds/profitbricks/features/DrivesApiMockTest.java (81)
M profitbricks/src/test/java/org/jclouds/profitbricks/http/parser/image/ImageListResponseHandlerTest.java (6)
A profitbricks/src/test/resources/drives/drives-add.xml (12)
A profitbricks/src/test/resources/drives/drives-remove.xml (12)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/146.patch
https://github.com/jclouds/jclouds-labs/pull/146.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/146
Re: [jclouds-labs] Profitbricks Drives API (#146)
Posted by jasminSPC <no...@github.com>.
This has been squashed, and ready to go.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/146#issuecomment-77446711
Re: [jclouds-labs] Profitbricks Drives API (#146)
Posted by Ignasi Barrera <no...@github.com>.
> + public String imageId;
> +
> + @Override
> + protected void initialize() {
> + super.initialize();
> +
> + List<Server> servers = api.serverApi().getAllServers();
> + assertNotNull(servers, "At least one server is required to run drives test.");
> +
> + Server server = Iterables.getFirst(servers, null);
> + assertNotNull(server);
> +
> + this.serverId = server.id();
> +
> + List<Image> images = api.imageApi().getAllImages();
> + assertNotNull(images, "At least one image is required to run drives test.");
Same comment about null lists.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/146/files#r25730311
Re: [jclouds-labs] Profitbricks Drives API (#146)
Posted by Ignasi Barrera <no...@github.com>.
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.jclouds.profitbricks.features;
> +
> +import com.squareup.okhttp.mockwebserver.MockResponse;
> +import com.squareup.okhttp.mockwebserver.MockWebServer;
> +import org.jclouds.profitbricks.ProfitBricksApi;
> +import org.jclouds.profitbricks.domain.Drive;
> +import org.jclouds.profitbricks.internal.BaseProfitBricksMockTest;
> +import static org.jclouds.profitbricks.internal.BaseProfitBricksMockTest.mockWebServer;
> +import static org.testng.Assert.assertNotNull;
> +import org.testng.annotations.Test;
> +
> +@Test(groups = "unit", testName = "DataCenterApiMockTest")
Fix the test name
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/146/files#r25730431
Re: [jclouds-labs] Profitbricks Drives API (#146)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +@Test(groups = "unit", testName = "AddRomDriveToServerRequestBinderTest")
> +public class AddRomDriveToServerRequestBinderTest {
> +
> + @Test
> + public void testAddRomDriveToServerPayload() {
> + AddRomDriveToServerRequestBinder binder = new AddRomDriveToServerRequestBinder();
> +
> + Drive.Request.AddRomDriveToServerPayload payload = Drive.Request.AddRomDriveToServerPayload.builder()
> + .serverId("server-id")
> + .storageId("image-id")
> + .deviceNumber("device-number")
> + .build();
> +
> + String actual = binder.createPayload(payload);
> + assertNotNull(actual, "Binder returned null payload");
nit: This is redundant, the next check will also fail as the expected payload is not null.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/146/files#r25730216
Re: [jclouds-labs] Profitbricks Drives API (#146)
Posted by Ignasi Barrera <no...@github.com>.
Pushed to master as [e6305c26](https://git1-us-west.apache.org/repos/asf?p=jclouds-labs.git;a=commit;h=e6305c26). Thanks @jasminSPC!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/146#issuecomment-77772187
Re: [jclouds-labs] Profitbricks Drives API (#146)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.profitbricks.domain.Server;
> +import static org.testng.Assert.assertNotNull;
> +import org.testng.annotations.Test;
> +
> +@Test(groups = "live", testName = "DrivesApiLiveTest", singleThreaded = true)
> +public class DrivesApiLiveTest extends BaseProfitBricksLiveTest {
> +
> + public String serverId;
> + public String imageId;
> +
> + @Override
> + protected void initialize() {
> + super.initialize();
> +
> + List<Server> servers = api.serverApi().getAllServers();
> + assertNotNull(servers, "At least one server is required to run drives test.");
Remove this assertion, as the server api never returns null. That's why it uses the fallback to an empty list. In jclouds we try to avoid returning null lists, so this is a pattern you can apply to the rest of the code.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/146/files#r25730295
Re: [jclouds-labs] Profitbricks Drives API (#146)
Posted by Ignasi Barrera <no...@github.com>.
Most comments are minors. Thanks @jasminSPC!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/146#issuecomment-77043936
Re: [jclouds-labs] Profitbricks Drives API (#146)
Posted by Ignasi Barrera <no...@github.com>.
> + assertNotNull(image);
> +
> + this.imageId = image.id();
> + }
> +
> + @Test
> + public void addRomDriveToServerTest() {
> + String requestId = api.drivesApi().addRomDriveToServer(Drive.Request.AddRomDriveToServerPayload.builder()
> + .serverId(serverId)
> + .storageId("05cadf29-6c12-11e4-beeb-52540066fee9")
> + .deviceNumber("0")
> + .build());
> + assertNotNull(requestId);
> + }
> +
> + @Test //(dependsOnMethods = "addRomDriveToServerTest")
Should the test dependency be uncommented?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/146/files#r25730359