Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use wildcard type arguments when locating cdi beans #143

Merged
merged 1 commit into from Oct 24, 2013

Conversation

@BrynCooke
Copy link
Contributor

@BrynCooke BrynCooke commented Oct 23, 2013

By using a wildcard instead of Object for type parameters the correct beans are found.

@BrynCooke
Copy link
Contributor Author

@BrynCooke BrynCooke commented Oct 23, 2013

Use https://github.com/ocpsoft/rewrite/pull/143/files?w=1 to view changes without whitespace issues.

@BrynCooke
Copy link
Contributor Author

@BrynCooke BrynCooke commented Oct 23, 2013

As discussed in #137

@lincolnthree
Copy link
Member

@lincolnthree lincolnthree commented Oct 24, 2013

Thanks Bryn!

Hmmmm... Travis says the build failed, but I'm going to merge this anyway and see what happens. I think it may have been a fluke.

lincolnthree added a commit that referenced this pull request Oct 24, 2013
Use wildcard type arguments when locating cdi beans
@lincolnthree lincolnthree merged commit 46af40a into ocpsoft:master Oct 24, 2013
1 check failed
1 check failed
default The Travis CI build failed
Details
@chkal
Copy link
Member

@chkal chkal commented Oct 24, 2013

Hmmm. Looks like the CDI tests now pass on Wildfly but fail on TomEE :)

@BrynCooke
Copy link
Contributor Author

@BrynCooke BrynCooke commented Oct 24, 2013

I couldn't see from Travis why this was failing on TomEE, I think the logs are tuncated?

@chkal
Copy link
Member

@chkal chkal commented Oct 24, 2013

There is a "download logs" link on the upper right side.

Here are the raw logs:

https://s3.amazonaws.com/archive.travis-ci.org/jobs/12997362/log.txt

@lincolnthree
Copy link
Member

@lincolnthree lincolnthree commented Oct 24, 2013

It looks like the CDI integration tests now fail to deploy on TomEE. That's
the reason. Not sure the root cause of that.

On Thu, Oct 24, 2013 at 2:27 PM, Christian Kaltepoth <
notifications@github.com> wrote:

There is a "download logs" link on the upper right side.

Here are the raw logs:

https://s3.amazonaws.com/archive.travis-ci.org/jobs/12997362/log.txt


Reply to this email directly or view it on GitHubhttps://github.com//pull/143#issuecomment-27017416
.

Lincoln Baxter, III
http://ocpsoft.org
"Simpler is better."

@BrynCooke
Copy link
Contributor Author

@BrynCooke BrynCooke commented Oct 24, 2013

On my local machine rolling back to before my change and running the build with:
export CONTAINER=TOMEE_MANAGED_1.5
produces the same errors

@BrynCooke
Copy link
Contributor Author

@BrynCooke BrynCooke commented Oct 24, 2013

But I need to have a deployable container on the classpath... So it could still be my changes

@BrynCooke
Copy link
Contributor Author

@BrynCooke BrynCooke commented Oct 24, 2013

I think the TomEE issue is unrelated to this change:
git checkout a0ef442
mvn clean test -PTOMEE_MANAGED_1.5

Gives me

Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 3.792 sec <<< FAILURE! - in org.ocpsoft.rewrite.cdi.bridge.CdiMultipleFeaturesTest
org.ocpsoft.rewrite.cdi.bridge.CdiMultipleFeaturesTest Time elapsed: 3.792 sec <<< ERROR!
org.jboss.arquillian.container.spi.client.container.DeploymentException: Unable to deploy

@chkal
Copy link
Member

@chkal chkal commented Oct 24, 2013

Hmmm. Weird. The tests were fine before we merged the pull request:

https://travis-ci.org/ocpsoft/rewrite/builds

However, I think your patch is fine. If there are problems with TomEE, they will either be unrelated to you patch or they are caused by a bug in OpenWebBeans.

I'll try to find some time tomorrow to have a deeper look.

@chkal
Copy link
Member

@chkal chkal commented Oct 25, 2013

I just checked what is going wrong with the TomEE builds. The tests were fine with:

actualTypeParameters[i] = Object.class;

But they broke after changing it to:

actualTypeParameters[i] = new WildcardTypeImpl(new Type[]{}, new Type[]{});

The cause seems to be an ArrayIndexOutOfBoundsException in the OpenWebBeans code. Changing it like this seems to fix the issue.

actualTypeParameters[i] = new WildcardTypeImpl(new Type[]{Object.class}, new Type[]{});

I pushed the change out:

6f3578d

Let's wait for Travis to tell us if this works on the other containers. :)

@chkal
Copy link
Member

@chkal chkal commented Oct 25, 2013

I just reported this issue:

https://issues.apache.org/jira/browse/OWB-907

@chkal
Copy link
Member

@chkal chkal commented Oct 25, 2013

The tests are green:

https://travis-ci.org/ocpsoft/rewrite

Thanks again for helping us with this. :)

@lincolnthree
Copy link
Member

@lincolnthree lincolnthree commented Oct 25, 2013

Yeah! Seriously! Thanks a lot for researching this fix! We really
appreciate it, and we'll get a new release out for you soon :) Join our
mailing list to get notifications of this! http://ocpsoft.org/subscribe/

On Fri, Oct 25, 2013 at 2:24 AM, Christian Kaltepoth <
notifications@github.com> wrote:

The tests are green:

https://travis-ci.org/ocpsoft/rewrite

Thanks again for helping us with this. :)


Reply to this email directly or view it on GitHubhttps://github.com//pull/143#issuecomment-27068218
.

Lincoln Baxter, III
http://ocpsoft.org
"Simpler is better."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.