Replace SinkHandler by mocking object and improve test design

[wx930910]

Fixes PROTON-2431

Description

Refactor test class SinkHandler by mocking object and improve test design

---

Motivation

• Decoupling test class SinkHandler from production class BaseHandler.
• Making test condition more clear by verifying mocking obejct's behavior with a local variable.
• Making test logic more clear by using method stub instead of method overriding.

---

Key changed/added classes in this PR

• Created mocking object to replace test subclass SinkHandler, decoupled test from production code.
• Extract variable received as a local variable to improve test logic and make test condition more explict.
• Make test logic more clear by using method stub instead of method overriding.

---

[gemmellr]

I'm not sure I would actually consider this a great improvement overall. Its just as (if not more) verbose as the original but harder to understand from a simple glance, doing effectively the same thing but in a less trivially readable way. The use of BaseHandler in the original doesn't seem an issue given that is the way it was intended to be used (and remains used that way in other parts of the same test). There are other approaches the test could have originally taken to use a local variable for the count, which is what I might have done personally, but I would say the logic is trivial as-is and far from unclear even without doing so. Overall the Reactor is not a focus though either way and hasnt been for some time.

Aside, the used imports should have been added rather than a wildcard, the comments being added dont seem necessary and are more of a distraction than a help, and the alignment is clearly way off for most of the changed lines, presumably from use of tabs vs spaces in the existing file. The commit log and PR title should have referenced the JIRA key to link them. Not necessarily an issue currently as I don't plan to merge this as is.

[wx930910]

I'm not sure I would actually consider this a great improvement overall. Its just as (if not more) verbose as the original but harder to understand from a simple glance, doing effectively the same thing but in a less trivially readable way. The use of BaseHandler in the original doesn't seem an issue given that is the way it was intended to be used (and remains used that way in other parts of the same test). There are other approaches the test could have originally taken to use a local variable for the count, which is what I might have done personally, but I would say the logic is trivial as-is and far from unclear even without doing so. Overall the Reactor is not a focus though either way and hasnt been for some time.

Aside, the used imports should have been added rather than a wildcard, the comments being added dont seem necessary and are more of a distraction than a help, and the alignment is clearly way off for most of the changed lines, presumably from use of tabs vs spaces in the existing file. The commit log and PR title should have referenced the JIRA key to link them. Not necessarily an issue currently as I don't plan to merge this as is.

@gemmellr Thanks for the comments! if we totally get rid of the variable received and use Mockito.verify() instead of assertion to check method execution status, will it make the test logic more explict?