> On Oct. 27, 2014, 7: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)
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()`. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27182/#review58668 ----------------------------------------------------------- On Oct. 25, 2014, 12:12 a.m., Joe Smith wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27182/ > ----------------------------------------------------------- > > (Updated Oct. 25, 2014, 12:12 a.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-mbp13-jsmith aurora (yasumoto/psutil_2.1.3)]$ ./pants > ./src/test/python/apache/thermos/monitoring:test_resource > Build operating on top level addresses: > set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/thermos/monitoring/BUILD, > test_resource)]) > ==================================================================================================================================================== > 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.21 seconds > ================================================================================================================================================== > src.test.python.apache.thermos.monitoring.test_resource > ..... SUCCESS > > > Thanks, > > Joe Smith > >
