Re: [jclouds/jclouds-labs] Jclouds 46 network and image tests (#401)
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)
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)
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)
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)
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)
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)
@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
Re: [jclouds/jclouds-labs] Jclouds 46 network and image tests (#401)
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 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 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 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 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 parsePath(String fullPath) { + return URLEncodedUtils.parse(fullPath, Charset.defaultCharset(), '?', '&'); + } + + private void assertParameters(List parsedPath, List 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
[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