cameronlee314 commented on a change in pull request #934: SAMZA-1935 : Refactor
TaskContextImpl to not include access to objects that are only used internally
URL: https://github.com/apache/samza/pull/934#discussion_r262181169
##########
File path:
samza-core/src/test/java/org/apache/samza/operators/impl/TestWindowOperator.java
##########
@@ -436,13 +434,6 @@ public void testCancelationOfRepeatingNestedTriggers()
throws Exception {
@Test
public void testEndOfStreamFlushesWithEarlyTriggerFirings() throws Exception
{
- EndOfStreamStates endOfStreamStates = new
EndOfStreamStates(ImmutableSet.of(new SystemStreamPartition("kafka",
Review comment:
Sounds good. If it's not needed for the test, it is best to remove it.
In the future, if you ever run into a use case where you do need to mock
something that is instantiated inside the constructor, here are a couple of
things you could consider:
Create a second package-private constructor in the class which accepts the
object you need to mock. The test class should have the same package as the
class under test, so the test class should be able to access this second
constructor. Guava has an annotation called @VisibleForTesting which you could
use to mark this second constructor. Any tests can use this second constructor
to inject a mock. Unfortunately, this pattern does mean you need some test-only
code in your class, which is generally not ideal.
Another thing you could look into is refactoring the class so that it does
not instantiate anything inside the constructor. Everything it needs gets
passed in directly. In my opinion, this is probably the ideal state, but
sometimes it is not practical or not possible to refactor something that way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
