Improve local mode

[davehughes]

This set of commits adds tests for some discrepancies between cuisine.run() and fabric.run() in local mode, then adds fixes for those discrepancies. Most changes have to do with wrapping commands the same way that fabric does so that various context managers and env variables get picked up and applied properly.

[sebastien]

Hi Dave! I had a look at your pull request and have a few comments/questions:

• I found that the new _run_command_local and _execute_local are rather complex for a benefit that is rather unclear to me. What exactly are the use cases where the current run_local doesn't work?
• The test failed after the pull because of the following exception

Traceback (most recent call last):
  File "tests/all.py", line 149, in testSudoPrefix
    run_result = cuisine.run(cmd)
  File "/home/sebastien/Projects/Local/lib/python/cuisine.py", line 237, in run
    return run_local(*args, **kwargs)
  File "/home/sebastien/Projects/Local/lib/python/cuisine.py", line 144, in run_local
    return _run_command_local(command, shell, combine_stderr, sudo)
  File "/home/sebastien/Projects/Local/lib/python/cuisine.py", line 153, in _run_command_local
    from fabric.operations import error
ImportError: cannot import name error

• last, the commit used spaces instead of tabs

Thanks!

[davehughes]

On the latter two points (issues with the patch):

• I will need to rework error handling to work with fabric versions previous to 1.4 (which I'm guessing you're using. The error() function seems to have been introduced in 1.4).
• I thought I had properly set vim up to use tabs this time, but I'll go back through and see what I messed up.

As far as the complexity of _run_command_local/_execute_local, I agree that they appear complex, but without the included steps, mode_local just won't transparently replace the handling that you'd get from fabric. The tests I included illustrate some of the cases, but one of the compelling ones is testCd:

def testCd( self ):  # the tests assume mode(MODE_LOCAL) == True
    with cd('/tmp'):
        self.assertEqual(cuisine.run('pwd'), '/tmp')

This actually fails in the current incarnation of cuisine, because run_local doesn't take any of the command-altering environment settings into account. _run_command_local is almost a direct copy of fabric.operations._run_command, with a goal of processing commands as identically to fabric as possible.

[sebastien]

Oh, I see -- this is great. Maybe more tests would be great to show the extend of features covered? I'm just a little bit worried about mimicking to closely how Fabric works, as we'll need to make sure that this actually works as expected (hence the need for a more extensive test suite).

Overall, it's a lot of work for something that looks to me as an edge case, but I guess it's a good thing to make cuisine.run behave as closely as possible to fabric's run, even in local mode.

[davehughes]

You're right, it does seem like a lot of work for something that's generally an edge case. I've been thinking about going back to just a normal 'remote' SSH connection to localhost for some of the more elaborate things I'm trying to do with mode_local. Still, I think it has its uses (especially in prototyping locally when you don't have access to test machines).

That last commit should fix that error handling import back to at least Fabric 1.1. Let me know if the tabs/spaces are still an issue, I'm starting to distrust my editor setup.

[sebastien]

OK, I'll merge it, thanks!

[sebastien]

OK, now the python tests/all.py hangs forever... it seems like I'll have to revert it, sorry.

[local] sudo: cat "/etc/passwd" | base64
Traceback (most recent call last):
  File "tests/all.py", line 213, in <module>
    unittest.main()
  File "/usr/lib/python2.7/unittest/main.py", line 95, in __init__
    self.runTests()
  File "/usr/lib/python2.7/unittest/main.py", line 229, in runTests
    self.result = testRunner.run(self.test)
  File "/usr/lib/python2.7/unittest/runner.py", line 151, in run
    test(result)
  File "/usr/lib/python2.7/unittest/suite.py", line 70, in __call__
    return self.run(*args, **kwds)
  File "/usr/lib/python2.7/unittest/suite.py", line 108, in run
    test(result)
  File "/usr/lib/python2.7/unittest/suite.py", line 70, in __call__
    return self.run(*args, **kwds)
  File "/usr/lib/python2.7/unittest/suite.py", line 108, in run
    test(result)
  File "/usr/lib/python2.7/unittest/case.py", line 391, in __call__
    return self.run(*args, **kwds)
  File "/usr/lib/python2.7/unittest/case.py", line 327, in run
    testMethod()
  File "tests/all.py", line 169, in testRead
    cuisine.file_read("/etc/passwd")
  File "/home/sebastien/Projects/Local/lib/python/cuisine.py", line 417, in file_read
    return base64.b64decode(run('cat "%s" | base64' % (location)))
  File "/home/sebastien/Projects/Local/lib/python/cuisine.py", line 245, in run
    return run_local(*args, **kwargs)
  File "/home/sebastien/Projects/Local/lib/python/cuisine.py", line 144, in run_local
    return _run_command_local(command, shell, combine_stderr, sudo)
  File "/home/sebastien/Projects/Local/lib/python/cuisine.py", line 191, in _run_command_local
    combine_stderr=combine_stderr)
  File "/home/sebastien/Projects/Local/lib/python/cuisine.py", line 234, in _execute_local
    out, err = process.communicate()
  File "/usr/lib/python2.7/subprocess.py", line 746, in communicate
    stdout = _eintr_retry_call(self.stdout.read)
  File "/usr/lib/python2.7/subprocess.py", line 478, in _eintr_retry_call
    return func(*args)
KeyboardInterrupt

[davehughes]

It works on my end, but if I had a dollar for every time I've heard that... :-)

What version of Fabric/Python are you running on? I'll try to reproduce and fix the issue if possible...

[sebastien]

I use Python 2.7.3 and Fabric 1.2.2 -- but I thought about it and I think I'll revert the pull request (and try to salvage as much as possible from what you did): my gut feeling was that it is a lot of complexity for really edge cases, which, as you suggested it, can be worked around with using a local SSH connection. What I will do though is to be explicit about the limitations of mode_local in the documentation and API doc.

I really liked how you updated the test suite!

[davehughes]

You'll probably want to remove the last four LocalExecution test methods when you revert, since they'll be testing things that don't actually work in mode_local without the elaborate _run_command_local stuff.

I've been thinking about trying to add more tests, because it's nice to have them to ensure that changes aren't breaking anything. The main issue I see is that most of the behavior you'd really want to test requires a remote test machine to run the tasks against. I've been doing my development using Vagrant, but that would be a pretty heavy dependency to just run tests. Have you given any thought to how to test cuisine's tasks remotely?