You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Boris Trishkin <no...@github.com> on 2017/07/07 13:28:42 UTC

[jclouds/jclouds-labs] Jclouds 46 network and image tests (#401)

JCLOUD-58 - Live test changes for ServerImageApi and NetworkApi
JCLOUD-53 - Correct Zones configuration in Mock Tests / Live Tests
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds-labs/pull/401

-- Commit Summary --

  * JCLOUD-58 - Live test changes for ServerImageApi and NetworkApi
  * JCLOUD-53 - Correct Zones configuration in Mock Tests / Live Tests

-- File Changes --

    M dimensiondata/pom.xml (7)
    M dimensiondata/src/main/java/org/jclouds/dimensiondata/cloudcontrol/features/InfrastructureApi.java (7)
    M dimensiondata/src/main/java/org/jclouds/dimensiondata/cloudcontrol/features/NetworkApi.java (26)
    M dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/features/InfrastructureApiLiveTest.java (11)
    M dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/features/InfrastructureApiMockTest.java (11)
    M dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/features/NetworkApiLiveTest.java (119)
    M dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/features/NetworkApiMockTest.java (197)
    M dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/features/ServerImageApiLiveTest.java (7)
    M dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/internal/BaseDimensionDataCloudControlApiLiveTest.java (30)
    M dimensiondata/src/test/java/org/jclouds/dimensiondata/cloudcontrol/internal/BaseDimensionDataCloudControlMockTest.java (21)

-- Patch Links --

https://github.com/jclouds/jclouds-labs/pull/401.patch
https://github.com/jclouds/jclouds-labs/pull/401.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/401

Re: [jclouds/jclouds-labs] Jclouds 46 network and image tests (#401)

Posted by Ignasi Barrera <no...@github.com>.
nacx requested changes on this pull request.

Thanks, @btrishkin and apologies for the delay in the review! We've been quite busy with the 2.0.2 release.
Just some minor comments to address.

>              DEFAULT_PRIVATE_IPV4_PREFIX_SIZE);
       assertNotNull(vlanId);
       waitForVlanStatus(api(), vlanId, State.NORMAL, 30 * 60 * 1000, "Error - unable to deploy vlan");
    }
 
    @Test
+   public void testAddPublicIPv4Block() {
+      publicIpv4BlockId = api().addPublicIpBlock(PREPARED_NETWORK_DOMAIN_ID);
+      assertNotNull(publicIpv4BlockId);
+   }
+
+   @Test(dependsOnMethods = "testAddPublicIPv4Block")
+   public void testListPublicIPv4AddressBlocks() {
+      PagedIterable<PublicIpBlock> ipBlockList = api().listPublicIPv4AddressBlocks(PREPARED_NETWORK_DOMAIN_ID);
+      assertTrue(!ipBlockList.isEmpty());
+      assertEquals(ipBlockList.last().get().first().get().size(), 2);
+      assertEquals(ipBlockList.last().get().first().get().networkDomainId(), PREPARED_NETWORK_DOMAIN_ID);
+   }
+
+   @Test(dependsOnMethods = "testListPublicIPv4AddressBlocks")

This could depend on the "add" test to maximize the chances of this test being executed.

> +
+   @Test(dependsOnMethods = "testGetPublicIPv4AddressBlocks")
+   public void testCreateNatRule() {
+      natRuleId = api()
+            .createNatRule(PREPARED_NETWORK_DOMAIN_ID, PREPARED_PRIVATE_IPV4_ADDRESS, publicIpBlock.baseIp());
+      assertNotNull(natRuleId);
+   }
+
+   @Test(dependsOnMethods = "testCreateNatRule")
+   public void testListNatRules() {
+      PagedIterable<NatRule> natRulesList = api().listNatRules(PREPARED_NETWORK_DOMAIN_ID);
+      assertTrue(!natRulesList.isEmpty());
+      assertEquals(natRulesList.last().get().first().get().networkDomainId(), PREPARED_NETWORK_DOMAIN_ID);
+   }
+
+   @Test(dependsOnMethods = "testListNatRules")

Also depend on the "add" test.

