You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by danbroudy <no...@github.com> on 2014/11/21 20:00:19 UTC
[jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
...tTest
What do you think of the format of the ImageApiMockTest?
If it looks good, ill start converting others to a similar format.
You can merge this Pull Request by running:
git pull https://github.com/GoogleCloudPlatform/jclouds-labs-google imageDeprecate
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs-google/pull/98
-- Commit Summary --
* Added Image.deprecate, ImageApiMockTest completed, removed ImageApiExpectTest
-- File Changes --
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/functions/GoogleComputeEngineImageToImage.java (2)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Deprecated.java (11)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/ImageApi.java (16)
M google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/options/DeprecateOptions.java (11)
D google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/ImageApiExpectTest.java (184)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/ImageApiLiveTest.java (30)
A google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/ImageApiMockTest.java (143)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/internal/BaseGoogleComputeEngineApiMockTest.java (4)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseImageListTest.java (9)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseImageTest.java (12)
M google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseOperationTest.java (13)
A google-compute-engine/src/test/resources/image_deprecate.json (1)
-- Patch Links --
https://github.com/jclouds/jclouds-labs-google/pull/98.patch
https://github.com/jclouds/jclouds-labs-google/pull/98.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Adrian Cole <no...@github.com>.
couple style notes. otherwise lgtm
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64021541
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Adrian Cole <no...@github.com>.
> @@ -59,7 +59,7 @@
> osBuilder.version(version);
>
> if (image.deprecated() != null) {
> - builder.userMetadata(ImmutableMap.of("deprecatedState", image.deprecated().state()));
> + builder.userMetadata(ImmutableMap.of("deprecatedState", image.deprecated().state().toString()));
I think toString handles the casing, which is why this needed to be changed. @danbroudy can you add a test to make sure this happens? Make GoogleComputeEngineImageToImageTest and pass an input image which is deprecated.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20819254
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Adrian Cole <no...@github.com>.
DELETED maps across fine.
DEPRECATED and OBSELETE are still AVAILABLE, so I'd use that. We can later introduce an enum for these concepts, if they are portable.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64427098
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #315](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/315/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64449204
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1785](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1785/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64449942
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by danbroudy <no...@github.com>.
Looks the same as always
```
Results :
Failed tests:
GoogleComputeEngineServiceLiveTest>BaseComputeServiceLiveTest.testCreateAndRunAService:706->BaseComputeServiceLiveTest.createAndRunAServiceInGroup:723 » RunNodes
GoogleComputeEngineServiceLiveTest>BaseComputeServiceLiveTest.testAScriptExecutionAfterBootWithBasicTemplate:223 » RunNodes
GoogleComputeEngineServiceLiveTest>BaseComputeServiceLiveTest.testConcurrentUseOfComputeServiceToCreateNodes:475 » Execution
Tests run: 145, Failures: 3, Errors: 0, Skipped: 15
```
@adriancole anything else? good to go?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64240258
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1779](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1779/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64021340
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by danbroudy <no...@github.com>.
> @@ -59,7 +59,7 @@
> osBuilder.version(version);
>
> if (image.deprecated() != null) {
> - builder.userMetadata(ImmutableMap.of("deprecatedState", image.deprecated().state()));
> + builder.userMetadata(ImmutableMap.of("deprecatedState", image.deprecated().state().toString()));
I did change to `name()` because it seems applicable reading the description.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20823331
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Adrian Cole <no...@github.com>.
ps don't forget to squash
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64427163
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by danbroudy <no...@github.com>.
> @@ -0,0 +1 @@
> +{"state":"DEPRECATED","replacement":"https://www.googleapis.com/compute/v1/projects/centos-cloud/global/images/centos-6-2-v20120326test","deprecated":"2014-07-16T22:16:13.468Z","obsolete":"2014-10-16T22:16:13.468Z","deleted":"2015-01-16T22:16:13.468Z"}
Awesome! I am totally using that now.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20820559
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Adrian Cole <no...@github.com>.
One gap wrt GoogleComputeEngineImageToImageTest. When done == +1 mergey! Thanks again!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64258227
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by danbroudy <no...@github.com>.
Looks like you cannot create new instances with an OBSELETE image.
@adriancole Is it fine to map to AVAILABLE and we let the user learn from the error or does it make it more of a UNRECOGNIZED or DELETED. What do you think?
>From the [website](https://cloud.google.com/compute/docs/reference/latest/images/deprecate):
"OBSOLETE": Existing instances already using an OBSOLETE image can continue to do so, but you cannot create new resources with this image. Google Compute Engine returns an error if you attempt to use the image for a new resource and recommends a replacement.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64447802
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by danbroudy <no...@github.com>.
Running the live tests now
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64235468
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Andrew Phillips <no...@github.com>.
> + }
> +
> + public void deleteImage_2xx() throws Exception {
> + server.enqueue(jsonResponse("/operation.json"));
> +
> + assertEquals(imageApi().delete("centos-6-2-v20120326"),
> + new ParseOperationTest().expected(url("/projects")));
> + assertSent(server, "DELETE", "/projects/party/global/images/centos-6-2-v20120326");
> + }
> +
> + public void deleteImage_4xx() throws Exception {
> + server.enqueue(response404());
> +
> + assertNull(imageApi().delete("centos-6-2-v20120326"));
> + assertSent(server, "DELETE", "/projects/party/global/images/centos-6-2-v20120326");
> + }
@adriancole I recall a recent comment of yours about not testing default fallbacks...is that relevant here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20816274
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by danbroudy <no...@github.com>.
> @@ -104,6 +104,10 @@ protected MockResponse jsonResponse(String resource) {
> return new MockResponse().addHeader("Content-Type", "application/json").setBody(stringFromResource(resource));
> }
>
> + protected MockResponse response404(){
> + return new MockResponse().setStatus("HTTP/1.1 404 Not Found");
fixed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20823368
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Adrian Cole <no...@github.com>.
> +import org.jclouds.date.internal.SimpleDateFormatDateService;
> +import org.jclouds.googlecomputeengine.domain.Deprecated.State;
> +import org.jclouds.googlecomputeengine.internal.BaseGoogleComputeEngineApiMockTest;
> +import org.jclouds.googlecomputeengine.options.DeprecateOptions;
> +import org.jclouds.googlecomputeengine.parse.ParseImageListTest;
> +import org.jclouds.googlecomputeengine.parse.ParseImageTest;
> +import org.jclouds.googlecomputeengine.parse.ParseOperationTest;
> +import org.testng.annotations.BeforeMethod;
> +import org.testng.annotations.Test;
> +
> +@Test(groups = "unit", testName = "ImageApiMockTest", singleThreaded = true)
> +public class ImageApiMockTest extends BaseGoogleComputeEngineApiMockTest {
> +
> + @BeforeMethod
> + @Override
> + public void start() throws IOException {
this is a little complicated, as we don't need to hook into lifecycle. just api().images() directly, or add a method at the end.
```java
ImageApi images(){
return api().images();
}
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20734473
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #311](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/311/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64025952
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by danbroudy <no...@github.com>.
Just pushed a squashed version that maps Obsolete -> Available.
Happy to update if you think something else is more applicable.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64449017
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #310](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/310/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64020804
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1782](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1782/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64267903
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Andrew Phillips <no...@github.com>.
> @@ -59,7 +59,7 @@
> osBuilder.version(version);
>
> if (image.deprecated() != null) {
> - builder.userMetadata(ImmutableMap.of("deprecatedState", image.deprecated().state()));
> + builder.userMetadata(ImmutableMap.of("deprecatedState", image.deprecated().state().toString()));
Quick question: do we want the `toString()` or the `name()` here? The contract for `name()` is a little narrower than `toString()`, which could change to be "prettier", I guess?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20816129
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by danbroudy <no...@github.com>.
> + }
> +
> + public void deleteImage_2xx() throws Exception {
> + server.enqueue(jsonResponse("/operation.json"));
> +
> + assertEquals(imageApi().delete("centos-6-2-v20120326"),
> + new ParseOperationTest().expected(url("/projects")));
> + assertSent(server, "DELETE", "/projects/party/global/images/centos-6-2-v20120326");
> + }
> +
> + public void deleteImage_4xx() throws Exception {
> + server.enqueue(response404());
> +
> + assertNull(imageApi().delete("centos-6-2-v20120326"));
> + assertSent(server, "DELETE", "/projects/party/global/images/centos-6-2-v20120326");
> + }
`Images:delete` has ` @Fallback(NullOnNotFoundOr404.class)`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20820296
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Adrian Cole <no...@github.com>.
> + }
> +
> + public void deleteImage_2xx() throws Exception {
> + server.enqueue(jsonResponse("/operation.json"));
> +
> + assertEquals(imageApi().delete("centos-6-2-v20120326"),
> + new ParseOperationTest().expected(url("/projects")));
> + assertSent(server, "DELETE", "/projects/party/global/images/centos-6-2-v20120326");
> + }
> +
> + public void deleteImage_4xx() throws Exception {
> + server.enqueue(response404());
> +
> + assertNull(imageApi().delete("centos-6-2-v20120326"));
> + assertSent(server, "DELETE", "/projects/party/global/images/centos-6-2-v20120326");
> + }
I believe there is a fallback to null here, right?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20817780
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Adrian Cole <no...@github.com>.
> + public void start() throws IOException {
> + super.start();
> + imageApi = api().images();
> + }
> +
> + ImageApi imageApi;
> +
> + public void get() throws Exception {
> + server.enqueue(jsonResponse("/image_get.json"));
> +
> + assertEquals(imageApi.get(URI.create(url("/projects/party/global/images/centos-6-2-v20120326"))),
> + new ParseImageTest().expected(url("/projects")));
> + assertSent(server, "GET", "/projects/party/global/images/centos-6-2-v20120326");
> + }
> +
> + public void getResponseIs4xx() throws Exception {
I've been switching to method_code convention lately.
Ex.
get, get_4xx
list, list_empty
createFromDisk
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20734593
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Adrian Cole <no...@github.com>.
Closed #98.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#event-198467207
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Andrew Phillips <no...@github.com>.
> @@ -104,6 +104,10 @@ protected MockResponse jsonResponse(String resource) {
> return new MockResponse().addHeader("Content-Type", "application/json").setBody(stringFromResource(resource));
> }
>
> + protected MockResponse response404(){
> + return new MockResponse().setStatus("HTTP/1.1 404 Not Found");
[minor] Indent (here and elsewhere). But we can clean this up during merging - no need to update the PR if you ask me.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20816330
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Adrian Cole <no...@github.com>.
> @@ -0,0 +1 @@
> +{"state":"DEPRECATED","replacement":"https://www.googleapis.com/compute/v1/projects/centos-cloud/global/images/centos-6-2-v20120326test","deprecated":"2014-07-16T22:16:13.468Z","obsolete":"2014-10-16T22:16:13.468Z","deleted":"2015-01-16T22:16:13.468Z"}
it is possible to use "jq" or similar to wrap these, as the tests will
tolerate it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20817835
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs-google #1780](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-google/1780/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64026575
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Andrew Phillips <no...@github.com>.
> @@ -0,0 +1 @@
> +{"state":"DEPRECATED","replacement":"https://www.googleapis.com/compute/v1/projects/centos-cloud/global/images/centos-6-2-v20120326test","deprecated":"2014-07-16T22:16:13.468Z","obsolete":"2014-10-16T22:16:13.468Z","deleted":"2015-01-16T22:16:13.468Z"}
Any way of wrapping/formatting this to make it easier to read? Or does it need to be on one line for the test?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98/files#r20816372
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Andrew Phillips <no...@github.com>.
Thanks, @danbroudy! A couple of questions, but all minors. +1 - good to go for me too!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64252295
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by Adrian Cole <no...@github.com>.
merged!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64451238
Re: [jclouds-labs-google] Added Image.deprecate, ImageApiMockTest
completed, removed ImageApiExpec... (#98)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-labs-google-pull-requests #312](https://jclouds.ci.cloudbees.com/job/jclouds-labs-google-pull-requests/312/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-google/pull/98#issuecomment-64267422