skyscreamer/nevado

Mechanism to teardown mock queues

arranbartish opened this issue · 7 comments

arranbartish commented

As I've said in emails recently. Exceptional work.

In some test cases there can be messages intentionally left in the mock queues at the end of the test. Currently there is no mechanism on the MockSQSConnector to "reset" the queues (teardown style) ready for the next test. The impact of this is that following tests can be impacted by these messages "randomly" which can break tests designed to be atomic.

As I have already stated in email, I've started to put something in place. Is this something you would be interested in me contributing back?

Thanks again.

carterpage commented

Sounds like a useful thing to have in the library. Basic rules for
contributions are 1) provide something useful (achieved), 2) don't break
backwards functionality, and 3) provide unit tests.

Thanks,
Carter

On Tue, Sep 10, 2013 at 2:14 PM, arranbartish notifications@github.comwrote:

As I've said in emails recently. Exceptional work.

In some test cases there can be messages intentionally left in the mock
queues at the end of the test. Currently there is no mechanism on the
MockSQSConnector to "reset" the queues (teardown style) ready for the next
test. The impact of this is that following tests can be impacted by these
messages "randomly" which can break tests designed to be atomic.

As I have already stated in email, I've started to put something in place.
Is this something you would be interested in me contributing back?

Thanks again.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/64
.

arranbartish commented

Thanks Carter,

Three rules I live by.

Looking through the existing tests it looks like they are mostly a form of integration unit test using SpringJUnit4ClassRunner. I’ll follow the same pattern.

I have a couple of questions though.

  1.   Is there any reason why the code base has so few pure unit tests (i.e. dependencies mocked and no spring context)? I could only find a few examples like BackoffSleeperTest and StringSerializationTest. 

  2.   Has there been a decision not to use a framework like Mockito to isolate some of your classes?

I’m putting together a list of the changes I want to add so you can make sure I am not going off in a direction that you don’t want.

Thanks,

Arran

From: Carter Page [mailto:notifications@github.com]
Sent: Tuesday, 10 September 2013 5:10 PM
To: skyscreamer/nevado
Cc: arranbartish
Subject: Re: [nevado] Mechanism to teardown mock queues (#64)

Sounds like a useful thing to have in the library. Basic rules for
contributions are 1) provide something useful (achieved), 2) don't break
backwards functionality, and 3) provide unit tests.

Thanks,
Carter

On Tue, Sep 10, 2013 at 2:14 PM, arranbartish <notifications@github.com mailto:notifications@github.com >wrote:

As I've said in emails recently. Exceptional work.

In some test cases there can be messages intentionally left in the mock
queues at the end of the test. Currently there is no mechanism on the
MockSQSConnector to "reset" the queues (teardown style) ready for the next
test. The impact of this is that following tests can be impacted by these
messages "randomly" which can break tests designed to be atomic.

As I have already stated in email, I've started to put something in place.
Is this something you would be interested in me contributing back?

Thanks again.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/64
.

—
Reply to this email directly or view it on GitHub #64 (comment) . https://github.com/notifications/beacon/CGSywq1gaKEoKthlhNZbYrXKHecjZO4LQbAKiDkIqqUtNTHk8J3O9wxmJEPcY0Ko.gif

arranbartish commented

Hi Carter,

To bounce my thoughts off you. I am thinking of adding a new interface ResettableMock which provides a method void reset(). The advantage of providing the interface is that you can use it in a couple of ways in tests

Unit tests using SpringJUnit4ClassRunner

@Autowired

private List allResetableMocks

Test frameworks like mine

final Map<String, ResettableMock> allResetableMocksManagedBySpring = ((ListableBeanFactory)context).getBeansOfType(ResettableMock.class);

In both cases then it’s quite convenient to either iterate and call reset, or target a specific bean(s).

I see three classes that I am targeting for this.

  1.   MockSQSConnector is the obvious choice as it has a reference to all the Mock queues however it doesn’t have access to the internal lists so rather than expose the list out I want assign the interface ResettableMock to the MockSQSQueue as well. This way the reset method on the MockSQSConnector can just create a set of all MockSQSQueue iterate through and call reset on each.

  2.   MockSQSQueue so that it provides a reset method and maintains control over its internal List. The method would simply call clear on the list. Though other things of note would be that the method is Synchronised and would only clear the list if the queue was not deleted. If the queue is deleted then it has been removed from the MockSQSConnector anyway.

