You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by kstyrc <no...@github.com> on 2014/03/18 19:57:00 UTC
[jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
You can merge this Pull Request by running:
git pull https://github.com/kstyrc/jclouds master
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/320
-- Commit Summary --
* JCLOUDS-184: Improving AzureBlob unit tests
* JCLOUDS-184: Improving AzureBlob unit tests
-- File Changes --
M providers/azureblob/src/test/java/org/jclouds/azureblob/blobstore/strategy/AzureBlobBlockUploadStrategyTest.java (39)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/320.patch
https://github.com/jclouds/jclouds/pull/320.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
>
> import java.util.List;
>
> -import static org.easymock.EasyMock.anyObject;
> -import static org.easymock.EasyMock.createMock;
> -import static org.easymock.EasyMock.eq;
> -import static org.easymock.EasyMock.expect;
> -import static org.easymock.EasyMock.expectLastCall;
> -import static org.easymock.EasyMock.replay;
> +import static org.easymock.EasyMock.*;
Please avoid wildcard imports
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320/files#r10731983
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
> @@ -70,5 +65,29 @@ public void testExecute() throws Exception {
> replay(slicer,client);
> String etag = strat.execute(container, blob);
> assertEquals(etag, "Fake ETAG");
> +
> + verify(slicer, client);
What is the benefit of this? Are we interested in how often exactly the slicer is called? For the client, yes, I can see the point of this (unexpected/unnecessary API calls could be expensive). But I don't see how/why we care about calls to the slicer?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320/files#r10732089
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
> public class AzureBlobBlockUploadStrategyTest {
>
> + @Test(groups = "unit", testName = "AzureBlobBlockUploadStrategyTest")
Why has this moved here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320/files#r10731992
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1142](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1142/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38097306
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #676](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/676/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38221041
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1306](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1306/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-59924337
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
> +
> + @Test(expectedExceptions = IllegalArgumentException.class)
> + public void testExceededContentLengthLimit() throws Exception {
> + String container = "test-container";
> + String blobName = "test-blob";
> +
> + AzureBlobClient client = createNiceMock(AzureBlobClient.class);
> + PayloadSlicer slicer = createNiceMock(PayloadSlicer.class);
> +
> + MutableBlobMetadata metadata = new MutableBlobMetadataImpl();
> + MutableContentMetadata contentMetadata = new BaseMutableContentMetadata();
> + contentMetadata.setContentLength(MultipartUploadStrategy.MAX_BLOCK_SIZE * MultipartUploadStrategy.MAX_NUMBER_OF_BLOCKS + 1);
> + metadata.setName(blobName);
> + metadata.setContentMetadata(contentMetadata);
> + Blob blob = new BlobImpl(metadata);
> + Payload payload = new StringPayload("ABCD");
Whoops, missed that! Thanks for spotting, @andrewgaul! Can also be changed in `testExecute`, please. Code change should be as simple as:
```
ByteSource bytes = ByteSource.wrap("ABCD".getBytes(Charsets.UTF_8));
Payload payload = new ByteSourcePayload(bytes);
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320/files#r10776838
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
> @@ -70,5 +73,29 @@ public void testExecute() throws Exception {
> replay(slicer,client);
[minor] Add a space before "client"? I know it's not your code, but a bit of cleanup can't hurt ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320/files#r10761433
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Everett Toews <no...@github.com>.
It's release time in jclouds and that means we like to do a little house cleaning by sweeping out the pull request queue. This PR hasn't been touched in over 6 months. Please give us a status update.
If we don't hear anything by the end of the week, we'll close this pull request.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-59872253
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #931](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/931/) 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/pull/320#issuecomment-38225969
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Everett Toews <no...@github.com>.
Closed #320.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#event-181573216
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
Almost there...thanks for the update, @kstyrc!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38083633
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1137](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1137/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-37979552
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1141](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1141/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38092384
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #925](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/925/) 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/pull/320#issuecomment-38090439
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1140](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1140/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38087104
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
> @@ -47,7 +50,7 @@ public void testExecute() throws Exception {
> String blobName = "test-blob";
> long oneMB = 1048576L;
> AzureBlobClient client = createMock(AzureBlobClient.class);
> - PayloadSlicer slicer = createMock(PayloadSlicer.class);
> + PayloadSlicer slicer = createStrictMock(PayloadSlicer.class);
Hm...would it make more sense to make the one we want to _verify_ strict (i.e. the client), and leave the _other_ one as a regular mock?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320/files#r10761415
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
> Unfortunately, I'm more familiar with Mockito rather than EasyMock..
A change I'm sure we'd like to make too ;-) Thanks for the updates!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38085146
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #924](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/924/) 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/pull/320#issuecomment-38084585
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1816](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1816/) 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/pull/320#issuecomment-59928210
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by kstyrc <no...@github.com>.
Hmm, from my side I've improved the test, but nobody pushed it forward?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-59883069
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
Pending the last PR builder, +1 - looks good to me. Could you squash and rebase your changes so we can merge this? Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38085273
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by kstyrc <no...@github.com>.
Thanks for the useful comments. I hope now it looks better.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38082494
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #671](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/671/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38091510
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by kstyrc <no...@github.com>.
That's my first jclouds contrib, hoping the coming ones will be bigger and better.
Unfortunately, I'm more familiar with Mockito rather than EasyMock..
Thanks
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38084816
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1146](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1146/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38221925
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-java-7-pull-requests #1142 UNSTABLE
Spurious [test failure](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/org.apache.jclouds$jclouds-compute/1142/testReport/junit/org.jclouds.compute.util/ConcurrentOpenSocketFinderTest/testChecksSocketsConcurrently/)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38098402
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #670](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/670/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38086271
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #667](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/667/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-37978819
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #672](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/672/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38096855
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #926](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/926/) 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/pull/320#issuecomment-38096414
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-java-7-pull-requests #1146 UNSTABLE
Spurious [test failure](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/org.apache.jclouds$jclouds-compute/1146/testReport/junit/org.jclouds.compute.util/ConcurrentOpenSocketFinderTest/testChecksSocketsConcurrently/)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-38222246
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Gaul <no...@github.com>.
> +
> + @Test(expectedExceptions = IllegalArgumentException.class)
> + public void testExceededContentLengthLimit() throws Exception {
> + String container = "test-container";
> + String blobName = "test-blob";
> +
> + AzureBlobClient client = createNiceMock(AzureBlobClient.class);
> + PayloadSlicer slicer = createNiceMock(PayloadSlicer.class);
> +
> + MutableBlobMetadata metadata = new MutableBlobMetadataImpl();
> + MutableContentMetadata contentMetadata = new BaseMutableContentMetadata();
> + contentMetadata.setContentLength(MultipartUploadStrategy.MAX_BLOCK_SIZE * MultipartUploadStrategy.MAX_NUMBER_OF_BLOCKS + 1);
> + metadata.setName(blobName);
> + metadata.setContentMetadata(contentMetadata);
> + Blob blob = new BlobImpl(metadata);
> + Payload payload = new StringPayload("ABCD");
Could you use a ByteSourcePayload instead? We deprecated StringPayload in 1.8.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320/files#r10776003
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Everett Toews <no...@github.com>.
Merged to master.
https://git-wip-us.apache.org/repos/asf?p=jclouds.git;a=commit;h=b220d889ca86b81d8d3badccd52215a9bf613529
Thanks @kstyrc!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-59943328
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #217](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/217/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-59920998
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
@kstyrc Sorry for the long pause - this must have slipped through the net somewhere :-(
Code itself looks fine to me. Just going to bump this to trigger the PR builders once more. Thanks for sticking with this!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-59920821
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Andrew Phillips <no...@github.com>.
> @@ -70,5 +65,29 @@ public void testExecute() throws Exception {
> replay(slicer,client);
> String etag = strat.execute(container, blob);
> assertEquals(etag, "Fake ETAG");
> +
> + verify(slicer, client);
> + }
> +
> + @Test(groups = "unit", expectedExceptions = IllegalArgumentException.class)
> + public void testExceededContentLengthLimit() throws Exception {
> + String container = "test-container";
> + String blobName = "test-blob";
> +
> + AzureBlobClient client = createMock(AzureBlobClient.class);
> + PayloadSlicer slicer = createMock(PayloadSlicer.class);
Don't we at least need to call `replay` on the mocks? And, since we're not actually expecting any behaviour, create nice mocks instead?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320/files#r10732060
Re: [jclouds] JCLOUDS-184: Improving AzureBlob unit tests for
AzureBlobBlockUploadStrategyTest (#320)
Posted by Everett Toews <no...@github.com>.
@andrewgaul @demobox Care to comment here?
If I don't hear from you by the end of the week and this test passes another build, I'll go ahead and merge it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/320#issuecomment-59918478