Skip to content

Commit

Permalink
Fix spurious failure of the check/all script (#6611)
Browse files Browse the repository at this point in the history
- Avoid passing an empty string argument to `check/format-incremental`
  when invoked from `check/all`

- Improve validation of the git-revision argument in check scripts by
  allowing git tags that resolve to a commit

- Fix invalid use of `?` in egrep pattern

- Make check/mypy pass in the dev.env.txt virtual environment
  • Loading branch information
pavoljuhas authored May 23, 2024
1 parent ee4d702 commit e4b6ab2
Show file tree
Hide file tree
Showing 9 changed files with 18 additions and 17 deletions.
17 changes: 9 additions & 8 deletions check/all
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,20 @@ cd "${topdir}" || exit $?
errors=()

# Parse arguments.
apply_arg=""
apply_arg=( )
only_changed=0
rev=""
rev=( )
for arg in "$@"; do
if [[ "${arg}" == "--only-changed-files" ]]; then
only_changed=1
elif [[ "${arg}" == "--apply-format-changes" ]]; then
apply_arg="--apply"
elif [ -z "${rev}" ]; then
apply_arg=( "--apply" )
elif [[ "${#rev[@]}" == 0 ]]; then
if [ "$(git cat-file -t "${arg}" 2> /dev/null)" != "commit" ]; then
echo -e "\033[31mNo revision '${arg}'.\033[0m" >&2
exit 1
fi
rev="${arg}"
rev=( "${arg}" )
else
echo -e "\033[31mInvalid arguments. Expected [BASE_REVISION] [--only-changed-files] [--apply-format].\033[0m" >&2
exit 1
Expand All @@ -73,15 +73,16 @@ echo "Running mypy"
check/mypy || errors+=( "check/mypy failed" )

echo "Running incremental format"
check/format-incremental "${rev}" "${apply_arg}" || errors+=( "check/format-incremental failed" )
check/format-incremental "${rev[@]}" "${apply_arg[@]}" ||
errors+=( "check/format-incremental failed" )

if [ ${only_changed} -ne 0 ]; then
echo "Running pytest and incremental coverage on changed files"
check/pytest-changed-files-and-incremental-coverage "${rev}" ||
check/pytest-changed-files-and-incremental-coverage "${rev[@]}" ||
errors+=( "check/pytest-changed-files-and-incremental-coverage failed" )
else
echo "Running pytest and incremental coverage"
check/pytest-and-incremental-coverage "${rev}" ||
check/pytest-and-incremental-coverage "${rev[@]}" ||
errors+=( "check/pytest-and-incremental-coverage failed" )
fi

Expand Down
4 changes: 2 additions & 2 deletions check/build-changed-protos
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ cd "${topdir}" || exit $?

# Figure out which revision to compare against.
if [ -n "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then
if ! git rev-parse --verify --quiet --no-revs "$1^{commit}"; then
echo -e "\033[31mNo revision '$1'.\033[0m" >&2
exit 1
fi
Expand Down Expand Up @@ -62,7 +62,7 @@ dev_tools/build-protos.sh

# Filenames with spaces will be ugly (each part will be listed separately)
# but the error logic will still work.
uncommitted=$(git status --porcelain 2>/dev/null | grep -E "^?? cirq-google" | cut -d " " -f 3)
uncommitted=$(git status --porcelain 2>/dev/null | grep -E "^[?][?] cirq-google" | cut -d " " -f 3)

if [[ -n "$uncommitted" ]]; then
echo -e "\033[31mERROR: Uncommitted generated files found! Please generate and commit these files using dev_tools/build-protos.sh:\033[0m"
Expand Down
2 changes: 1 addition & 1 deletion check/format-incremental
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ for arg in "$@"; do
elif [[ "${arg}" == "--all" ]]; then
only_changed=0
elif [ -z "${rev}" ]; then
if [ "$(git cat-file -t "${arg}" 2> /dev/null)" != "commit" ]; then
if ! git rev-parse --verify --quiet --no-revs "${arg}^{commit}"; then
echo -e "\033[31mNo revision '${arg}'.\033[0m" >&2
exit 1
fi
Expand Down
2 changes: 1 addition & 1 deletion check/pylint-changed-files
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ cd "${topdir}" || exit $?

# Figure out which revision to compare against.
if [ -n "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then
if ! git rev-parse --verify --quiet --no-revs "$1^{commit}"; then
echo -e "\033[31mNo revision '$1'.\033[0m" >&2
exit 1
fi
Expand Down
2 changes: 1 addition & 1 deletion check/pytest-and-incremental-coverage
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ done

# Figure out which revision to compare against.
if [ -n "${BASEREV}" ]; then
if [ "$(git cat-file -t "${BASEREV}" 2> /dev/null)" != "commit" ]; then
if ! git rev-parse --verify --quiet --no-revs "${BASEREV}^{commit}"; then
echo -e "\033[31mNo revision '${BASEREV}'.\033[0m" >&2
exit 1
fi
Expand Down
2 changes: 1 addition & 1 deletion check/pytest-changed-files
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ cd "${topdir}" || exit $?
# Figure out which branch to compare against.
rest=( "$@" )
if [ -n "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then
if ! git rev-parse --verify --quiet --no-revs "$1^{commit}"; then
echo -e "\033[31mNo revision '$1'.\033[0m" >&2
exit 1
fi
Expand Down
2 changes: 1 addition & 1 deletion check/pytest-changed-files-and-incremental-coverage
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ cd "$(git rev-parse --show-toplevel)" || exit 1

# Figure out which revision to compare against.
if [ -n "$1" ] && [[ $1 != -* ]]; then
if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then
if ! git rev-parse --verify --quiet --no-revs "$1^{commit}"; then
echo -e "\033[31mNo revision '$1'.\033[0m" >&2
exit 1
fi
Expand Down
2 changes: 1 addition & 1 deletion cirq-core/cirq/contrib/svg/svg_test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# pylint: disable=wrong-or-nonexistent-copyright-notice
import IPython.display # type: ignore
import IPython.display
import numpy as np
import pytest

Expand Down
2 changes: 1 addition & 1 deletion dev_tools/conf/mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ follow_imports = silent
ignore_missing_imports = true

# Non-Google
[mypy-sympy.*,matplotlib.*,proto.*,pandas.*,scipy.*,freezegun.*,mpl_toolkits.*,networkx.*,ply.*,astroid.*,pytest.*,_pytest.*,pylint.*,setuptools.*,qiskit.*,quimb.*,pylatex.*,filelock.*,sortedcontainers.*,tqdm.*,ruamel.*,absl.*,tensorflow_docs.*,ipywidgets.*, cachetools.*]
[mypy-IPython.*,sympy.*,matplotlib.*,proto.*,pandas.*,scipy.*,freezegun.*,mpl_toolkits.*,networkx.*,ply.*,astroid.*,pytest.*,_pytest.*,pylint.*,setuptools.*,qiskit.*,quimb.*,pylatex.*,filelock.*,sortedcontainers.*,tqdm.*,ruamel.*,absl.*,tensorflow_docs.*,ipywidgets.*,cachetools.*]
follow_imports = silent
ignore_missing_imports = true

Expand Down

0 comments on commit e4b6ab2

Please sign in to comment.