That is as far as I wanted to go. However after inspecting the code I realised that in the cases of the MockSQSConnector and MockSQSQueue all instances are created as a new and the Spring context would have no visibility of them. This would mean that the @Autowired annotation and any calls to a ListableBeanFactory would not return or find any beans.

  1.   MockSQSConnectorFactory is the only class that seems to get managed by spring and as such I would also target this with a ResettableMock. The implementation would simply call reset on the internal MockSQSConnector.

I believe these changes;

  1.   Are in the spirit of the interface as each of these three classes keep some form of state or stateful bean that could be reset. There is only one other Mock class MockSQSMessage however this is a simple bean that keeps no real state and are a create and throw away bean as opposed to the other three which may stick around through the life of the spring context. 

  2.   Achieve the desire result by calling reset on all ResettableMocks returning them to an empty state. 

  3.   Nonintrusive as by adding an interface to the mock classes it simply enriches their behaviour in a consistent way without impacting anything that doesn’t care about mocks.

Testing

As most of the classes (MockSQSConnector, MockSQSQueue) are managed outside of spring I am going to test them with plain old unit tests and mock objects. Without a mocking framework I’ll use some simple mock instances to test their behaviour. I will add some new tests to the ConnectionFactoryEndpointTest which will be integration form unit tests of the reset.

Any thoughts or suggestions before I crack on?

Thanks,

Arran

From: Carter Page [mailto:notifications@github.com]
Sent: Tuesday, 10 September 2013 5:10 PM
To: skyscreamer/nevado
Cc: arranbartish
Subject: Re: [nevado] Mechanism to teardown mock queues (#64)

Sounds like a useful thing to have in the library. Basic rules for
contributions are 1) provide something useful (achieved), 2) don't break
backwards functionality, and 3) provide unit tests.

Thanks,
Carter

On Tue, Sep 10, 2013 at 2:14 PM, arranbartish <notifications@github.com mailto:notifications@github.com >wrote:

As I've said in emails recently. Exceptional work.

In some test cases there can be messages intentionally left in the mock
queues at the end of the test. Currently there is no mechanism on the
MockSQSConnector to "reset" the queues (teardown style) ready for the next
test. The impact of this is that following tests can be impacted by these
messages "randomly" which can break tests designed to be atomic.

As I have already stated in email, I've started to put something in place.
Is this something you would be interested in me contributing back?

Thanks again.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/64
.

—
Reply to this email directly or view it on GitHub #64 (comment) . https://github.com/notifications/beacon/CGSywq1gaKEoKthlhNZbYrXKHecjZO4LQbAKiDkIqqUtNTHk8J3O9wxmJEPcY0Ko.gif

arranbartish commented

Hi Carter,

Dev complete. Instead of adding to ConnectionFactoryEndpointTest I created ConnectionFactoryResetTest.

Do you want to do a quick code review?

git@github.com:arranbartish/nevado.git

branch is
mock-queue-reset

Thanks,
Arran

carterpage commented

Sorry for the delay, but I was on vacation. I like the approach. I look
forward to seeing the pull request once it's done!

On Tue, Sep 10, 2013 at 5:53 PM, arranbartish notifications@github.comwrote:

Hi Carter,

To bounce my thoughts off you. I am thinking of adding a new interface
ResettableMock which provides a method void reset(). The advantage of
providing the interface is that you can use it in a couple of ways in tests

Unit tests using SpringJUnit4ClassRunner

@Autowired

private List allResetableMocks

Test frameworks like mine

final Map<String, ResettableMock> allResetableMocksManagedBySpring =
((ListableBeanFactory)context).getBeansOfType(ResettableMock.class);

In both cases then it’s quite convenient to either iterate and call reset,
or target a specific bean(s).

