[GitHub] [samza] cameronlee314 commented on a change in pull request #934: SAMZA-1935 : Refactor TaskContextImpl to not include access to objects that are only used internally

[GitBox <gi...@apache.org>]

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


With regards,
Apache Git Services