Skip to content

Commit 38ce205

Browse files
pavoljuhasjselig-rigetti
authored andcommitted
Fix spurious failure of the check/all script (quantumlib#6611)
- 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
1 parent fc5f932 commit 38ce205

9 files changed

+18
-17
lines changed

check/all

+9-8
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,20 @@ cd "${topdir}" || exit $?
3838
errors=()
3939

4040
# Parse arguments.
41-
apply_arg=""
41+
apply_arg=( )
4242
only_changed=0
43-
rev=""
43+
rev=( )
4444
for arg in "$@"; do
4545
if [[ "${arg}" == "--only-changed-files" ]]; then
4646
only_changed=1
4747
elif [[ "${arg}" == "--apply-format-changes" ]]; then
48-
apply_arg="--apply"
49-
elif [ -z "${rev}" ]; then
48+
apply_arg=( "--apply" )
49+
elif [[ "${#rev[@]}" == 0 ]]; then
5050
if [ "$(git cat-file -t "${arg}" 2> /dev/null)" != "commit" ]; then
5151
echo -e "\033[31mNo revision '${arg}'.\033[0m" >&2
5252
exit 1
5353
fi
54-
rev="${arg}"
54+
rev=( "${arg}" )
5555
else
5656
echo -e "\033[31mInvalid arguments. Expected [BASE_REVISION] [--only-changed-files] [--apply-format].\033[0m" >&2
5757
exit 1
@@ -73,15 +73,16 @@ echo "Running mypy"
7373
check/mypy || errors+=( "check/mypy failed" )
7474

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

7879
if [ ${only_changed} -ne 0 ]; then
7980
echo "Running pytest and incremental coverage on changed files"
80-
check/pytest-changed-files-and-incremental-coverage "${rev}" ||
81+
check/pytest-changed-files-and-incremental-coverage "${rev[@]}" ||
8182
errors+=( "check/pytest-changed-files-and-incremental-coverage failed" )
8283
else
8384
echo "Running pytest and incremental coverage"
84-
check/pytest-and-incremental-coverage "${rev}" ||
85+
check/pytest-and-incremental-coverage "${rev[@]}" ||
8586
errors+=( "check/pytest-and-incremental-coverage failed" )
8687
fi
8788

check/build-changed-protos

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ cd "${topdir}" || exit $?
3232

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

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

6767
if [[ -n "$uncommitted" ]]; then
6868
echo -e "\033[31mERROR: Uncommitted generated files found! Please generate and commit these files using dev_tools/build-protos.sh:\033[0m"

check/format-incremental

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ for arg in "$@"; do
4646
elif [[ "${arg}" == "--all" ]]; then
4747
only_changed=0
4848
elif [ -z "${rev}" ]; then
49-
if [ "$(git cat-file -t "${arg}" 2> /dev/null)" != "commit" ]; then
49+
if ! git rev-parse --verify --quiet --no-revs "${arg}^{commit}"; then
5050
echo -e "\033[31mNo revision '${arg}'.\033[0m" >&2
5151
exit 1
5252
fi

check/pylint-changed-files

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ cd "${topdir}" || exit $?
3131

3232
# Figure out which revision to compare against.
3333
if [ -n "$1" ] && [[ $1 != -* ]]; then
34-
if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then
34+
if ! git rev-parse --verify --quiet --no-revs "$1^{commit}"; then
3535
echo -e "\033[31mNo revision '$1'.\033[0m" >&2
3636
exit 1
3737
fi

check/pytest-and-incremental-coverage

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ done
4545

4646
# Figure out which revision to compare against.
4747
if [ -n "${BASEREV}" ]; then
48-
if [ "$(git cat-file -t "${BASEREV}" 2> /dev/null)" != "commit" ]; then
48+
if ! git rev-parse --verify --quiet --no-revs "${BASEREV}^{commit}"; then
4949
echo -e "\033[31mNo revision '${BASEREV}'.\033[0m" >&2
5050
exit 1
5151
fi

check/pytest-changed-files

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ cd "${topdir}" || exit $?
3535
# Figure out which branch to compare against.
3636
rest=( "$@" )
3737
if [ -n "$1" ] && [[ $1 != -* ]]; then
38-
if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then
38+
if ! git rev-parse --verify --quiet --no-revs "$1^{commit}"; then
3939
echo -e "\033[31mNo revision '$1'.\033[0m" >&2
4040
exit 1
4141
fi

check/pytest-changed-files-and-incremental-coverage

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ cd "$(git rev-parse --show-toplevel)" || exit 1
3434

3535
# Figure out which revision to compare against.
3636
if [ -n "$1" ] && [[ $1 != -* ]]; then
37-
if [ "$(git cat-file -t "$1" 2> /dev/null)" != "commit" ]; then
37+
if ! git rev-parse --verify --quiet --no-revs "$1^{commit}"; then
3838
echo -e "\033[31mNo revision '$1'.\033[0m" >&2
3939
exit 1
4040
fi

cirq-core/cirq/contrib/svg/svg_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# pylint: disable=wrong-or-nonexistent-copyright-notice
2-
import IPython.display # type: ignore
2+
import IPython.display
33
import numpy as np
44
import pytest
55

dev_tools/conf/mypy.ini

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ follow_imports = silent
1717
ignore_missing_imports = true
1818

1919
# Non-Google
20-
[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.*]
20+
[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.*]
2121
follow_imports = silent
2222
ignore_missing_imports = true
2323

0 commit comments

Comments
 (0)