I see three classes that I am targeting for this.

  1. MockSQSConnector is the obvious choice as it has a reference to all the
    Mock queues however it doesn’t have access to the internal lists so rather
    than expose the list out I want assign the interface ResettableMock to the
    MockSQSQueue as well. This way the reset method on the MockSQSConnector can
    just create a set of all MockSQSQueue iterate through and call reset on
    each.
  2. MockSQSQueue so that it provides a reset method and maintains control
    over its internal List. The method would simply call clear on the list.
    Though other things of note would be that the method is Synchronised and
    would only clear the list if the queue was not deleted. If the queue is
    deleted then it has been removed from the MockSQSConnector anyway.

That is as far as I wanted to go. However after inspecting the code I
realised that in the cases of the MockSQSConnector and MockSQSQueue all
instances are created as a new and the Spring context would have no
visibility of them. This would mean that the @Autowired annotation and any
calls to a ListableBeanFactory would not return or find any beans.

  1. MockSQSConnectorFactory is the only class that seems to get managed by
    spring and as such I would also target this with a ResettableMock. The
    implementation would simply call reset on the internal MockSQSConnector.

I believe these changes;

  1. Are in the spirit of the interface as each of these three classes keep
    some form of state or stateful bean that could be reset. There is only one
    other Mock class MockSQSMessage however this is a simple bean that keeps no
    real state and are a create and throw away bean as opposed to the other
    three which may stick around through the life of the spring context.
  2. Achieve the desire result by calling reset on all ResettableMocks
    returning them to an empty state.
  3. Nonintrusive as by adding an interface to the mock classes it simply
    enriches their behaviour in a consistent way without impacting anything
    that doesn’t care about mocks.

Testing

As most of the classes (MockSQSConnector, MockSQSQueue) are managed
outside of spring I am going to test them with plain old unit tests and
mock objects. Without a mocking framework I’ll use some simple mock
instances to test their behaviour. I will add some new tests to the
ConnectionFactoryEndpointTest which will be integration form unit tests of
the reset.

Any thoughts or suggestions before I crack on?

Thanks,

Arran

From: Carter Page [mailto:notifications@github.com]
Sent: Tuesday, 10 September 2013 5:10 PM
To: skyscreamer/nevado
Cc: arranbartish
Subject: Re: [nevado] Mechanism to teardown mock queues (#64)

Sounds like a useful thing to have in the library. Basic rules for
contributions are 1) provide something useful (achieved), 2) don't break
backwards functionality, and 3) provide unit tests.

Thanks,
Carter

On Tue, Sep 10, 2013 at 2:14 PM, arranbartish <notifications@github.com<mailto:
notifications@github.com> >wrote:

As I've said in emails recently. Exceptional work.

In some test cases there can be messages intentionally left in the mock
queues at the end of the test. Currently there is no mechanism on the
MockSQSConnector to "reset" the queues (teardown style) ready for the
next
test. The impact of this is that following tests can be impacted by
these
messages "randomly" which can break tests designed to be atomic.

As I have already stated in email, I've started to put something in
place.
Is this something you would be interested in me contributing back?

Thanks again.

—
Reply to this email directly or view it on GitHub<
https://github.com/skyscreamer/nevado/issues/64>
.

—
Reply to this email directly or view it on GitHub <
https://github.com/skyscreamer/nevado/issues/64#issuecomment-24194850> . <
https://github.com/notifications/beacon/CGSywq1gaKEoKthlhNZbYrXKHecjZO4LQbAKiDkIqqUtNTHk8J3O9wxmJEPcY0Ko.gif>

—

Reply to this email directly or view it on GitHubhttps://github.com//issues/64#issuecomment-24206058
.

carterpage commented

Oh just saw your second email. Please just submit a pull request. That's
the easiest way for me to do the code review.

On Sun, Sep 22, 2013 at 7:00 PM, Carter Page cpage@pobox.com wrote:

Sorry for the delay, but I was on vacation. I like the approach. I look
forward to seeing the pull request once it's done!

On Tue, Sep 10, 2013 at 5:53 PM, arranbartish notifications@github.comwrote:

Hi Carter,

To bounce my thoughts off you. I am thinking of adding a new interface
ResettableMock which provides a method void reset(). The advantage of
providing the interface is that you can use it in a couple of ways in tests

Unit tests using SpringJUnit4ClassRunner

@Autowired

private List allResetableMocks

Test frameworks like mine

final Map<String, ResettableMock> allResetableMocksManagedBySpring =
((ListableBeanFactory)context).getBeansOfType(ResettableMock.class);

In both cases then it’s quite convenient to either iterate and call
reset, or target a specific bean(s).

I see three classes that I am targeting for this.

  1. MockSQSConnector is the obvious choice as it has a reference to all
    the Mock queues however it doesn’t have access to the internal lists so
    rather than expose the list out I want assign the interface ResettableMock
    to the MockSQSQueue as well. This way the reset method on the
    MockSQSConnector can just create a set of all MockSQSQueue iterate through
    and call reset on each.
  2. MockSQSQueue so that it provides a reset method and maintains control
    over its internal List. The method would simply call clear on the list.
    Though other things of note would be that the method is Synchronised and
    would only clear the list if the queue was not deleted. If the queue is
    deleted then it has been removed from the MockSQSConnector anyway.

That is as far as I wanted to go. However after inspecting the code I
realised that in the cases of the MockSQSConnector and MockSQSQueue all
instances are created as a new and the Spring context would have no
visibility of them. This would mean that the @Autowired annotation and any
calls to a ListableBeanFactory would not return or find any beans.

  1. MockSQSConnectorFactory is the only class that seems to get managed by
    spring and as such I would also target this with a ResettableMock. The
    implementation would simply call reset on the internal MockSQSConnector.

I believe these changes;

  1. Are in the spirit of the interface as each of these three classes keep
    some form of state or stateful bean that could be reset. There is only one
    other Mock class MockSQSMessage however this is a simple bean that keeps no
    real state and are a create and throw away bean as opposed to the other
    three which may stick around through the life of the spring context.
  2. Achieve the desire result by calling reset on all ResettableMocks
    returning them to an empty state.
  3. Nonintrusive as by adding an interface to the mock classes it simply
    enriches their behaviour in a consistent way without impacting anything
    that doesn’t care about mocks.

Testing

As most of the classes (MockSQSConnector, MockSQSQueue) are managed
outside of spring I am going to test them with plain old unit tests and
mock objects. Without a mocking framework I’ll use some simple mock
instances to test their behaviour. I will add some new tests to the
ConnectionFactoryEndpointTest which will be integration form unit tests of
the reset.

Any thoughts or suggestions before I crack on?

Thanks,

Arran

From: Carter Page [mailto:notifications@github.com]
Sent: Tuesday, 10 September 2013 5:10 PM
To: skyscreamer/nevado
Cc: arranbartish
Subject: Re: [nevado] Mechanism to teardown mock queues (#64)

Sounds like a useful thing to have in the library. Basic rules for
contributions are 1) provide something useful (achieved), 2) don't break
backwards functionality, and 3) provide unit tests.

Thanks,
Carter

On Tue, Sep 10, 2013 at 2:14 PM, arranbartish <notifications@github.com<mailto:
notifications@github.com> >wrote:

As I've said in emails recently. Exceptional work.

In some test cases there can be messages intentionally left in the mock
queues at the end of the test. Currently there is no mechanism on the
MockSQSConnector to "reset" the queues (teardown style) ready for the
next
test. The impact of this is that following tests can be impacted by
these
messages "randomly" which can break tests designed to be atomic.

As I have already stated in email, I've started to put something in
place.
Is this something you would be interested in me contributing back?

Thanks again.

—
Reply to this email directly or view it on GitHub<
https://github.com/skyscreamer/nevado/issues/64>
.

—
Reply to this email directly or view it on GitHub <
https://github.com/skyscreamer/nevado/issues/64#issuecomment-24194850> .
<
https://github.com/notifications/beacon/CGSywq1gaKEoKthlhNZbYrXKHecjZO4LQbAKiDkIqqUtNTHk8J3O9wxmJEPcY0Ko.gif>

—

Reply to this email directly or view it on GitHubhttps://github.com//issues/64#issuecomment-24206058
.

carterpage commented

Version 1.2.6 has been cut. It should propagate to repos by the end of the week. Thanks for the contrib!