You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by Joe Skora <js...@gmail.com> on 2015/12/18 21:20:38 UTC
Testing handling of static class methods
For unit testing, one problem I've run into is overriding the returns from
static class methods.
For instance, PutJMS contains this code:
try {
> wrappedProducer = JmsFactory.createMessageProducer(context, true);
> logger.info("Connected to JMS server {}",
> new Object[]{context.getProperty(URL).getValue()});
> } catch (final JMSException e) {
> logger.error("Failed to connect to JMS Server due to {}", new
> Object[]{e});
> session.transfer(flowFiles, REL_FAILURE);
> context.yield();
> return;
> }
>
where JmsFactory.createmessageProducer call being defined as
public static WrappedMessageProducer createMessageProducer(...
>
which presents a problem since it can't be easily overridden for a unit
test. Exercising the
How you handle this problem?
Regards,
Joe
Re: Testing handling of static class methods
Posted by Matthew Burgess <ma...@gmail.com>.
Definitely a topic ripe for debate :) In my view there’s a whole spectrum, with one side being the case Oleg describes, where the existing encapsulation is not compromised solely for the sake of testing. On the far side is pure design-by-contract. For example, the case could be made that the JMS processor should not be so tightly coupled to a particular client, and certainly not to a class but rather to an interface. Another upside for moving the client call to a protected method is not just for testing but so that child classes can override, which is not an encapsulation thing but inheritance. That might not be useful in this particular case, but if we’re talking OO in general then it applies.
Since Bryan has cited precedence for the inner class stuff in NiFi, I tend towards that as a consistent approach. Then again, to quote my close friend Oscar Wilde ;) "Consistency is the last refuge of the unimaginative" lol
Cheers!
Matt
On 12/18/15, 5:54 PM, "Oleg Zhurakousky" <oz...@hortonworks.com> wrote:
>Personally I am with Joe on this one
>
>Exposing visibility on the method just for testing is very dangerous as it breaks encapsulation. There are different expectations and consideration on things that are made private, protected and public. Yes, all of that is meaningless when one uses reflection, but that’s a whole other discussion. Relaxing visibility implies advertisement that something is up for grabs. And in fact in Joe’s case while his intentions were noble, the fallout could be anything but (see my comments here https://github.com/apache/nifi/pull/145).
>
>Just my opinion
>
>Cheers
>Oleg
>
>On Dec 18, 2015, at 5:42 PM, Joe Skora <js...@gmail.com>> wrote:
>
>Wrapping createMessageProducer() in an instance method is a good
>suggestion, but it seems overkill just to enable testing. Prompted by
>Oleg's suggestion, I got around instance variable visibility with
>Reflection, which is nice because it doesn't require "private" be changed
>to "protected" in the class under test and doesn't require an inner test
>class. But, that doesn't appear to be possible for static methods, so
>wrapping with class methods may be the only choice. Hopefully, I've missed
>something.
>
>
>On Fri, Dec 18, 2015 at 3:58 PM, Bryan Bende <bb...@gmail.com>> wrote:
>
>If you get it into a protected instance method, you can also make an inner
>class in your test, something like TestablePutJMS extends PutJMS, and
>overrides that method to return a mock or whatever you want. That is a
>common pattern in a lot of the processor tests.
>
>On Fri, Dec 18, 2015 at 3:44 PM, Matt Burgess <ma...@gmail.com>> wrote:
>
>You could move the one static call into an instance method of PutJMS, and
>use Mockito.spy() to get a partial mock of the processor, then use when()
>to override the instance method in the test. Not sure if that's how it's
>done in other places but it's worked for me in the past.
>
>Regards,
>Matt
>
>Sent from my iPhone
>
>On Dec 18, 2015, at 3:20 PM, Joe Skora <js...@gmail.com>> wrote:
>
>For unit testing, one problem I've run into is overriding the returns
>from
>static class methods.
>
>For instance, PutJMS contains this code:
>
>try {
> wrappedProducer = JmsFactory.createMessageProducer(context, true);
> logger.info("Connected to JMS server {}",
> new Object[]{context.getProperty(URL).getValue()});
>} catch (final JMSException e) {
> logger.error("Failed to connect to JMS Server due to {}", new
>Object[]{e});
> session.transfer(flowFiles, REL_FAILURE);
> context.yield();
> return;
>}
>
>where JmsFactory.createmessageProducer call being defined as
>
>public static WrappedMessageProducer createMessageProducer(...
>
>which presents a problem since it can't be easily overridden for a unit
>test. Exercising the
>
>How you handle this problem?
>
>Regards,
>Joe
>
>
>
Re: Testing handling of static class methods
Posted by Oleg Zhurakousky <oz...@hortonworks.com>.
Personally I am with Joe on this one
Exposing visibility on the method just for testing is very dangerous as it breaks encapsulation. There are different expectations and consideration on things that are made private, protected and public. Yes, all of that is meaningless when one uses reflection, but that’s a whole other discussion. Relaxing visibility implies advertisement that something is up for grabs. And in fact in Joe’s case while his intentions were noble, the fallout could be anything but (see my comments here https://github.com/apache/nifi/pull/145).
Just my opinion
Cheers
Oleg
On Dec 18, 2015, at 5:42 PM, Joe Skora <js...@gmail.com>> wrote:
Wrapping createMessageProducer() in an instance method is a good
suggestion, but it seems overkill just to enable testing. Prompted by
Oleg's suggestion, I got around instance variable visibility with
Reflection, which is nice because it doesn't require "private" be changed
to "protected" in the class under test and doesn't require an inner test
class. But, that doesn't appear to be possible for static methods, so
wrapping with class methods may be the only choice. Hopefully, I've missed
something.
On Fri, Dec 18, 2015 at 3:58 PM, Bryan Bende <bb...@gmail.com>> wrote:
If you get it into a protected instance method, you can also make an inner
class in your test, something like TestablePutJMS extends PutJMS, and
overrides that method to return a mock or whatever you want. That is a
common pattern in a lot of the processor tests.
On Fri, Dec 18, 2015 at 3:44 PM, Matt Burgess <ma...@gmail.com>> wrote:
You could move the one static call into an instance method of PutJMS, and
use Mockito.spy() to get a partial mock of the processor, then use when()
to override the instance method in the test. Not sure if that's how it's
done in other places but it's worked for me in the past.
Regards,
Matt
Sent from my iPhone
On Dec 18, 2015, at 3:20 PM, Joe Skora <js...@gmail.com>> wrote:
For unit testing, one problem I've run into is overriding the returns
from
static class methods.
For instance, PutJMS contains this code:
try {
wrappedProducer = JmsFactory.createMessageProducer(context, true);
logger.info("Connected to JMS server {}",
new Object[]{context.getProperty(URL).getValue()});
} catch (final JMSException e) {
logger.error("Failed to connect to JMS Server due to {}", new
Object[]{e});
session.transfer(flowFiles, REL_FAILURE);
context.yield();
return;
}
where JmsFactory.createmessageProducer call being defined as
public static WrappedMessageProducer createMessageProducer(...
which presents a problem since it can't be easily overridden for a unit
test. Exercising the
How you handle this problem?
Regards,
Joe
Re: Testing handling of static class methods
Posted by Sean Busbey <bu...@apache.org>.
You could use PowerMock to either call private static methods directly
or to mock them out.
https://github.com/jayway/powermock/wiki/MockStatic
https://github.com/jayway/powermock/wiki/MockPrivate
https://github.com/jayway/powermock/wiki/BypassEncapsulation#invoking-a-private-method
On Fri, Dec 18, 2015 at 4:42 PM, Joe Skora <js...@gmail.com> wrote:
> Wrapping createMessageProducer() in an instance method is a good
> suggestion, but it seems overkill just to enable testing. Prompted by
> Oleg's suggestion, I got around instance variable visibility with
> Reflection, which is nice because it doesn't require "private" be changed
> to "protected" in the class under test and doesn't require an inner test
> class. But, that doesn't appear to be possible for static methods, so
> wrapping with class methods may be the only choice. Hopefully, I've missed
> something.
>
>
> On Fri, Dec 18, 2015 at 3:58 PM, Bryan Bende <bb...@gmail.com> wrote:
>
>> If you get it into a protected instance method, you can also make an inner
>> class in your test, something like TestablePutJMS extends PutJMS, and
>> overrides that method to return a mock or whatever you want. That is a
>> common pattern in a lot of the processor tests.
>>
>> On Fri, Dec 18, 2015 at 3:44 PM, Matt Burgess <ma...@gmail.com> wrote:
>>
>> > You could move the one static call into an instance method of PutJMS, and
>> > use Mockito.spy() to get a partial mock of the processor, then use when()
>> > to override the instance method in the test. Not sure if that's how it's
>> > done in other places but it's worked for me in the past.
>> >
>> > Regards,
>> > Matt
>> >
>> > Sent from my iPhone
>> >
>> > > On Dec 18, 2015, at 3:20 PM, Joe Skora <js...@gmail.com> wrote:
>> > >
>> > > For unit testing, one problem I've run into is overriding the returns
>> > from
>> > > static class methods.
>> > >
>> > > For instance, PutJMS contains this code:
>> > >
>> > > try {
>> > >> wrappedProducer = JmsFactory.createMessageProducer(context, true);
>> > >> logger.info("Connected to JMS server {}",
>> > >> new Object[]{context.getProperty(URL).getValue()});
>> > >> } catch (final JMSException e) {
>> > >> logger.error("Failed to connect to JMS Server due to {}", new
>> > >> Object[]{e});
>> > >> session.transfer(flowFiles, REL_FAILURE);
>> > >> context.yield();
>> > >> return;
>> > >> }
>> > >
>> > > where JmsFactory.createmessageProducer call being defined as
>> > >
>> > > public static WrappedMessageProducer createMessageProducer(...
>> > >
>> > > which presents a problem since it can't be easily overridden for a unit
>> > > test. Exercising the
>> > >
>> > > How you handle this problem?
>> > >
>> > > Regards,
>> > > Joe
>> >
>>
Re: Testing handling of static class methods
Posted by Joe Skora <js...@gmail.com>.
Wrapping createMessageProducer() in an instance method is a good
suggestion, but it seems overkill just to enable testing. Prompted by
Oleg's suggestion, I got around instance variable visibility with
Reflection, which is nice because it doesn't require "private" be changed
to "protected" in the class under test and doesn't require an inner test
class. But, that doesn't appear to be possible for static methods, so
wrapping with class methods may be the only choice. Hopefully, I've missed
something.
On Fri, Dec 18, 2015 at 3:58 PM, Bryan Bende <bb...@gmail.com> wrote:
> If you get it into a protected instance method, you can also make an inner
> class in your test, something like TestablePutJMS extends PutJMS, and
> overrides that method to return a mock or whatever you want. That is a
> common pattern in a lot of the processor tests.
>
> On Fri, Dec 18, 2015 at 3:44 PM, Matt Burgess <ma...@gmail.com> wrote:
>
> > You could move the one static call into an instance method of PutJMS, and
> > use Mockito.spy() to get a partial mock of the processor, then use when()
> > to override the instance method in the test. Not sure if that's how it's
> > done in other places but it's worked for me in the past.
> >
> > Regards,
> > Matt
> >
> > Sent from my iPhone
> >
> > > On Dec 18, 2015, at 3:20 PM, Joe Skora <js...@gmail.com> wrote:
> > >
> > > For unit testing, one problem I've run into is overriding the returns
> > from
> > > static class methods.
> > >
> > > For instance, PutJMS contains this code:
> > >
> > > try {
> > >> wrappedProducer = JmsFactory.createMessageProducer(context, true);
> > >> logger.info("Connected to JMS server {}",
> > >> new Object[]{context.getProperty(URL).getValue()});
> > >> } catch (final JMSException e) {
> > >> logger.error("Failed to connect to JMS Server due to {}", new
> > >> Object[]{e});
> > >> session.transfer(flowFiles, REL_FAILURE);
> > >> context.yield();
> > >> return;
> > >> }
> > >
> > > where JmsFactory.createmessageProducer call being defined as
> > >
> > > public static WrappedMessageProducer createMessageProducer(...
> > >
> > > which presents a problem since it can't be easily overridden for a unit
> > > test. Exercising the
> > >
> > > How you handle this problem?
> > >
> > > Regards,
> > > Joe
> >
>
Re: Testing handling of static class methods
Posted by Bryan Bende <bb...@gmail.com>.
If you get it into a protected instance method, you can also make an inner
class in your test, something like TestablePutJMS extends PutJMS, and
overrides that method to return a mock or whatever you want. That is a
common pattern in a lot of the processor tests.
On Fri, Dec 18, 2015 at 3:44 PM, Matt Burgess <ma...@gmail.com> wrote:
> You could move the one static call into an instance method of PutJMS, and
> use Mockito.spy() to get a partial mock of the processor, then use when()
> to override the instance method in the test. Not sure if that's how it's
> done in other places but it's worked for me in the past.
>
> Regards,
> Matt
>
> Sent from my iPhone
>
> > On Dec 18, 2015, at 3:20 PM, Joe Skora <js...@gmail.com> wrote:
> >
> > For unit testing, one problem I've run into is overriding the returns
> from
> > static class methods.
> >
> > For instance, PutJMS contains this code:
> >
> > try {
> >> wrappedProducer = JmsFactory.createMessageProducer(context, true);
> >> logger.info("Connected to JMS server {}",
> >> new Object[]{context.getProperty(URL).getValue()});
> >> } catch (final JMSException e) {
> >> logger.error("Failed to connect to JMS Server due to {}", new
> >> Object[]{e});
> >> session.transfer(flowFiles, REL_FAILURE);
> >> context.yield();
> >> return;
> >> }
> >
> > where JmsFactory.createmessageProducer call being defined as
> >
> > public static WrappedMessageProducer createMessageProducer(...
> >
> > which presents a problem since it can't be easily overridden for a unit
> > test. Exercising the
> >
> > How you handle this problem?
> >
> > Regards,
> > Joe
>
Re: Testing handling of static class methods
Posted by Matt Burgess <ma...@gmail.com>.
You could move the one static call into an instance method of PutJMS, and use Mockito.spy() to get a partial mock of the processor, then use when() to override the instance method in the test. Not sure if that's how it's done in other places but it's worked for me in the past.
Regards,
Matt
Sent from my iPhone
> On Dec 18, 2015, at 3:20 PM, Joe Skora <js...@gmail.com> wrote:
>
> For unit testing, one problem I've run into is overriding the returns from
> static class methods.
>
> For instance, PutJMS contains this code:
>
> try {
>> wrappedProducer = JmsFactory.createMessageProducer(context, true);
>> logger.info("Connected to JMS server {}",
>> new Object[]{context.getProperty(URL).getValue()});
>> } catch (final JMSException e) {
>> logger.error("Failed to connect to JMS Server due to {}", new
>> Object[]{e});
>> session.transfer(flowFiles, REL_FAILURE);
>> context.yield();
>> return;
>> }
>
> where JmsFactory.createmessageProducer call being defined as
>
> public static WrappedMessageProducer createMessageProducer(...
>
> which presents a problem since it can't be easily overridden for a unit
> test. Exercising the
>
> How you handle this problem?
>
> Regards,
> Joe