Skip to content

Commit

Permalink
Add “check” prefix to all testing/linting/code-style dev scripts (#1791)
Browse files Browse the repository at this point in the history
Related #1716.

This PR introduces a consistent `check-` prefix for all dev scripts that
are concerned with anything related to checking (i.e., testing, linting,
code-style).

- I consolidated `check-bash`+`build-bash` and
`lint-frontend`+`build-javascript` into one script respectively. To me,
that would make more sense than having too many fine-granular scripts.
We do it the same way in the [`check-python`
script](https://github.com/tiny-pilot/tinypilot/blob/master/dev-scripts/build-python)
already.
- I also renamed all CircleCI jobs to reflect the dev script names.
- I renamed `build` to `check-all`, and, while on it, also added the
missing invocations. The only script that I wouldn’t cover here is
`check-e2e`, because it’s noticeably resource-heavy than all other
scripts, and requires non-trivial prerequisites.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1791"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>

---------

Co-authored-by: Jan Heuermann <[email protected]>
  • Loading branch information
jotaen4tinypilot and jotaen authored Apr 26, 2024
1 parent 3323b23 commit 80c0420
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 76 deletions.
54 changes: 22 additions & 32 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,16 @@ jobs:
docker:
- image: koalaman/shellcheck-alpine:v0.9.0
steps:
- checkout
- run:
name: Install dependencies
command: apk add bash git openssh-client grep
- checkout
command: |
apk add bash git openssh-client grep
git clone --depth 1 --branch v1.10.0 https://github.com/bats-core/bats-core.git
cd bats-core
./install.sh /usr/local
- run:
name: Run static analysis on bash scripts
name: Run tests and static analysis on bash scripts
command: ./dev-scripts/check-bash
check_privilege_guard:
executor: ubuntu
Expand All @@ -55,7 +59,7 @@ jobs:
- run:
name: Check for unnecessary privilege escalation
command: ./dev-scripts/check-privilege-guard
lint_sql:
check_sql:
executor: python
steps:
- checkout
Expand All @@ -67,7 +71,7 @@ jobs:
command: |
. venv/bin/activate
pip install --requirement dev_requirements.txt
./dev-scripts/lint-sql
./dev-scripts/check-sql
check_style:
executor: node
steps:
Expand Down Expand Up @@ -95,7 +99,7 @@ jobs:
. venv/bin/activate
pip install --requirement requirements.txt
./dev-scripts/decode-edid
build_python:
check_python:
executor: python
steps:
- checkout
Expand All @@ -107,8 +111,8 @@ jobs:
command: |
. venv/bin/activate
pip install --requirement dev_requirements.txt
./dev-scripts/build-python
build_javascript:
./dev-scripts/check-python
check_javascript:
executor: node
steps:
- checkout
Expand All @@ -117,25 +121,12 @@ jobs:
command: npm install
- run:
name: Run build script
command: ./dev-scripts/build-javascript
build_bash:
executor: ubuntu
steps:
- checkout
- run:
name: Install dependencies
command: |
git clone --depth 1 --branch v1.10.0 https://github.com/bats-core/bats-core.git
cd bats-core
sudo ./install.sh /usr/local
- run:
name: Run build script
command: ./dev-scripts/build-bash
e2e:
command: ./dev-scripts/check-javascript
run_e2e_tests:
docker:
# To run tests against the dev server, Playwright needs a CircleCI image
# with Python, Node, and a browser. While the build_python and
# build_javascript steps use python-3.9.17 and node-18.16.1 respectively,
# with Python, Node, and a browser. While the check_python and
# check_javascript steps use python-3.9.17 and node-18.16.1 respectively,
# the CircleCI image with *both* python and node in the closest versions
# is python:3.9.17-browsers. It has python-3.9.17 and node-18.16.0.
#
Expand Down Expand Up @@ -186,7 +177,7 @@ jobs:
root: ./debian-pkg/releases
paths:
- "*.deb"
lint_debian_package:
check_debian_package:
executor: ubuntu
steps:
- checkout
Expand Down Expand Up @@ -288,15 +279,14 @@ workflows:
- check_whitespace
- check_bash
- check_privilege_guard
- lint_sql
- check_sql
- check_style
- decode_edid
- build_python
- build_javascript
- build_bash
- check_python
- check_javascript
- run_e2e_tests
- build_debian_package
- e2e
- lint_debian_package:
- check_debian_package:
requires:
- build_debian_package
- build_bundle:
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ python3 -m venv venv && \
To run TinyPilot's build scripts, run:

```bash
./dev-scripts/build
./dev-scripts/check-all
```

### Run end-to-end tests
Expand Down
17 changes: 0 additions & 17 deletions dev-scripts/build-bash

This file was deleted.

11 changes: 7 additions & 4 deletions dev-scripts/build → dev-scripts/check-all
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ set -x
# Exit on unset variable.
set -u

./dev-scripts/check-trailing-whitespace
./dev-scripts/check-trailing-newline
./dev-scripts/check-bash
./dev-scripts/build-python
./dev-scripts/build-javascript
./dev-scripts/check-for-init-files
./dev-scripts/check-javascript
./dev-scripts/check-privilege-guard
./dev-scripts/check-python
./dev-scripts/check-sql
./dev-scripts/check-style
./dev-scripts/check-trailing-whitespace
./dev-scripts/check-trailing-newline
20 changes: 14 additions & 6 deletions dev-scripts/check-bash
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
#!/bin/bash
#
# Run static analysis on bash scripts.

# Exit on first failing command.
# Run tests and static analysis on bash scripts.

# Exit on first failure.
set -e

# Echo commands before executing them, by default to stderr.
set -x

# Exit on unset variable.
set -u

# Run static analysis with shellcheck.
BASH_SCRIPTS=()

while read -r filepath; do
if head -n 1 "${filepath}" | grep --quiet \
--regexp '#!/bin/bash' \
Expand All @@ -20,7 +23,12 @@ while read -r filepath; do
BASH_SCRIPTS+=("${filepath}")
fi
done < <(git ls-files)

readonly BASH_SCRIPTS

shellcheck "${BASH_SCRIPTS[@]}"

# Run bats tests.
bats \
--recursive \
scripts/ \
debian-pkg/ \
dev-scripts/
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ set -u
# Location of app source files.
SOURCE_DIR="app"

./dev-scripts/lint-frontend
# Check for JavaScript anti-patterns.
./node_modules/.bin/eslint "./**/*.js" "./**/*.html"

# Run unit tests.
./node_modules/.bin/mocha \
Expand Down
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion dev-scripts/git-hooks/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ set -x
# Exit on unset variable.
set -u

./dev-scripts/build
./dev-scripts/check-all
14 changes: 0 additions & 14 deletions dev-scripts/lint-frontend

This file was deleted.

0 comments on commit 80c0420

Please sign in to comment.