You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Andrew Gaul <no...@github.com> on 2013/09/05 02:53:58 UTC
[jclouds] Delete containers after integration tests (#143)
Also return newly allocated containers to the pool.
You can merge this Pull Request by running:
git pull https://github.com/maginatics/jclouds delete-container-integration-tests
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/143
-- Commit Summary --
* Delete containers after integration tests
-- File Changes --
M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseBlobStoreIntegrationTest.java (12)
M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerIntegrationTest.java (5)
M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerLiveTest.java (4)
M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseServiceIntegrationTest.java (2)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/143.patch
https://github.com/jclouds/jclouds/pull/143.diff
Re: [jclouds] Delete containers after integration tests (#143)
Posted by Andrew Gaul <no...@github.com>.
> @@ -101,6 +102,15 @@ public void setUpResourcesForAllThreads(ITestContext testContext) throws Excepti
> view.close();
> view = null;
> }
> +
> + @AfterSuite(groups = { "integration", "live" })
> + protected void destroyResources() throws Exception {
> + setupContext();
You can see immediately above that BeforeSuite destroys the context (view).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143/files#r6172575
Re: [jclouds] Delete containers after integration tests (#143)
Posted by Andrew Gaul <no...@github.com>.
> @@ -101,6 +102,15 @@ public void setUpResourcesForAllThreads(ITestContext testContext) throws Excepti
> view.close();
> view = null;
> }
> +
> + @AfterSuite(groups = { "integration", "live" })
> + protected void destroyResources() throws Exception {
> + setupContext();
Yes these are different contexts. Seems like we could reuse them throughout, although this can follow in a subsequent commit.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143/files#r6253269
Re: [jclouds] Delete containers after integration tests (#143)
Posted by Andrew Gaul <no...@github.com>.
Pushed to master and 1.6.x.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143#issuecomment-24124217
Re: [jclouds] Delete containers after integration tests (#143)
Posted by Andrew Gaul <no...@github.com>.
> @@ -101,6 +102,15 @@ public void setUpResourcesForAllThreads(ITestContext testContext) throws Excepti
> view.close();
> view = null;
> }
> +
> + @AfterSuite(groups = { "integration", "live" })
> + protected void destroyResources() throws Exception {
> + setupContext();
> + deleteEverything(view);
> +
> + view.close();
> + view = null;
Mimicking BeforeSuite exactly.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143/files#r6172578
Re: [jclouds] Delete containers after integration tests (#143)
Posted by Andrew Gaul <no...@github.com>.
> @@ -101,6 +102,15 @@ public void setUpResourcesForAllThreads(ITestContext testContext) throws Excepti
> view.close();
> view = null;
> }
> +
> + @AfterSuite(groups = { "integration", "live" })
> + protected void destroyResources() throws Exception {
> + setupContext();
> + deleteEverything(view);
> +
> + view.close();
> + view = null;
Will follow up with the finally block in a separate commit.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143/files#r6253279
Re: [jclouds] Delete containers after integration tests (#143)
Posted by Andrew Phillips <no...@github.com>.
> @@ -101,6 +102,15 @@ public void setUpResourcesForAllThreads(ITestContext testContext) throws Excepti
> view.close();
> view = null;
> }
> +
> + @AfterSuite(groups = { "integration", "live" })
> + protected void destroyResources() throws Exception {
> + setupContext();
> + deleteEverything(view);
> +
> + view.close();
> + view = null;
Why would this be required?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143/files#r6171612
Re: [jclouds] Delete containers after integration tests (#143)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #200](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/200/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143#issuecomment-23838941
Re: [jclouds] Delete containers after integration tests (#143)
Posted by Andrew Phillips <no...@github.com>.
> @@ -101,6 +102,15 @@ public void setUpResourcesForAllThreads(ITestContext testContext) throws Excepti
> view.close();
> view = null;
> }
> +
> + @AfterSuite(groups = { "integration", "live" })
> + protected void destroyResources() throws Exception {
> + setupContext();
> + deleteEverything(view);
> +
> + view.close();
> + view = null;
I guess BeforeSuite has to be a bit more careful not to pollute the coming tests but it can't hurt here, either. Thanks for explaining.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143/files#r6172716
Re: [jclouds] Delete containers after integration tests (#143)
Posted by Andrew Phillips <no...@github.com>.
> @@ -101,6 +102,15 @@ public void setUpResourcesForAllThreads(ITestContext testContext) throws Excepti
> view.close();
> view = null;
> }
> +
> + @AfterSuite(groups = { "integration", "live" })
> + protected void destroyResources() throws Exception {
> + setupContext();
> + deleteEverything(view);
> +
> + view.close();
> + view = null;
Would it make sense to extract a "doWithContext(Callable...)" or Runnable or whatever here, that additionally wraps a `try {...} finally...` around the operation to be performed?
I guess in both BeforeSuite and AfterSuite, we'd want to close and null out the view even if the operation itself (e.g. "deleteEverything" in the AfterSuite case) fails..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143/files#r6172747
Re: [jclouds] Delete containers after integration tests (#143)
Posted by Andrew Phillips <no...@github.com>.
> @@ -101,6 +102,15 @@ public void setUpResourcesForAllThreads(ITestContext testContext) throws Excepti
> view.close();
> view = null;
> }
> +
> + @AfterSuite(groups = { "integration", "live" })
> + protected void destroyResources() throws Exception {
> + setupContext();
Ah, OK...that's not the actual context that the tests will use, that's a "temporary" context created purely to set up (or, in this case, tear down) resources..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143/files#r6172708
Re: [jclouds] Delete containers after integration tests (#143)
Posted by Andrew Gaul <no...@github.com>.
Pushed to master. We do not seem to have a proper live test for this; I will backfill this later.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143#issuecomment-24123868
Re: [jclouds] Delete containers after integration tests (#143)
Posted by Andrew Phillips <no...@github.com>.
> @@ -101,6 +102,15 @@ public void setUpResourcesForAllThreads(ITestContext testContext) throws Excepti
> view.close();
> view = null;
> }
> +
> + @AfterSuite(groups = { "integration", "live" })
> + protected void destroyResources() throws Exception {
> + setupContext();
Hasn't the context already been created at this point? Or are we envisaging a situation in which the `@BeforeSuite` hasn't been run?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143/files#r6171604
Re: [jclouds] Delete containers after integration tests (#143)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #661](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/661/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/143#issuecomment-23838852