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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to