> On Oct. 27, 2014, 12:46 p.m., Kevin Sweeney wrote:
> > src/test/python/apache/thermos/monitoring/test_resource.py, line 64
> > <https://reviews.apache.org/r/27182/diff/1/?file=733141#file733141line64>
> >
> >     patching a private method assumes intimate knowledge of the class under 
> > test and suggests refactoring
> 
> Joe Smith wrote:
>     Not quite- if this were for a different test then I'd agree, but since 
> we're actually testing TaskResourceMonitor, we'll want to mock out that 
> behavior (and test it separately) to ensure this method is using it right.
>     
>     (At some point in the TestTaskResourceMonitor we'll need to mock out this 
> behavior- so there will always be a patch around this spot)
> 
> Bill Farner wrote:
>     I agree with Kevin, patching a private method is an encapsulation 
> violation, it should be used as a last resort.  Consider, for example, if the 
> true implementation of `_get_active_processes` handled internal state in a 
> way that is expected to be consistent with other methods in the class.  
> Swapping out the behavior compromises the encapsulation put in place.
>     
>     In this case, you should be mocking `TaskMonitor`, the external interface 
> consumed by the class, to supply behavior for `get_active_processes()`.

Done


- Joe


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27182/#review58668
-----------------------------------------------------------


On Feb. 5, 2015, 9:16 p.m., Joe Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27182/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2015, 9:16 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add a test for the thermos resource module
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/thermos/monitoring/monitor.py 
> 8f87f5ffc39c87e87ff78b941ea30df7138bd1ef 
>   src/test/python/apache/thermos/monitoring/BUILD 
> 33d6bba43aff6d62b2646491f004475c27ed99db 
>   src/test/python/apache/thermos/monitoring/test_resource.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/27182/diff/
> 
> 
> Testing
> -------
> 
> [tw-mbp-jsmith incubator-aurora (yasumoto/thermos_test)]$ ./pants test.pytest 
> ./src/test/python/apache/thermos/monitoring:test_resource
> 
> 21:15:56 00:00 [main]
>                (To run a reporting server: ./pants server)
> 21:15:56 00:00   [bootstrap]
> 21:15:57 00:01   [setup]
> 21:15:57 00:01     [parse]
>                Executing tasks in goals: bootstrap -> imports -> gen -> 
> check-exclusives -> resolve -> compile -> resources -> test
> 21:15:57 00:01   [bootstrap]
> 21:15:57 00:01     [bootstrap-jvm-tools]
> 21:15:57 00:01   [imports]
> 21:15:57 00:01     [ivy-imports]
> 21:15:57 00:01   [gen]
> 21:15:57 00:01     [thrift]
> 21:15:57 00:01     [scrooge]
> 21:15:57 00:01     [protoc]
> 21:15:57 00:01     [antlr]
> 21:15:57 00:01     [ragel]
> 21:15:57 00:01     [jaxb]
> 21:15:57 00:01     [wire]
> 21:15:57 00:01     [aapt]
> 21:15:57 00:01   [check-exclusives]
> 21:15:57 00:01     [check-exclusives]
> 21:15:57 00:01   [resolve]
> 21:15:57 00:01     [ivy]
> 21:15:57 00:01   [compile]
> 21:15:57 00:01     [jvm]
> 21:15:57 00:01       [jvm-compilers]
> 21:15:57 00:01   [resources]
> 21:15:57 00:01     [prepare]
> 21:15:57 00:01   [test]
> 21:15:57 00:01     [run_prep_command]
> 21:15:57 00:01     [pytest]
> 21:15:57 00:01       [run]
>                      ============== test session starts ===============
>                      platform darwin -- Python 2.7.6 -- py-1.4.26 -- 
> pytest-2.6.4
>                      plugins: cov, timeout
>                      collected 5 items 
>                      
>                      
> src/test/python/apache/thermos/monitoring/test_resource.py .....
>                      
>                      ============ 5 passed in 0.18 seconds ============
>                      
> 21:16:04 00:08     [junit]
> 21:16:04 00:08     [specs]
>                SUCCESS
> 
> 
> Thanks,
> 
> Joe Smith
> 
>

Reply via email to