> +
+   @Test(dependsOnMethods = "testCreateNatRule")
+   public void testListNatRules() {
+      PagedIterable<NatRule> natRulesList = api().listNatRules(PREPARED_NETWORK_DOMAIN_ID);
+      assertTrue(!natRulesList.isEmpty());
+      assertEquals(natRulesList.last().get().first().get().networkDomainId(), PREPARED_NETWORK_DOMAIN_ID);
+   }
+
+   @Test(dependsOnMethods = "testListNatRules")
+   public void testGetNatRule() {
+      NatRule natRule = api().getNatRule(natRuleId);
+      assertNotNull(natRule);
+      assertEquals(natRule.networkDomainId(), PREPARED_NETWORK_DOMAIN_ID);
+   }
+
+   @Test(dependsOnMethods = "testGetNatRule")

Annotate cleanup methods with `alwaysRun = true` to make sure they're executed even the tests they depend on have failed. This way we avoid leaking resources used in tests.

> @@ -123,4 +213,13 @@ private NetworkApi api() {
       return api.getNetworkApi();
    }
 
+   private FirewallRule findById(List<FirewallRule> collection, String id) {
+      FirewallRule result = null;
+      for (FirewallRule rule : collection) {
+         if (rule.id().equals(id)) {
+            result = rule;

Just `return rule`.

> @@ -217,4 +225,15 @@ protected void assertBodyContains(RecordedRequest recordedRequest, String expect
          throw Throwables.propagate(e);
       }
    }
+
+   private List<NameValuePair> parsePath(String fullPath) {
+      return URLEncodedUtils.parse(fullPath, Charset.defaultCharset(), '?', '&');
+   }
+
+   private void assertParameters(List<NameValuePair> parsedPath, List<NameValuePair> parsedRequest) {
+      assertEquals(parsedPath.size(), parsedRequest.size());
+      for (NameValuePair parameter : parsedPath) {
+         assertTrue(parsedRequest.contains(parameter));
+      }
+   }

I don't really get the value in comparing the requests like this. Can't just we compare the strings and avoid the need of adding an additional dependency just for this?
Also, this check does not guarantee paths are the same; it does not check the order of the value pairs. Consider just comparing the requests path strings, as we do in all other providers.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/401#pullrequestreview-50242150

Re: [jclouds/jclouds-labs] Jclouds 46 network and image tests (#401)

Posted by Boris Trishkin <no...@github.com>.
Hi @nacx. Branch synchronized with master and commits squashed

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/401#issuecomment-352775820

Re: [jclouds/jclouds-labs] Jclouds 46 network and image tests (#401)

Posted by Ignasi Barrera <no...@github.com>.
nacx approved this pull request.





-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/401#pullrequestreview-80497568

Re: [jclouds/jclouds-labs] Jclouds 46 network and image tests (#401)

Posted by Ignasi Barrera <no...@github.com>.
merged to [master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/9c74d22b) and [2.0.x](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/6a25c793). Thanks!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/401#issuecomment-352986708

Re: [jclouds/jclouds-labs] Jclouds 46 network and image tests (#401)

Posted by stevegomez17 <no...@github.com>.
Great

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/401#issuecomment-352795548

Re: [jclouds/jclouds-labs] Jclouds 46 network and image tests (#401)

Posted by Ignasi Barrera <no...@github.com>.
Thanks, @btrishkin! Mind squashing the commits into a single one so I can cleanly merge the PR?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/401#issuecomment-348496470

Re: [jclouds/jclouds-labs] Jclouds 46 network and image tests (#401)

Posted by Ignasi Barrera <no...@github.com>.
Closed #401.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/401#event-1395566789

Re: [jclouds/jclouds-labs] Jclouds 46 network and image tests (#401)

Posted by Boris Trishkin <no...@github.com>.
@btrishkin pushed 2 commits.

84aabdb  Merge branch 'master' into jclouds-46-network-and-image-tests
def3271  JCLOUD-46 / JCLOUD-47 - Add missing mock / live tests for Network API


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/401/files/a003ad70b5862ca3b4379dfb412371936f2bebb7..def3271d745e3d16a9041fd66d284874dfc16a3a