Skip to content

Commit a915678

Browse files
Fix and simplify check-python dev script (#1934)
I revisited and simplified a few things in the `check-python` dev script, partially as follow-up of tiny-pilot/tinypilot-pro#1714: - Fix `ADDITIONAL_PY_SCRIPTS` (which was outdated) for a dynamic lookup mechanism, which ensures to always catch all scripts in `scripts/`. - Consolidate `check-for-init-python-files` into the `check-python` script. It seemed weird to me to have this script separately and everything else embedded. - Fix some bash quotes (`"`→`'`) This structure also makes it a bit easier to include the front panel sources here: I realized that the `check-python` script also doesn’t sufficiently account for the front panel. So for the upstream merge, I plan to add these changes ([see this Pro branch I used for testing](tiny-pilot/tinypilot-pro@master...check-python)): - 👉 In Pro’s CircleCI config, we have to install the front panel venv before running `check-python`. That actually triggers some missing ignore-directives (e.g., in `.yapfignore`), that also would show up when running locally. - 👉 `ruff` doesn’t cover the front panel, so we have to add it there. - ✅ `yapf` already covers the front panel. - ❌ `coverage` doesn’t include the front panel – we don’t have any tests in the front panel, but I also wasn’t able to figure out how to make it even look there… - ❌ `pylint` doesn’t include the front panel – unfortunately, [I wasn’t able to make that happen trivially](tiny-pilot/tinypilot-pro#1731), so we should look into this separately – there are some minor style violations there. I think the aforementioned problems would profit from a [revised project structure](tiny-pilot/tinypilot-pro#1721), where we properly share dev dependencies for both our Python services. The current nested structure doesn’t seem to lend itself as nicely for this. <a data-ca-tag href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1934"><img src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review on CodeApprove" /></a> --------- Co-authored-by: Jan Heuermann <jan@jotaen.net>
1 parent 8da71bc commit a915678

3 files changed

Lines changed: 35 additions & 49 deletions

File tree

dev-scripts/check-all

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ set -x
1414
set -u
1515

1616
./dev-scripts/check-bash
17-
./dev-scripts/check-for-init-files
1817
./dev-scripts/check-javascript
1918
./dev-scripts/check-privilege-guard
2019
./dev-scripts/check-python

dev-scripts/check-for-init-py-files

Lines changed: 0 additions & 27 deletions
This file was deleted.

dev-scripts/check-python

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,44 +11,58 @@ set -x
1111
# Exit on unset variable.
1212
set -u
1313

14-
# Location of app source files.
15-
SOURCE_DIR="app"
16-
17-
ADDITIONAL_PY_SCRIPTS=()
18-
ADDITIONAL_PY_SCRIPTS+=("scripts/render-template")
19-
ADDITIONAL_PY_SCRIPTS+=("scripts/update-service")
20-
21-
# Location of virtualenv.
22-
VIRTUALENV_DIR=venv
23-
24-
./dev-scripts/check-for-init-py-files
25-
2614
# Delete pyc files from previous builds.
2715
find . \
28-
-name "*.pyc" \
16+
-name '*.pyc' \
2917
-type f \
30-
-not -path "./${VIRTUALENV_DIR}/*" \
18+
-not -path './venv/*' \
3119
-delete
3220

3321
# Run unit tests and calculate code coverage.
3422
# Load module `app.log` to initialize our custom logger.
3523
coverage run \
36-
--source "$SOURCE_DIR" \
37-
--omit "*_test.py" \
24+
--source app/ \
25+
--omit '*_test.py' \
3826
--module \
39-
unittest discover --pattern "*_test.py" \
27+
unittest discover --pattern '*_test.py' \
4028
app.log
4129
coverage report
4230

31+
# Check for Python init files.
32+
INIT_FILES_OK=true
33+
while read -r directory; do
34+
if [[ ! -f "${directory}/__init__.py" ]]; then
35+
printf "Directory missing __init__.py file: %s\n" "${directory}" >&2
36+
INIT_FILES_OK=false
37+
fi
38+
done < <(
39+
find . \
40+
-type f \
41+
-name '*.py' \
42+
-not -path './venv/*' \
43+
-not -path './.git/*' \
44+
-exec dirname {} \; \
45+
| sort --unique
46+
)
47+
"${INIT_FILES_OK}" || exit 1
48+
4349
# Check that source has correct formatting.
4450
yapf --diff --recursive ./
4551

52+
# Gather names of all Python scripts in the scripts/ folder.
53+
PYTHON_SCRIPTS=()
54+
while read -r line; do
55+
PYTHON_SCRIPTS+=("${line}")
56+
done < <(grep --recursive --files-with-matches '^#!.*python' scripts/)
57+
readonly PYTHON_SCRIPTS
58+
4659
# Run static analysis for Python bugs/cruft.
4760
ruff check \
48-
"${SOURCE_DIR}/" \
49-
"${ADDITIONAL_PY_SCRIPTS[@]}"
61+
app/ \
62+
"${PYTHON_SCRIPTS[@]}"
5063

5164
# Check for other style violations.
52-
PYTHONPATH="${SOURCE_DIR}" \
65+
PYTHONPATH='app/' \
5366
pylint \
54-
"${SOURCE_DIR}" "${ADDITIONAL_PY_SCRIPTS[@]}"
67+
app/ \
68+
"${PYTHON_SCRIPTS[@]}"

0 commit comments

Comments
 (0)