REGRESSION (r90564): webkitpy's Checkout class uses Executive inappropriately
Created attachment 100015 [details] Make Checkout use SCM's Executive instead of conjuring up its own
Comment on attachment 100015 [details] Make Checkout use SCM's Executive instead of conjuring up its own Thanks Adam.
Committed r90584: <http://trac.webkit.org/changeset/90584>
Comment on attachment 100015 [details] Make Checkout use SCM's Executive instead of conjuring up its own View in context: https://bugs.webkit.org/attachment.cgi?id=100015&action=review > Tools/Scripts/webkitpy/common/checkout/checkout.py:123 > + message_text = self._scm.run([self._scm.script_path('commit-log-editor'), '--print-log'] + changelog_paths, return_stderr=False) SCM's run may do more than you want... like setting the cwd. But should be OK. I would have probably used self._scm._executive directly (even though grabbing at _ members is ugly too). Part of the trouble here is that scm.py is some of the vey very oldest code in webkitpy, and is poorly tested and factored. Checkout is pretty old itself. We've gotten better at this whole mocking thing since then. Thanks again for the fix. > Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py:128 > + def mock_run(*args, **kwargs): > + # Note that we use a real Executive here, not a MockExecutive, so we can test that we're > + # invoking commit-log-editor correctly. > + return Executive().run_command(*args, **kwargs) > + I suspect what we wanted to do was make Checkout hold an self.executive pointer. But we can do that later when we go clean up the rest of Checkout too.