Skip to content

Commit

Permalink
Rewrite test_git_hooks.py to be hermetic (#8085)
Browse files Browse the repository at this point in the history
This test failed when remoting because it relied on access to several scripts in `build-support/bin` that were not explicitly declared.

This is fixed by explicitly depending on the scripts. We must jump through some hoops to then copy the scripts into the temporary workdir created by the tests.
  • Loading branch information
Eric-Arellano authored Jul 26, 2019
1 parent b5d41da commit 44c2741
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 70 deletions.
27 changes: 16 additions & 11 deletions build-support/bin/BUILD
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_library(
name = 'common',
sources = 'common.py',
tags = {'type_checked'},
files(
name = 'bash_scripts',
sources = globs('*.sh'),
)

python_binary(
name = 'check_banned_imports',
sources = 'check_banned_imports.py',
source = 'check_banned_imports.py',
dependencies = [
':common',
],
Expand All @@ -18,7 +17,7 @@ python_binary(

python_binary(
name = 'check_header',
sources = 'check_header.py',
source = 'check_header.py',
dependencies = [
':common',
],
Expand All @@ -27,7 +26,7 @@ python_binary(

python_binary(
name = 'check_pants_pex_abi',
sources = 'check_pants_pex_abi.py',
source = 'check_pants_pex_abi.py',
dependencies = [
':common',
],
Expand All @@ -36,16 +35,22 @@ python_binary(

python_binary(
name = 'ci',
sources = 'ci.py',
source = 'ci.py',
dependencies = [
':common',
],
tags = {'type_checked'},
)

python_library(
name = 'common',
source = 'common.py',
tags = {'type_checked'},
)

python_binary(
name = 'deploy_to_s3',
sources = 'deploy_to_s3.py',
source = 'deploy_to_s3.py',
dependencies = [
':common',
],
Expand All @@ -64,13 +69,13 @@ python_binary(

python_binary(
name = 'mypy',
sources = 'mypy.py',
source = 'mypy.py',
tags = {'type_checked'},
)

python_binary(
name = 'shellcheck',
sources = 'shellcheck.py',
source = 'shellcheck.py',
dependencies = [
':common',
],
Expand Down
1 change: 0 additions & 1 deletion build-support/unit_test_remote_blacklist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,3 @@ tests/python/pants_test/ivy:bootstrapper
tests/python/pants_test/ivy:ivy_subsystem
tests/python/pants_test/java/distribution:distribution
tests/python/pants_test/option:testing
tests/python/pants_test/repo_scripts:git_hooks
2 changes: 2 additions & 0 deletions tests/python/pants_test/repo_scripts/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,7 @@ python_tests(
'src/python/pants/util:contextutil',
'src/python/pants/util:dirutil',
'tests/python/pants_test/testutils:git_util',
'build-support/bin:bash_scripts',
'build-support/bin:check_header',
]
)
73 changes: 37 additions & 36 deletions tests/python/pants_test/repo_scripts/test_git_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,56 +3,59 @@

import datetime
import os
import shutil
import subprocess
import unittest
from contextlib import contextmanager
from pathlib import Path
from textwrap import dedent
from typing import Optional, Sequence

from pants.util.contextutil import temporary_dir
from pants.util.dirutil import safe_file_dump, touch
from pants_test.testutils.git_util import get_repo_root, initialize_repo
from pants.util.dirutil import safe_file_dump, safe_mkdir_for
from pants_test.testutils.git_util import initialize_repo


class PreCommitHookTest(unittest.TestCase):

def setUp(self):
self.pants_repo_root = get_repo_root()

@contextmanager
def _create_tiny_git_repo(self):
def _create_tiny_git_repo(self, *, copy_files: Optional[Sequence[Path]] = None):
with temporary_dir() as gitdir,\
temporary_dir() as worktree:
# A tiny little fake git repo we will set up. initialize_repo() requires at least one file.
readme_file = os.path.join(worktree, 'README')
touch(readme_file)
(Path(worktree) / 'README').touch()
# The contextmanager interface is only necessary if an explicit gitdir is not provided.
with initialize_repo(worktree, gitdir=gitdir) as git:
if copy_files is not None:
for fp in copy_files:
new_fp = Path(worktree) / fp
safe_mkdir_for(str(new_fp))
shutil.copy(fp, new_fp)
yield git, worktree, gitdir

def _assert_subprocess_error(self, worktree, cmd, expected_excerpt):
proc = subprocess.Popen(
result = subprocess.run(
cmd,
cwd=worktree,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
encoding="utf-8",
)
(stdout_data, stderr_data) = proc.communicate()
stdout_data = stdout_data.decode()
stderr_data = stderr_data.decode()
self.assertNotEqual(0, proc.returncode)
all_output = '{}\n{}'.format(stdout_data, stderr_data)
self.assertIn(expected_excerpt, all_output)
self.assertNotEqual(0, result.returncode)
self.assertIn(expected_excerpt, f'{result.stdout}\n{result.stderr}')

def _assert_subprocess_success(self, worktree, cmd, **kwargs):
self.assertEqual(0, subprocess.check_call(cmd, cwd=worktree, **kwargs))

def _assert_subprocess_success_with_output(self, worktree, cmd, full_expected_output):
output = subprocess.check_output(cmd, cwd=worktree)
self.assertEqual(full_expected_output, output.decode())
stdout = subprocess.run(
cmd, cwd=worktree, check=True, stdout=subprocess.PIPE, encoding="utf-8"
).stdout
self.assertEqual(full_expected_output, stdout)

def test_check_packages(self):
package_check_script = os.path.join(self.pants_repo_root, 'build-support/bin/check_packages.sh')
with self._create_tiny_git_repo() as (_, worktree, _):
package_check_script = 'build-support/bin/check_packages.sh'
with self._create_tiny_git_repo(copy_files=[Path(package_check_script)]) as (_, worktree, _):
init_py_path = os.path.join(worktree, 'subdir/__init__.py')

# Check that an invalid __init__.py errors.
Expand All @@ -77,9 +80,8 @@ def test_check_packages(self):
# more testing git functionality, but since it's not clear how this is measured, it could be
# useful if correctly detecting copies and moves ever becomes a concern.
def test_added_files_correctly_detected(self):
get_added_files_script = os.path.join(self.pants_repo_root,
'build-support/bin/get_added_files.sh')
with self._create_tiny_git_repo() as (git, worktree, _):
get_added_files_script = 'build-support/bin/get_added_files.sh'
with self._create_tiny_git_repo(copy_files=[Path(get_added_files_script)]) as (git, worktree, _):
# Create a new file.
new_file = os.path.join(worktree, 'wow.txt')
safe_file_dump(new_file, '')
Expand All @@ -89,15 +91,15 @@ def test_added_files_correctly_detected(self):
self._assert_subprocess_success_with_output(
worktree, [get_added_files_script],
# This should be the only entry in the index, and it is a newly added file.
full_expected_output="{}\n".format(rel_new_file))
full_expected_output=f"{rel_new_file}\n")

def test_check_headers(self):
header_check_script = os.path.join(
self.pants_repo_root, 'build-support/bin/check_header.py'
)
header_check_script = 'build-support/bin/check_header.py'
cur_year_num = datetime.datetime.now().year
cur_year = str(cur_year_num)
with self._create_tiny_git_repo() as (_, worktree, _):
with self._create_tiny_git_repo(
copy_files=[Path(header_check_script), "build-support/bin/common.py"]
) as (_, worktree, _):
new_py_path = os.path.join(worktree, 'subdir/file.py')

def assert_header_check(added_files, expected_excerpt):
Expand All @@ -122,12 +124,11 @@ def assert_header_check(added_files, expected_excerpt):
)

# Check that a file with a typo in the header fails
safe_file_dump(new_py_path, dedent("""\
# Copyright {} Pants project contributors (see CONTRIBUTORS.md).
safe_file_dump(new_py_path, dedent(f"""\
# Copyright {cur_year} Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the MIT License, Version 3.3 (see LICENSE).
""".format(cur_year))
)
"""))
assert_header_check(
added_files=[],
expected_excerpt="subdir/file.py: header does not match the expected header"
Expand All @@ -144,22 +145,22 @@ def assert_header_check(added_files, expected_excerpt):
added_files=[],
expected_excerpt=(
r"subdir/file.py: copyright year must match '20\d\d' (was YYYY): "
"current year is {}".format(cur_year)
f"current year is {cur_year}"
)
)

# Check that a newly added file must have the current year.
last_year = str(cur_year_num - 1)
safe_file_dump(new_py_path, dedent("""\
# Copyright {} Pants project contributors (see CONTRIBUTORS.md).
safe_file_dump(new_py_path, dedent(f"""\
# Copyright {last_year} Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).
""".format(last_year))
""")
)
rel_new_py_path = os.path.relpath(new_py_path, worktree)
assert_header_check(
added_files=[rel_new_py_path],
expected_excerpt="subdir/file.py: copyright year must be {} (was {})".format(cur_year, last_year)
expected_excerpt=f"subdir/file.py: copyright year must be {cur_year} (was {last_year})"
)

# Check that a file isn't checked against the current year if it is not passed as an
Expand Down
40 changes: 18 additions & 22 deletions tests/python/pants_test/testutils/git_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import re
import subprocess
from contextlib import contextmanager
from typing import Iterator, Optional

from pants.base.revision import Revision
from pants.scm.git import Git
Expand All @@ -13,50 +14,45 @@
MIN_REQUIRED_GIT_VERSION = Revision.semver('1.7.10')


def git_version():
def git_version() -> Revision:
"""Get a Version() based on installed command-line git's version"""
process = subprocess.Popen(['git', '--version'], stdout=subprocess.PIPE)
(stdout, stderr) = process.communicate()
assert process.returncode == 0, "Failed to determine git version."
stdout = subprocess.run(
['git', '--version'], stdout=subprocess.PIPE, encoding="utf-8", check=True
).stdout
# stdout is like 'git version 1.9.1.598.g9119e8b\n' We want '1.9.1.598'
matches = re.search(r'\s(\d+(?:\.\d+)*)[\s\.]', stdout.decode())
matches = re.search(r'\s(\d+(?:\.\d+)*)[\s\.]', stdout)
if matches is None:
raise ValueError(f"Not able to parse git version from {stdout}.")
return Revision.lenient(matches.group(1))


def get_repo_root():
"""Return the absolute path to the root directory of the Pants git repo."""
return subprocess.check_output(['git', 'rev-parse', '--show-toplevel']).strip().decode()


@contextmanager
def initialize_repo(worktree, gitdir=None):
def initialize_repo(worktree: str, *, gitdir: Optional[str] = None) -> Iterator[Git]:
"""Initialize a git repository for the given `worktree`.
NB: The given `worktree` must contain at least one file which will be committed to form an initial
commit.
:param string worktree: The path to the git work tree.
:param string gitdir: An optional path to the `.git` dir to use.
:param worktree: The path to the git work tree.
:param gitdir: An optional path to the `.git` dir to use.
:returns: A `Git` repository object that can be used to interact with the repo.
:rtype: :class:`pants.scm.git.Git`
"""
@contextmanager
def use_gitdir():
def use_gitdir() -> Iterator[str]:
if gitdir:
yield gitdir
else:
with temporary_dir() as d:
yield d

with use_gitdir() as git_dir, environment_as(GIT_DIR=git_dir, GIT_WORK_TREE=worktree):
subprocess.check_call(['git', 'init'])
subprocess.check_call(['git', 'config', 'user.email', '[email protected]'])
subprocess.run(['git', 'init'], check=True)
subprocess.run(['git', 'config', 'user.email', '[email protected]'], check=True)
# TODO: This method inherits the global git settings, so if a developer has gpg signing on, this
# will turn that off. We should probably just disable reading from the global config somehow:
# https://git-scm.com/docs/git-config.
subprocess.check_call(['git', 'config', 'commit.gpgSign', 'false'])
subprocess.check_call(['git', 'config', 'user.name', 'Your Name'])
subprocess.check_call(['git', 'add', '.'])
subprocess.check_call(['git', 'commit', '-am', 'Add project files.'])

subprocess.run(['git', 'config', 'commit.gpgSign', 'false'], check=True)
subprocess.run(['git', 'config', 'user.name', 'Your Name'], check=True)
subprocess.run(['git', 'add', '.'], check=True)
subprocess.run(['git', 'commit', '-am', 'Add project files.'], check=True)
yield Git(gitdir=git_dir, worktree=worktree)

0 comments on commit 44c2741

Please sign in to comment.