> On Oct. 16, 2014, 5 p.m., Bill Farner wrote: > > src/test/python/apache/aurora/client/cli/test_aurora_command_line.py, line > > 34 > > <https://reviews.apache.org/r/26802/diff/1/?file=722892#file722892line34> > > > > I'm not sure what the right answer is here, and we may be too far down > > the monkeypatch road to change tacks in this effort, but overriding > > builtins feels like a special kind of abstraction violation. > > > > Do any other reviewers know the lay of the land well enough to suggest > > a better way? Ideally we would be injecting this functionallity rather > > than patching it, but i don't want to turn a small fix into a giant yak > > shave. > > Maxim Khutornenko wrote: > Seems like the appropriate use of patching given the current > implementation. Refactoring here would only hurt readability and would rather > feel unnatural.
+1, I'm not an expert on what is or isn't pythonic, but this certainly seems in line with how I've done testing in other dynamic languages. I'm not sure cluttering up the interface with explicit injection points for dependencies is warranted/necessary when you can just patch them in tests? - Joshua ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26802/#review56929 ----------------------------------------------------------- On Oct. 16, 2014, 6:02 a.m., Joe Smith wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26802/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2014, 6:02 a.m.) > > > Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji. > > > Bugs: AURORA-827 > https://issues.apache.org/jira/browse/AURORA-827 > > > Repository: aurora > > > Description > ------- > > Set a default for the error log dir > > This has been a weird issue to wrap my head around, it's really using lots > of low-level systems (writing tracebacks to files) so I feel like I've done > some weird things in the tests. Feedback and critque welcome + appreciated. > > > Diffs > ----- > > src/main/python/apache/aurora/client/cli/__init__.py > da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8 > src/test/python/apache/aurora/client/cli/BUILD > d33e86643a59879c115876c98bd1dc19aa7ae61c > src/test/python/apache/aurora/client/cli/test_aurora_command_line.py > PRE-CREATION > > Diff: https://reviews.apache.org/r/26802/diff/ > > > Testing > ------- > > [tw-172-25-132-201 aurora (yasumoto/error_log_dir_default)]$ ./pants > src/test/python/apache/aurora/client/cli:aurora_command_line > Build operating on top level addresses: > set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, > aurora_command_line)]) > ==================================================================== test > session starts > ===================================================================== > platform darwin -- Python 2.7.5 -- py-1.4.25 -- pytest-2.6.3 > plugins: cov, timeout > collected 1 items > > src/test/python/apache/aurora/client/cli/test_aurora_command_line.py . > > ================================================================== 1 passed > in 0.63 seconds > ================================================================== > src.test.python.apache.aurora.client.cli.aurora_command_line > ..... SUCCESS > > > Thanks, > > Joe Smith > >
