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

ISPN-4478 ServerFailureRetrySingleOwnerTest should remove listener #2692

Closed
wants to merge 1 commit into from

Conversation

galderz
Copy link
Member

@galderz galderz commented Jul 3, 2014

  • Before, the error inducing listener was not being removed, and that was not appearing as a result that each test was creating a new bunch of cache managers. Now, clean up is done after the test class has finished and the listeners are correctly removed.

public class ServerFailureRetrySingleOwnerTest extends AbstractRetryTest {

public ServerFailureRetrySingleOwnerTest() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the cleanup required after test still? I would think if we are properly removing the listener we should okay or is it that data is an issue as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It speeds the test hugely by not starting/stopping a cache manager for each test. This is independent of the actual fix.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I for some reason got AFTER_TEST and AFTER_METHOD mixed up :) We are on the same page :)

* Before, the error inducing listener was not being removed, and that
was not appearing as a result that each test was creating a new bunch
of cache managers. Now, clean up is done after the test class has
finished and the listeners are correctly removed.
@wburns
Copy link
Member

wburns commented Jul 17, 2014

Pulling...

@wburns
Copy link
Member

wburns commented Jul 17, 2014

Integrated into master, thanks @galderz !

@wburns wburns closed this Jul 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants