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

WELD-2427 WeldInstance#select and WeldEvent#select with Type. #1750

Merged
merged 1 commit into from Nov 2, 2017

Conversation

Projects
None yet
2 participants
@manovotn
Contributor

manovotn commented Nov 2, 2017

No description provided.

@@ -50,7 +51,8 @@
public class ResolvableBuilder {
private static final Class<?>[] FACADE_TYPES = new Class<?>[] { Event.class, Instance.class, WeldInstance.class, Provider.class, InterceptionFactory.class };
private static final Class<?>[] FACADE_TYPES = new Class<?>[] { Event.class, Instance.class, WeldEvent.class,

This comment has been minimized.

@manovotn

manovotn Nov 2, 2017

Contributor

Not really sure we need to change this?

@manovotn

manovotn Nov 2, 2017

Contributor

Not really sure we need to change this?

This comment has been minimized.

@mkouba

mkouba Nov 2, 2017

Member

I believe we need this.

@mkouba

mkouba Nov 2, 2017

Member

I believe we need this.

This comment has been minimized.

@manovotn

manovotn Nov 2, 2017

Contributor

I think it's for when you resolve bean via BM (why would you do that with Event though?), but I am not really sure. If I remove it @Inject will keep working.

@manovotn

manovotn Nov 2, 2017

Contributor

I think it's for when you resolve bean via BM (why would you do that with Event though?), but I am not really sure. If I remove it @Inject will keep working.

* @author <a href="mailto:manovotn@redhat.com">Matej Novotny</a>
*/
@RunWith(Arquillian.class)
public class WeldEventTest {

This comment has been minimized.

@manovotn

manovotn Nov 2, 2017

Contributor

The tests are verifying:

  • ability to @Inject WeldEvent<X>
  • known functionality when replacing Event with WeldEvent
  • new selections based on Type and expected exceptions

I am accepting additional ideas for corner cases and/or possible problematic areas.

@manovotn

manovotn Nov 2, 2017

Contributor

The tests are verifying:

  • ability to @Inject WeldEvent<X>
  • known functionality when replacing Event with WeldEvent
  • new selections based on Type and expected exceptions

I am accepting additional ideas for corner cases and/or possible problematic areas.

}
// verify valid behaviour, following selections should work
bean.getEventObject().select(firstType);

This comment has been minimized.

@mkouba

mkouba Nov 2, 2017

Member

We should probably also test observability here... i.e. that a child Event works as expected?

@mkouba

mkouba Nov 2, 2017

Member

We should probably also test observability here... i.e. that a child Event works as expected?

This comment has been minimized.

@manovotn

manovotn Nov 2, 2017

Contributor

See testEventObservability method

@manovotn

manovotn Nov 2, 2017

Contributor

See testEventObservability method

This comment has been minimized.

@mkouba

mkouba Nov 2, 2017

Member

Ah, you're right ;-)

@mkouba

mkouba Nov 2, 2017

Member

Ah, you're right ;-)

public void testValidSelect() {
try (WeldContainer container = new Weld().initialize()) {
// Class<T> implements Type, so let's make use of that
Type firstType = SomeInterface.class;

This comment has been minimized.

@mkouba

mkouba Nov 2, 2017

Member

Maybe we should add some tests for parameterized types...

@mkouba

mkouba Nov 2, 2017

Member

Maybe we should add some tests for parameterized types...

This comment has been minimized.

@manovotn

manovotn Nov 2, 2017

Contributor

Hmm, can add that too. Though there isn't much of a change here (as opposed to WeldEvent where I have such test). You could have done the same before with WeldInstance.

@manovotn

manovotn Nov 2, 2017

Contributor

Hmm, can add that too. Though there isn't much of a change here (as opposed to WeldEvent where I have such test). You could have done the same before with WeldInstance.

@manovotn

This comment has been minimized.

Show comment
Hide comment
@manovotn

manovotn Nov 2, 2017

Contributor

Added tests for @PreDestroy and @Typed and one simple valid select for ParameterizedType

Contributor

manovotn commented Nov 2, 2017

Added tests for @PreDestroy and @Typed and one simple valid select for ParameterizedType

@manovotn manovotn requested a review from mkouba Nov 2, 2017

@mkouba

mkouba approved these changes Nov 2, 2017

Looks good.

@manovotn manovotn removed the api-upgrade label Nov 2, 2017

@mkouba mkouba merged commit b7c1f4b into weld:master Nov 2, 2017

3 checks passed

Weld Central CI Build finished.
Details
Weld Publish CI Finished
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment