Closed #333.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#event-191653695
@lahirus Thanks for the note. I'll hold off until the end of next week then
(Oct. 31).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-60289412
Okay.. i think this is expecting few more test cases to be completed. I
might not able to attend it this week. I'll try to re-send this with test
cases ASAP.
Thanks.
On Tue, Oct 21, 2014 at 8:29 AM, Everett Toews
wrote:
> It's release time in jclouds and that means we like to do a little house
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 t
I think I didn't explain properly what I wanted to say. We need the three tests
I mentioned in my last comment.
Regarding the `addSecurityGroups` one, there are not tests for other use cases
of that method, but we need at least the ones that validate the changes in this
PR. What I suggested was
@nacx I thought I can avoid "addSecurityGroups" test case as per one of your
previous replies :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-45178771
> jclouds-java-7-pull-requests #1306 UNSTABLE
The usual [Three Azure
Failures](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1306/testReport/)
of the Parallel Apocalypse...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#iss
[jclouds-java-7-pull-requests
#1306](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1306/)
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/333#issuecomment-44318658
[jclouds-pull-requests
#835](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/835/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44317170
[jclouds » jclouds
#1152](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1152/) 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/333#issuecomme
The PR changes more things apart from the binder, and I think there should be,
at least, the following tests, especially the expect ones:
>* CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsTest
>(simply add a method that tests the addSecurityGroups method)
* AWSRunInstancesOpti
> @@ -125,8 +125,12 @@ public void endElement(String uri, String name, String
> qName) {
> builder.ramdiskId(currentOrNull());
>} else if (qName.equals("enabled")) {
> String monitoringEnabled = currentOrNull();
> - if (monitoringEnabled != null)
> -
> -.subnetId("subnet-xyz").build();
> -
> - assertEquals(binder.apply(spec),
> ImmutableMap.of("LaunchSpecification.InstanceType", "t1.micro",
> -"LaunchSpecification.ImageId", "ami-123",
> "LaunchSpecification.SubnetId", "subnet-xyz"));
> - }
> +@Test
> +pu
> I can do another if you mean that.
If that is all that has changed, I don't think we need another test run.
@nacx Are your test cases covered?
@abayer Any thoughts on other steps to take to validate this PR, if any?
---
Reply to this email directly or view it on GitHub:
https://github.com/jcl
Do you mean after this test case addition? I already did a one. I can do
another if you mean that.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44070662
Glad that we're back to a successfully building PR here, @lahirus! Do you have
any live test output for this?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44058892
> jclouds-java-7-pull-requests #1292 SUCCESS
No test failures this time. Curious to see if this stays this way...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44058807
[jclouds-pull-requests
#822](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/822/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44053001
[jclouds-java-7-pull-requests
#1292](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1292/)
SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-44051429
[jclouds » jclouds
#1134](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1134/) 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/333#issuecomme
> jclouds-pull-requests #821 FAILURE
Looks like a [slave
timeout](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/821/console)
here - unrelated to the PR.
> jclouds-java-7-pull-requests #1291 FAILURE
These [test
failures](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-req
Closed #333.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#event-124350951
[jclouds-java-7-pull-requests
#1291](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1291/)
FAILURE
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/333#issuecomment-43978380
[jclouds-pull-requests
#821](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/821/) FAILURE
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/333#issuecomment-43978362
[jclouds » jclouds
#1133](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1133/) 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/333#issuecomme
> + // setup expectations
> + expect(options.getGroupIds()).andReturn(ImmutableSet. of());
> + expect(options.getGroups()).andReturn(groupNames).atLeastOnce();
> + expect(options.getInboundPorts()).andReturn(ports).atLeastOnce();
> + RegionNameAndIngressRules regionNameAndI
> + // setup expectations
> + expect(options.getGroupIds()).andReturn(ImmutableSet. of());
> + expect(options.getGroups()).andReturn(groupNames).atLeastOnce();
> + expect(options.getInboundPorts()).andReturn(ports).atLeastOnce();
> + RegionNameAndIngressRules regionNameAndI
> + // setup expectations
> + expect(options.getGroupIds()).andReturn(ImmutableSet. of());
> + expect(options.getGroups()).andReturn(groupNames).atLeastOnce();
> + expect(options.getInboundPorts()).andReturn(ports).atLeastOnce();
> + RegionNameAndIngressRules regionNameAndI
"Real" [compilation
failures](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1055/console)
here:
```
[INFO] 5 warnings
[INFO] -
[INFO] -
[ERROR] COMPILATION ERROR :
[
[jclouds-java-7-pull-requests
#1246](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1246/)
FAILURE
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/333#issuecomment-41782708
[jclouds » jclouds
#1055](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1055/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclo
[jclouds-pull-requests
#776](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/776/) FAILURE
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/333#issuecomment-41780038
I think we don't have other tests right now. Perhaps @abayer can help providing
info about the different use cases.
Anyway, I'd start adding some tests that directly call the method with
different inputs that make sense to your changes and verifying that the options
are properly populated. Cover
I was able to add test for "BindLaunchSpecificationToFormParamsTest" class. Do
we have any test to test other usecases of "addSecurityGroups"? I'm trying to
understand the model it works, since it is bit complex to me :)
---
Reply to this email directly or view it on GitHub:
https://github.com/j
[jclouds-java-7-pull-requests
#1231](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1231/)
SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40666087
[jclouds-pull-requests
#761](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/761/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40665662
[jclouds » jclouds
#1034](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1034/) 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/333#issuecomme
Thanks a lot for review @nacx. Will do these test case additions.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40617184
Thanks for the detailed review, @nacx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40593176
First of all thanks for the contribution @lahirus!
There were already many live tests failing in master, so I assume those
failures aren't related to your changes. However, before merging this pull
requests, some unit tests that assert your additions should be added to the
following classes
*
Will do the style adjustments.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40567156
> @@ -182,7 +197,7 @@ public AWSEC2TemplateOptions subnetId(String subnetId) {
> /**
> * @see
> org.jclouds.aws.ec2.options.AWSRunInstancesOptions#withIAMInstanceProfileArn(String)
> */
> - @SinceApiVersion("2012-06-01")
> + @SinceApiVersion("2014-02-01")
Oh. Got the idea. I wil
> @@ -124,9 +124,13 @@ public void endElement(String uri, String name, String
> qName) {
>} else if (qName.equals("ramdiskId")) {
> builder.ramdiskId(currentOrNull());
>} else if (qName.equals("enabled")) {
> - String monitoringEnabled = currentOrNull();
> -
> return false;
> - } else if (!monitoringEnabled.equals(other.monitoringEnabled))
> - return false;
> - if (ramdiskId == null) {
> + if (publicIpAddressAssociated == null) {
> +if (other.publicIpAddressAssociated != null)
> + return fal
>if (subnetId != null) {
> -
> AWSRunInstancesOptions.class.cast(instanceOptions).withSubnetId(subnetId);
> + if(associatePublicIpAddress){
> +
> AWSRunInstancesOptions.class.cast(instanceOptions).associatePublicIpAddressAndSubnetId(subnetId);
> +
> @@ -182,7 +197,7 @@ public AWSEC2TemplateOptions subnetId(String subnetId) {
> /**
> * @see
> org.jclouds.aws.ec2.options.AWSRunInstancesOptions#withIAMInstanceProfileArn(String)
> */
> - @SinceApiVersion("2012-06-01")
> + @SinceApiVersion("2014-02-01")
This shouldn't be chang
> @@ -65,8 +65,17 @@
>if (launchSpec.getSecurityGroupIds().size() > 0)
> options.withSecurityGroupIds(launchSpec.getSecurityGroupIds());
>options.asType(checkNotNull(launchSpec.getInstanceType(),
> "instanceType"));
> - if (launchSpec.getSubnetId() != null)
> -
> Here is the results from live test, There were 19 failures and one skipped.
Ah, great, thanks for that. @abayer @nacx Is this what we'd be expected to see
here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40505544
Here is the results from live test, There were 19 failures and one skipped.
Failed tests:
testAmazonLinuxInstanceStore(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest):
expected [pv-2013.09.rc-1] but found [pv-2014.03.rc-1]
testFastestTemplateBuilder(org.jclouds.aws.ec2.compute.AW
:) I was wondering if you have a sample for AWS there.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40442470
[jclouds-java-7-pull-requests
#1226](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1226/)
SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40432798
[jclouds-pull-requests
#756](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/756/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40432444
[jclouds » jclouds
#1028](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1028/) 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/333#issuecomme
Reopened #333.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333
Closed #333.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333
> Is it mentioned in Apache documentation site?
You've caught us in a bit of an in-between state here. The [testing
guide](https://github.com/jclouds/jclouds-site/blob/4bb3b95c39152828ffe6ae44d52b83f827f9fb8d/documentation/devguides/provider-testing.markdown#running-live-tests)
is still being po
Hi,
Could not find exact instruction run live test from the link provided. Is it
mentioned in Apache documentation site?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40425442
Hi,
Sorry that i could not reply early. I tried building with clean repository. I
could not reproduce any test failure :(.
Ah.. ok.. i will improve the commit message.
Thanks.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-402526
Can you also improve the commit message?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40252083
> Will try to run a live test. It might take a bit time for me :( ..
Have you had any chance to look into this, @lahirus?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-40230928
> jclouds » jclouds #978 UNSTABLE
> jclouds-java-7-pull-requests #1190 UNSTABLE
Both unrelated
[test](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/org.apache.jclouds$jclouds-compute/1190/testReport/junit/org.jclouds.compute.util/ConcurrentOpenSocketFinderTest/testChecksSocket
[jclouds-java-7-pull-requests
#1190](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1190/)
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/333#issuecomment-39054073
[jclouds-pull-requests
#720](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/720/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-39053897
[jclouds » jclouds
#978](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/978/) UNSTABLE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclou
Hi @demobox
I have fixed the Checkstyle violations.
Will try to run a live test. It might take a bit time for me :( ..
Thanks.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-39052252
Hi,
Thanks for looking into it. Will work on these.
Thanks.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-38884799
Another thing that we will certainly want to do, by the way, is run the live
test suite against AWS EC2. See the ["Running live tests"
section](http://2bfcdbe0cc28b674f568-6e84ef02f7d038e5f4526bc5b8a12a51.r1.cf1.rackcdn.com/documentation/devguides/provider-testing/)
from this cached version of t
Will try to have a look at this soon, but could you in the meantime clean up
the four small [Checkstyle
violations](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/717/org.apache.jclouds.provider$aws-ec2/violations/)
that have been introduced?
Thanks, @lahirus!
---
Reply to this ema
[jclouds-java-7-pull-requests
#1187](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1187/)
SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-38850335
[jclouds-pull-requests
#717](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/717/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/333#issuecomment-38849202
[jclouds » jclouds
#974](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/974/) 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/333#issuecomment
I have fixed imports movements.
You can merge this Pull Request by running:
git pull https://github.com/lahirus/jclouds master
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/333
-- Commit Summary --
* Fixing Jclouds-509
-- File Changes --
72 matches
Mail list logo
