Skip to content

Commit

Permalink
Add check/pytest-changed-files-and-incremental-coverage (#2510)
Browse files Browse the repository at this point in the history 
- Checks if changed lines are covered by tests associated with changed files (e.g. modifying x.py results in x_test.py being included)
- This check is slightly stricter than the usual incremental coverage check (because it runs a subset of all tests) but MUCH faster
- Fixed pytest-changed-files deduping by piping into uniq without first piping into sort
- Refactored run_pytest_and_incremental_coverage.py into check_incremental_coverage_annotations.py
  • Loading branch information
@Strilanc @CirqBot
Strilanc authored and CirqBot committed Nov 13, 2019
1 parent a167b64 commit 0a5d3d4
Show file tree
Hide file tree
Showing 9 changed files with 262 additions and 168 deletions.
8 changes: 4 additions & 4 deletions check/all
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@ echo "Running mypy"
check/mypy

echo "Running incremental format"
check/format-incremental ${rev} "${apply_arg}"
check/format-incremental "${rev}" "${apply_arg}"

if [ -z "${only_changed}" ]; then
echo "Running pytest on changed files without incremental coverage"
check/pytest-changed-files ${rev}
echo "Running pytest and incremental coverage on changed files"
check/pytest-changed-files-and-incremental-coverage "${rev}"
else
echo "Running pytest and incremental coverage"
check/pytest-and-incremental-coverage ${rev}
check/pytest-and-incremental-coverage "${rev}"
fi

echo "Running doctest"
Expand Down
25 changes: 19 additions & 6 deletions check/pytest-and-incremental-coverage
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
################################################################################

# Get the working directory to the repo root.
cd "$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
cd "$(git rev-parse --show-toplevel)"
cd "$( dirname "${BASH_SOURCE[0]}" )" || exit 1
cd "$(git rev-parse --show-toplevel)" || exit 1

# Figure out which revision to compare against.
if [ ! -z "$1" ] && [[ $1 != -* ]]; then
Expand All @@ -51,11 +51,24 @@ else
rev="${base}"
fi

# Run it.
PYTHONPATH="$(pwd)" python dev_tools/run_pytest_and_incremental_coverage.py "${rev}"
result=$?
# Run tests while producing coverage files.
check/pytest . \
--actually-quiet \
--cov \
--cov-report=annotate \
--cov-config=dev_tools/conf/.coveragerc \
--benchmark-skip
pytest_result=$?

# Analyze coverage files.
PYTHONPATH="$(pwd)" python dev_tools/check_incremental_coverage_annotations.py "${rev}"
cover_result=$?

# Clean up generated coverage files.
find . | grep "\.py,cover$" | xargs rm -f

exit ${result}
# Report result.
if [ "${pytest_result}" -ne "0" ] || [ "${cover_result}" -ne "0" ]; then
exit 1
fi
exit 0
1 change: 1 addition & 0 deletions check/pytest-changed-files
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ changed=$(git diff --name-only ${rev} -- \
| sed -e "s/\.py$/_test.py/" \
| sed -e "s/_test_test\.py$/_test.py/" \
| perl -ne 'chomp(); if (-e $_) {print "$_\n"}' \
| sort \
| uniq \
)
num_changed=$(echo -e "${changed}" | wc -w)
Expand Down
101 changes: 101 additions & 0 deletions check/pytest-changed-files-and-incremental-coverage
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
#!/usr/bin/env bash

################################################################################
# Finds changed lines not covered by tests related to changed files.
#
# Usage:
# check/pytest-changed-files-and-incremental-coverage [BASE_REVISION]
#
# This tool is not clever; it does not understand dependencies between all
# files. If "d/x.py" or "d/x_test.py" have been modified, it will include
# "d/x_test.py" in the arguments given to pytest. It will furthermore include
# "--cov=d" as an argument to pytest-cov, to restrict coverage results to
# directories containing changed files.
#
# You can specify a base git revision to compare against (i.e. to use when
# determining whether or not a line is considered to have "changed"). To make
# the tool more consistent, it actually diffs against the most recent common
# ancestor of the specified id and HEAD. So if you choose 'origin/master' you're
# actually diffing against the output of 'git merge-base origin/master HEAD'.
#
# If you don't specify a base revision, the following defaults will be tried,
# in order, until one exists:
#
# 1. upstream/master
# 2. origin/master
# 3. master
#
# If none exists, the script fails.
################################################################################

# Get the working directory to the repo root.
cd "$( dirname "${BASH_SOURCE[0]}" )" || exit 1
cd "$(git rev-parse --show-toplevel)" || exit 1

# Figure out which revision to compare against.
if [ ! -z "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t $1 2> /dev/null)" != "commit" ]; then
echo -e "\033[31mNo revision '$1'.\033[0m" >&2
exit 1
fi
rev=$1
elif [ "$(git cat-file -t upstream/master 2> /dev/null)" == "commit" ]; then
rev=upstream/master
elif [ "$(git cat-file -t origin/master 2> /dev/null)" == "commit" ]; then
rev=origin/master
elif [ "$(git cat-file -t master 2> /dev/null)" == "commit" ]; then
rev=master
else
echo -e "\033[31mNo default revision found to compare against. Argument #1 must be what to diff against (e.g. 'origin/master' or 'HEAD~1').\033[0m" >&2
exit 1
fi
base="$(git merge-base ${rev} HEAD)"
if [ "$(git rev-parse ${rev})" == "${base}" ]; then
echo -e "Comparing against revision '${rev}'." >&2
else
echo -e "Comparing against revision '${rev}' (merge base ${base})." >&2
rev="${base}"
fi

# Find involved files.
cov_changed_python_file_dirs=$(git diff --name-only "${rev}" -- \
| grep "\.py$" \
| grep -v "_pb2\.py$" \
| xargs dirname \
| perl -ne 'chomp(); if (-e $_) {print "--cov=$_\n"}' \
| sort \
| uniq \
)
changed_python_tests=$(git diff --name-only "${rev}" -- \
| grep "\.py$" \
| sed -e "s/\.py$/_test.py/" \
| sed -e "s/_test_test\.py$/_test.py/" \
| perl -ne 'chomp(); if (-e $_) {print "$_\n"}' \
| sort \
| uniq \
)
if [ "${#changed_python_tests[@]}" -eq 0 ]; then
echo -e "\033[33mNo changed files with associated python tests.\033[0m" >&2
exit 0
fi

# Run tests while producing coverage files.
check/pytest "${changed_python_tests[@]}" \
"${cov_changed_python_file_dirs[@]}" \
--cov-report=annotate \
--cov-config=dev_tools/conf/.coveragerc \
--benchmark-skip
pytest_result=$?

# Analyze coverage files.
PYTHONPATH="$(pwd)" python dev_tools/check_incremental_coverage_annotations.py "${rev}"
cover_result=$?

# Clean up generated coverage files.
find . | grep "\.py,cover$" | xargs rm -f

# Report result.
if [ "${pytest_result}" -ne "0" ] || [ "${cover_result}" -ne "0" ]; then
exit 1
fi
exit 0
73 changes: 54 additions & 19 deletions dev_tools/bash_scripts_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# limitations under the License.

import os
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, Iterable

from dev_tools import shell_tools

Expand All @@ -33,6 +33,7 @@ def run(
tmpdir_factory: '_pytest.tmpdir.TempdirFactory',
arg: str = '',
setup: str = '',
additional_intercepts: Iterable[str] = (),
) -> shell_tools.CommandOutput:
"""Invokes the given script within a temporary test environment."""

Expand All @@ -50,6 +51,7 @@ def run(
'pytest',
'mypy',
'yapf',
*additional_intercepts,
]
assert script_lines[0] == '#!/usr/bin/env bash\n'
for e in intercepted:
Expand Down Expand Up @@ -259,11 +261,15 @@ def test_pytest_changed_files_branch_selection(tmpdir_factory):
def test_pytest_and_incremental_coverage_branch_selection(tmpdir_factory):
result = run(script_file='check/pytest-and-incremental-coverage',
tmpdir_factory=tmpdir_factory,
arg='HEAD')
arg='HEAD',
additional_intercepts=['check/pytest'])
assert result.exit_code == 0
assert result.out == (
'INTERCEPTED python '
'dev_tools/run_pytest_and_incremental_coverage.py HEAD\n')
'INTERCEPTED check/pytest '
'. --actually-quiet --cov --cov-report=annotate '
'--cov-config=dev_tools/conf/.coveragerc --benchmark-skip\n'
'INTERCEPTED '
'python dev_tools/check_incremental_coverage_annotations.py HEAD\n')
assert result.err == "Comparing against revision 'HEAD'.\n"

