From 9b92f96ed0dd461dca6a37821af880719afdd350 Mon Sep 17 00:00:00 2001 From: Ben Picolo Date: Wed, 4 Jan 2017 10:35:00 -0500 Subject: [PATCH] Code cleanup and tests --- .travis.yml | 4 +- CONTRIBUTING.md | 5 +- pre_commit/languages/docker.py | 60 +++++++++++++------ .../resources/docker_hooks_repo/Dockerfile | 3 + .../resources/docker_hooks_repo/hooks.yaml | 11 ++++ testing/util.py | 6 ++ tests/repository_test.py | 24 ++++++++ 7 files changed, 92 insertions(+), 21 deletions(-) create mode 100644 testing/resources/docker_hooks_repo/Dockerfile create mode 100644 testing/resources/docker_hooks_repo/hooks.yaml diff --git a/.travis.yml b/.travis.yml index c5f989b5..a01003c3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,7 +20,9 @@ before_install: - git --version after_success: - coveralls -sudo: false +sudo: required +services: + - docker cache: directories: - $HOME/.cache/pip diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a5170902..1e0b2460 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,7 +2,7 @@ ## Local development -- The tests depend on having at least the following installed (possibly not +- The complete test suite depends on having at least the following installed (possibly not a complete list) - git (A sufficiently newer version is required to run pre-push tests) - python @@ -10,6 +10,7 @@ - python3.5 (Required by a test which checks different python versions) - tox (or virtualenv) - ruby + gem + - docker ### Setting up an environemnt @@ -63,7 +64,7 @@ function_call( ``` Some notable features: -- The intial parenthese is at the end of the line +- The initial parenthesis is at the end of the line - Parameters are indented one indentation level further than the function name - The last parameter contains a trailing comma (This helps make `git blame` more accurate and reduces merge conflicts when adding / removing parameters). diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index afb16831..bc4d0623 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -1,8 +1,10 @@ +from __future__ import absolute_import from __future__ import unicode_literals import hashlib import os +from pre_commit import five from pre_commit.languages import helpers from pre_commit.util import clean_path_on_failure from pre_commit.util import mkdirp @@ -10,12 +12,11 @@ from pre_commit.xargs import xargs ENVIRONMENT_DIR = 'docker' +PRE_COMMIT_LABEL = 'PRE_COMMIT' def md5(s): - m = hashlib.md5() - m.update(s) - return m.hexdigest() + return five.to_text(hashlib.md5(s).hexdigest()) def docker_tag(repo_cmd_runner): @@ -24,39 +25,62 @@ def docker_tag(repo_cmd_runner): ).lower() +def docker_is_running(): + return xargs(('docker',), ['ps'])[0] == 0 + + +def assert_docker_available(): + assert docker_is_running(), ( + 'Docker is either not running or not configured in this environment' + ) + + +def build_docker_image(repo_cmd_runner): + cmd = ( + 'docker', 'build', '--pull', + '--tag', docker_tag(repo_cmd_runner), + '--label', PRE_COMMIT_LABEL, + '.' + ) + helpers.run_setup_cmd(repo_cmd_runner, cmd) + + def install_environment( repo_cmd_runner, version='default', additional_dependencies=(), ): - assert repo_cmd_runner.exists('Dockerfile') - # I don't know of anybody trying to juggle multiple docker installations - # so this seems sufficient + assert repo_cmd_runner.exists('Dockerfile'), ( + 'No Dockerfile was found in the hook repository' + ) + assert version == 'default', ( + 'Pre-commit does not support language_version for docker ' + ) + assert_docker_available() + directory = helpers.environment_dir(ENVIRONMENT_DIR, 'default') mkdirp(os.path.join(repo_cmd_runner.path(), directory)) - cmd = ( - 'docker', 'build', '--pull', - '--tag', docker_tag(repo_cmd_runner), - '.' - ) - # Docker doesn't really have relevant disk environment, but pre-commit # still needs to cleanup it's state files on failure env_dir = repo_cmd_runner.path(directory) with clean_path_on_failure(env_dir): - helpers.run_setup_cmd(repo_cmd_runner, cmd) + build_docker_image(repo_cmd_runner) def run_hook(repo_cmd_runner, hook, file_args): + assert_docker_available() + # Rebuild the docker image in case it has gone missing, as many people do + # automated cleanup of docker images. + build_docker_image(repo_cmd_runner) + # the docker lib doesn't return stdout on non-zero exit codes, + # so we run the container directly on the command line cmd = ( 'docker', 'run', - '-t', + '--rm', '-v', '{}:/src'.format(os.getcwd()), '--workdir', '/src', + '--entrypoint', hook['entry'], docker_tag(repo_cmd_runner) ) - - return xargs( - cmd + (hook['entry'],) + tuple(hook['args']), file_args, - ) + return xargs(cmd + tuple(hook['args']), file_args) diff --git a/testing/resources/docker_hooks_repo/Dockerfile b/testing/resources/docker_hooks_repo/Dockerfile new file mode 100644 index 00000000..acb5f54e --- /dev/null +++ b/testing/resources/docker_hooks_repo/Dockerfile @@ -0,0 +1,3 @@ +FROM cogniteev/echo + +CMD ["echo", "This is overwritten by the hooks.yaml 'entry'"] diff --git a/testing/resources/docker_hooks_repo/hooks.yaml b/testing/resources/docker_hooks_repo/hooks.yaml new file mode 100644 index 00000000..2c9a115d --- /dev/null +++ b/testing/resources/docker_hooks_repo/hooks.yaml @@ -0,0 +1,11 @@ +- id: docker-hook + name: Docker test hook + entry: echo + language: docker + files: \.txt$ + +- id: docker-hook-failing + name: Docker test hook with nonzero exit code + entry: bork + language: docker + files: \.txt$ diff --git a/testing/util.py b/testing/util.py index 10e84462..cf9dde9d 100644 --- a/testing/util.py +++ b/testing/util.py @@ -6,6 +6,7 @@ import shutil import jsonschema import pytest +from pre_commit.languages.docker import docker_is_running from pre_commit.util import cmd_output from pre_commit.util import cwd @@ -57,6 +58,11 @@ def cmd_output_mocked_pre_commit_home(*args, **kwargs): return cmd_output(*args, env=env, **kwargs) +skipif_cant_run_docker = pytest.mark.skipif( + docker_is_running() is False, + reason='Docker isn\'t running or can\'t be accessed' +) + skipif_slowtests_false = pytest.mark.skipif( os.environ.get('slowtests') == 'false', reason='slowtests=false', diff --git a/tests/repository_test.py b/tests/repository_test.py index f61ee88d..5d8ed8ab 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -29,6 +29,7 @@ from testing.fixtures import git_dir from testing.fixtures import make_config_from_repo from testing.fixtures import make_repo from testing.fixtures import modify_manifest +from testing.util import skipif_cant_run_docker from testing.util import skipif_slowtests_false from testing.util import xfailif_no_pcre_support from testing.util import xfailif_windows_no_node @@ -129,6 +130,29 @@ def test_versioned_python_hook(tempdir_factory, store): ) +@skipif_slowtests_false +@skipif_cant_run_docker +@pytest.mark.integration +def test_run_a_docker_hook(tempdir_factory, store): + _test_hook_repo( + tempdir_factory, store, 'docker_hooks_repo', + 'docker-hook', + ['Hello World from docker'], b'Hello World from docker\n', + ) + + +@skipif_slowtests_false +@skipif_cant_run_docker +@pytest.mark.integration +def test_run_a_failing_docker_hook(tempdir_factory, store): + _test_hook_repo( + tempdir_factory, store, 'docker_hooks_repo', + 'docker-hook-failing', + ['Hello World from docker'], b'', + expected_return_code=1 + ) + + @skipif_slowtests_false @xfailif_windows_no_node @pytest.mark.integration