Testing handling of static class methods

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Testing handling of static class methods

Joe Skora
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
Reply | Threaded
Open this post in threaded view
|

Re: Testing handling of static class methods

Matt Burgess
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 <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: Testing handling of static class methods

Bryan Bende
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 <[hidden email]> 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 <[hidden email]> 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
>
Reply | Threaded
Open this post in threaded view
|

Re: Testing handling of static class methods

Joe Skora
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Testing handling of static class methods

Oleg Zhurakousky
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 <[hidden email]<mailto:[hidden email]>> 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 <[hidden email]<mailto:[hidden email]>> 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 <[hidden email]<mailto:[hidden email]>> 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 <[hidden email]<mailto:[hidden email]>> 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



Reply | Threaded
Open this post in threaded view
|

Re: Testing handling of static class methods

Matt Burgess
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" <[hidden email]> 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 <[hidden email]<mailto:[hidden email]>> 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 <[hidden email]<mailto:[hidden email]>> 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 <[hidden email]<mailto:[hidden email]>> 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 <[hidden email]<mailto:[hidden email]>> 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
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Testing handling of static class methods

Sean Busbey-2
In reply to this post by Joe Skora
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
>> >
>>