result = run(script_file='check/pytest-and-incremental-coverage',
Expand All @@ -274,44 +280,61 @@ def test_pytest_and_incremental_coverage_branch_selection(tmpdir_factory):
assert "No revision 'HEAD~999999'." in result.err

result = run(script_file='check/pytest-and-incremental-coverage',
tmpdir_factory=tmpdir_factory)
tmpdir_factory=tmpdir_factory,
additional_intercepts=['check/pytest'])
assert result.exit_code == 0
assert result.out == (
'INTERCEPTED check/pytest '
'. --actually-quiet --cov --cov-report=annotate '
'--cov-config=dev_tools/conf/.coveragerc --benchmark-skip\n'
'INTERCEPTED python '
'dev_tools/run_pytest_and_incremental_coverage.py master\n')
'dev_tools/check_incremental_coverage_annotations.py master\n')
assert result.err == "Comparing against revision 'master'.\n"

result = run(script_file='check/pytest-and-incremental-coverage',
tmpdir_factory=tmpdir_factory,
setup='git branch origin/master')
setup='git branch origin/master',
additional_intercepts=['check/pytest'])
assert result.exit_code == 0
assert result.out == (
'INTERCEPTED check/pytest '
'. --actually-quiet --cov --cov-report=annotate '
'--cov-config=dev_tools/conf/.coveragerc --benchmark-skip\n'
'INTERCEPTED python '
'dev_tools/run_pytest_and_incremental_coverage.py origin/master\n')
'dev_tools/check_incremental_coverage_annotations.py origin/master\n')
assert result.err == "Comparing against revision 'origin/master'.\n"

result = run(script_file='check/pytest-and-incremental-coverage',
tmpdir_factory=tmpdir_factory,
setup='git branch upstream/master')
setup='git branch upstream/master',
additional_intercepts=['check/pytest'])
assert result.exit_code == 0
assert result.out == (
'INTERCEPTED check/pytest '
'. --actually-quiet --cov --cov-report=annotate '
'--cov-config=dev_tools/conf/.coveragerc --benchmark-skip\n'
'INTERCEPTED python '
'dev_tools/run_pytest_and_incremental_coverage.py upstream/master\n')
'dev_tools/check_incremental_coverage_annotations.py upstream/master\n')
assert result.err == "Comparing against revision 'upstream/master'.\n"

result = run(script_file='check/pytest-and-incremental-coverage',
tmpdir_factory=tmpdir_factory,
setup='git branch upstream/master; git branch origin/master')
setup='git branch upstream/master; git branch origin/master',
additional_intercepts=['check/pytest'])
assert result.exit_code == 0
assert result.out == (
'INTERCEPTED check/pytest '
'. --actually-quiet --cov --cov-report=annotate '
'--cov-config=dev_tools/conf/.coveragerc --benchmark-skip\n'
'INTERCEPTED python '
'dev_tools/run_pytest_and_incremental_coverage.py upstream/master\n')
'dev_tools/check_incremental_coverage_annotations.py upstream/master\n')
assert result.err == "Comparing against revision 'upstream/master'.\n"

result = run(script_file='check/pytest-and-incremental-coverage',
tmpdir_factory=tmpdir_factory,
setup='git checkout -b other --quiet\n'
'git branch -D master --quiet\n')
'git branch -D master --quiet\n',
additional_intercepts=['check/pytest'])
assert result.exit_code == 1
assert result.out == ''
assert 'No default revision found to compare against' in result.err
Expand All @@ -322,22 +345,30 @@ def test_pytest_and_incremental_coverage_branch_selection(tmpdir_factory):
arg='HEAD',
setup='touch HEAD\n'
'git add -A\n'
'git commit -m test --quiet --no-gpg-sign\n')
'git commit -m test --quiet --no-gpg-sign\n',
additional_intercepts=['check/pytest'])
assert result.exit_code == 0
assert result.out == (
'INTERCEPTED check/pytest '
'. --actually-quiet --cov --cov-report=annotate '
'--cov-config=dev_tools/conf/.coveragerc --benchmark-skip\n'
'INTERCEPTED python '
'dev_tools/run_pytest_and_incremental_coverage.py HEAD\n')
'dev_tools/check_incremental_coverage_annotations.py HEAD\n')
assert result.err == "Comparing against revision 'HEAD'.\n"

result = run(script_file='check/pytest-and-incremental-coverage',
tmpdir_factory=tmpdir_factory,
setup='touch master\n'
'git add -A\n'
'git commit -m test --quiet --no-gpg-sign\n')
'git commit -m test --quiet --no-gpg-sign\n',
additional_intercepts=['check/pytest'])
assert result.exit_code == 0
assert result.out == (
'INTERCEPTED check/pytest '
'. --actually-quiet --cov --cov-report=annotate '
'--cov-config=dev_tools/conf/.coveragerc --benchmark-skip\n'
'INTERCEPTED python '
'dev_tools/run_pytest_and_incremental_coverage.py master\n')
'dev_tools/check_incremental_coverage_annotations.py master\n')
assert result.err == "Comparing against revision 'master'.\n"

result = run(script_file='check/pytest-and-incremental-coverage',
Expand All @@ -349,11 +380,15 @@ def test_pytest_and_incremental_coverage_branch_selection(tmpdir_factory):
'touch master2\n'
'git add -A\n'
'git commit -q -m test2 --no-gpg-sign\n'
'git checkout -q alt\n')
'git checkout -q alt\n',
additional_intercepts=['check/pytest'])
assert result.exit_code == 0
assert result.out.startswith(
'INTERCEPTED check/pytest '
'. --actually-quiet --cov --cov-report=annotate '
'--cov-config=dev_tools/conf/.coveragerc --benchmark-skip\n'
'INTERCEPTED python '
'dev_tools/run_pytest_and_incremental_coverage.py ')
'dev_tools/check_incremental_coverage_annotations.py ')
assert result.err.startswith(
"Comparing against revision 'master' (merge base ")

Expand Down
34 changes: 0 additions & 34 deletions dev_tools/check_incremental_coverage.py
View file

This file was deleted.

Loading

0 comments on commit 0a5d3d4

Please sign in to comment.