Tidy up test runner #246
Description
The test runner was converted from shell scripts to python in #244 . While a developer doesn't need arcane shell knowledge to modify this code anymore, it's not exactly pythonic either. @SamirTalwar proposed the following changes:
Why not use docker-py instead of shelling out to Docker everywhere?
Note from Sergio: I've got some reservations about this one. Currently we are dependency free, and I think it's maybe better to avoid having "yet another Pipenv environment" just for this. Also, the command line is probably more stable than the python api, and maybe the python api needs to be kept in sync with the docker installation. The less dependencies the better and I don't see that calling the libraries instead of calling the executable makes a big change in this context.
But you might be right. And maybe we could reuse the pipenv from the controller. Will give this a think.
Break test_utils.py down. At a glance, I can see the following files extracted:
- constants.py
- docker.py
- execution.py (assuming you can't just get rid of it)
- log.py
- test_context.py (not "cleanup context; that's the action, not the purpose)
execute_command everywhere is a smell. I'd like to limit it to the things we just can't do with Python, and isolate it to those few areas. Docker stuff can be done with docker-py, and file manipulation stuff with shutil.
Executing git diff probably is better done with the shell than a library, but even so, it could go into git.py and just use subprocess.run.