Skip to content

Commit 360c63d

Browse files
Add Makefile + pre-commit + pinned black for local CI parity
Brings every PR-blocking GitHub Actions check under a single `make` invocation so contributors can reproduce CI locally before pushing. Why --- The Tests workflow installs `black` via `pip install black` (latest available), which silently drifted from contributors' environments and caused two force-pushes on #20 to chase formatting fixes the local `black` couldn't see. CI also has four other workflows (js-sync, e2e, notebooks, lighthouse) with no documented local equivalent. What ---- * `Makefile` with one target per workflow: - `lint` / `test-py` / `test-js` / `test-notebook-fmt` (fast) - `test-notebooks` / `test-e2e` / `test-lighthouse` (slow) - `check` = lint + fast tests; `check-all` = full CI parity. * `.pre-commit-config.yaml` runs the same black + flake8 checks that CI does, automatically on every commit. * `pyproject.toml`: black bumped from `>=23.0.0` to `==26.3.1` (the current CI version) and `pre-commit` added to `[dev]` extras. * `.github/workflows/tests.yml`: lint job now `pip install -e ".[dev]"` so CI uses the pinned version too — eliminates the drift class of failure. * `CONTRIBUTING.md`: one-time setup, daily-use, before-push, and a per-target table mapping each Make target to its CI workflow. Bumping black requires updating .pre-commit-config.yaml + pyproject.toml + running `make lint` in lockstep.
1 parent 1017d7e commit 360c63d

6 files changed

Lines changed: 254 additions & 3 deletions

File tree

.github/workflows/tests.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,10 @@ jobs:
7474
- name: Install linting dependencies
7575
run: |
7676
python -m pip install --upgrade pip
77-
pip install flake8 black
77+
# Install dev extras so `black` is pinned to the version in
78+
# pyproject.toml. Keep this in sync with .pre-commit-config.yaml
79+
# and the local `make lint` target.
80+
pip install -e ".[dev]"
7881
7982
- name: Check code formatting with black
8083
run: |

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ GWPvsStrength.png
7171
# Planning files
7272
*.plan.md
7373

74+
# Local `make test-notebooks` output (executed copies of notebooks).
75+
.notebook-runs/
76+
7477
# E2E / Playwright / Lighthouse CI
7578
node_modules/
7679
playwright-report/

.pre-commit-config.yaml

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Local pre-commit hooks. Mirrors the CI lint gate in
2+
# .github/workflows/tests.yml so that "passes locally" implies
3+
# "passes the lint job on CI".
4+
#
5+
# One-time setup:
6+
# pip install -e ".[dev]" # installs pre-commit + pinned black
7+
# pre-commit install # registers the git hook
8+
#
9+
# Manual run across all tracked files (matches the CI invocation):
10+
# pre-commit run --all-files
11+
repos:
12+
- repo: https://github.com/psf/black
13+
# Pinned to match the CI lint job. Keep this in sync with the
14+
# `black==X.Y.Z` constraint in pyproject.toml's [project.optional-dependencies].dev.
15+
rev: 26.3.1
16+
hooks:
17+
- id: black
18+
19+
- repo: https://github.com/pycqa/flake8
20+
rev: 7.1.1
21+
hooks:
22+
- id: flake8
23+
# CI's hard gate: fail only on syntax errors / undefined names.
24+
# Style warnings are reported but non-blocking on CI, and we keep
25+
# the same posture locally to avoid pre-commit becoming a nag.
26+
args:
27+
- --count
28+
- --select=E9,F63,F7,F82
29+
- --show-source
30+
- --statistics

CONTRIBUTING.md

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,66 @@ We actively welcome your pull requests.
1212
5. Make sure your code lints.
1313
6. If you haven't already, complete the Contributor License Agreement ("CLA").
1414

15+
## Running CI checks locally
16+
17+
The repo has five GitHub Actions workflows in `.github/workflows/` that
18+
gate every PR. A `Makefile` exposes one target per workflow so "passes
19+
locally" implies "passes on CI". Run `make help` to list them all.
20+
21+
### One-time setup
22+
23+
```bash
24+
pip install -e ".[dev]" # lint + Python unit tests + pre-commit
25+
pip install -e ".[notebooks]" # for notebook execution / validation
26+
npm ci # for JS sync, e2e, lighthouse
27+
npx playwright install --with-deps chromium # for e2e
28+
pre-commit install # registers the git pre-commit hook
29+
```
30+
31+
### Daily use
32+
33+
```bash
34+
make check # lint + fast tests (recommended pre-commit gate)
35+
make format # apply black formatting in-place
36+
```
37+
38+
`make check` runs (in seconds): black, flake8, pytest with the 100%
39+
coverage gate, the JS GP sync test, and notebook format validation.
40+
41+
### Before pushing — full CI parity
42+
43+
```bash
44+
make check-all # everything `make check` runs PLUS:
45+
# - execute every notebook (4-mode matrix)
46+
# - Playwright (desktop + mobile)
47+
# - Lighthouse CI
48+
```
49+
50+
`make check-all` mirrors every workflow in `.github/workflows/` and is
51+
the right command to run before a force-push. It takes several minutes.
52+
53+
### Individual targets
54+
55+
| Target | Mirrors workflow |
56+
|---------------------------|------------------------------------------------|
57+
| `make lint` | `tests.yml` :lint |
58+
| `make test-py` | `tests.yml` :test |
59+
| `make test-js` | `js-sync.yml` |
60+
| `make test-notebook-fmt` | `notebooks.yml` :notebook-lint |
61+
| `make test-notebooks` | `notebooks.yml` :mode-{dependent,independent} |
62+
| `make test-e2e` | `e2e.yml` |
63+
| `make test-lighthouse` | `lighthouse.yml` |
64+
65+
### Pre-commit hook
66+
67+
`.pre-commit-config.yaml` runs the same `black` and `flake8` checks
68+
that `make lint` does, automatically on every commit. The black version
69+
is pinned to match `pyproject.toml` and the CI workflow.
70+
71+
If you bump black, update all three in lockstep:
72+
`.pre-commit-config.yaml`, the `black==X.Y.Z` constraint in
73+
`pyproject.toml`, and re-run `make lint` to confirm.
74+
1575
## Contributor License Agreement ("CLA")
1676
In order to accept your pull request, we need you to submit a CLA. You only need
1777
to do this once to work on any of Facebook's open source projects.
@@ -28,4 +88,4 @@ outlined on that page and do not file a public issue.
2888

2989
## License
3090
By contributing to SustainableConcrete, you agree that your contributions will be licensed
31-
under the LICENSE file in the root directory of this source tree.
91+
under the LICENSE file in the root directory of this source tree.

Makefile

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
# Local CI-equivalent targets. Mirrors the suite of GitHub Actions
2+
# workflows in .github/workflows/ so that `make check-all` matches the
3+
# checks a PR will run on CI.
4+
#
5+
# make help - list targets
6+
#
7+
# === Fast (seconds) ===
8+
# make lint - black --check + flake8 (tests.yml :lint)
9+
# make format - apply black formatting in-place
10+
# make test-py - pytest with 100% coverage (tests.yml :test)
11+
# make test-js - JS GP sync test (js-sync.yml)
12+
# make test-notebook-fmt - nbformat validation (notebooks.yml :notebook-lint)
13+
#
14+
# === Slow (minutes) ===
15+
# make test-notebooks - execute every notebook (notebooks.yml :mode-*)
16+
# make test-e2e - Playwright desktop+mobile (e2e.yml)
17+
# make test-lighthouse - Lighthouse CI (lighthouse.yml)
18+
#
19+
# === Aggregates ===
20+
# make test - fast tests (py + js + notebook-fmt)
21+
# make check - lint + test (recommended pre-commit gate)
22+
# make check-all - lint + every test (full local CI parity)
23+
#
24+
# Most targets need extras installed:
25+
# pip install -e ".[dev]" # lint, test-py
26+
# pip install -e ".[notebooks]" # test-notebook-fmt, test-notebooks
27+
# npm ci # test-js, test-e2e, test-lighthouse
28+
# npx playwright install --with-deps chromium # test-e2e
29+
30+
PYTHON ?= python
31+
32+
.PHONY: help \
33+
lint format \
34+
test-py test-js test-notebook-fmt test-notebooks \
35+
test-e2e test-lighthouse \
36+
test check check-all
37+
38+
help:
39+
@echo "Local CI-equivalent targets (see Makefile header for full list):"
40+
@echo ""
41+
@echo " Fast:"
42+
@echo " make lint - black --check + flake8"
43+
@echo " make format - apply black formatting in-place"
44+
@echo " make test-py - pytest with 100% coverage gate"
45+
@echo " make test-js - JS GP sync test"
46+
@echo " make test-notebook-fmt - nbformat validation"
47+
@echo ""
48+
@echo " Slow:"
49+
@echo " make test-notebooks - execute every notebook"
50+
@echo " make test-e2e - Playwright (desktop + mobile)"
51+
@echo " make test-lighthouse - Lighthouse CI"
52+
@echo ""
53+
@echo " Aggregates:"
54+
@echo " make test - fast tests"
55+
@echo " make check - lint + test (recommended)"
56+
@echo " make check-all - lint + every test (full CI parity)"
57+
58+
# Tracked Python files. CI's `black --check --diff .` only sees files
59+
# in the checked-out commit, so locally we must scope to tracked files
60+
# too — otherwise an untracked work-in-progress script in the working
61+
# tree would block `make lint` even though it can't fail the PR. Falls
62+
# back from `sl files` (Sapling) to `git ls-files` (vanilla git).
63+
TRACKED_PY := $(shell (sl files 2>/dev/null || git ls-files) | grep -E '\.py$$')
64+
65+
# --- Lint -----------------------------------------------------------
66+
# Mirrors .github/workflows/tests.yml :lint, scoped to TRACKED_PY so
67+
# untracked working-tree files don't poison the result. Black version
68+
# is pinned via pyproject.toml's [project.optional-dependencies].dev
69+
# so the formatter output is bit-for-bit identical to CI.
70+
lint:
71+
$(PYTHON) -m black --check --diff $(TRACKED_PY)
72+
$(PYTHON) -m flake8 $(TRACKED_PY) --count --select=E9,F63,F7,F82 --show-source --statistics
73+
$(PYTHON) -m flake8 $(TRACKED_PY) --count --exit-zero --statistics
74+
75+
format:
76+
$(PYTHON) -m black $(TRACKED_PY)
77+
78+
# --- Python unit tests ---------------------------------------------
79+
# Mirrors .github/workflows/tests.yml :test. We drop --cov-report=xml
80+
# because we don't need the coverage.xml artefact locally.
81+
test-py:
82+
$(PYTHON) -m pytest test/ -v --tb=short --cov=boxcrete --cov-report=term-missing --cov-fail-under=100
83+
84+
# --- JS GP sync test ------------------------------------------------
85+
# Mirrors .github/workflows/js-sync.yml. Verifies docs/gp.mjs predicts
86+
# the same values as the Python reference for the committed model.
87+
test-js:
88+
node test/test_js_gp.mjs
89+
90+
# --- Notebook format validation ------------------------------------
91+
# Mirrors .github/workflows/notebooks.yml :notebook-lint. Just validates
92+
# the JSON; does not execute the notebooks.
93+
test-notebook-fmt:
94+
$(PYTHON) -c "import nbformat, pathlib; \
95+
nbs = sorted(pathlib.Path('notebooks').glob('*.ipynb')); \
96+
[(nbformat.read(open(p), as_version=4), print(f'OK {p}')) for p in nbs]; \
97+
print(f'Validated {len(nbs)} notebook(s).')"
98+
99+
# --- Notebook execution --------------------------------------------
100+
# Mirrors .github/workflows/notebooks.yml :mode-{dependent,independent}.
101+
# Slow: each notebook runs nbconvert with a 600s timeout.
102+
# Mode-dependent notebooks are executed once per (mode, include-cost)
103+
# combination, matching the CI matrix.
104+
#
105+
# Unlike CI (which writes --inplace and uploads the executed notebook as
106+
# an artifact), we redirect output to a temp dir so the local working
107+
# copy isn't dirtied by `make` runs.
108+
NOTEBOOK_OUT := $(CURDIR)/.notebook-runs
109+
NBCONVERT := $(PYTHON) -m jupyter nbconvert --to notebook --execute \
110+
--output-dir=$(NOTEBOOK_OUT) \
111+
--ExecutePreprocessor.timeout=600 \
112+
--ExecutePreprocessor.kernel_name=python3
113+
test-notebooks:
114+
@mkdir -p $(NOTEBOOK_OUT)
115+
BOXCRETE_OPTIMIZATION_MODE=concrete BOXCRETE_INCLUDE_COST=false \
116+
$(NBCONVERT) --output=mode_concrete_cost_false.ipynb \
117+
notebooks/prediction_and_optimization_tutorial.ipynb
118+
BOXCRETE_OPTIMIZATION_MODE=concrete BOXCRETE_INCLUDE_COST=true \
119+
$(NBCONVERT) --output=mode_concrete_cost_true.ipynb \
120+
notebooks/prediction_and_optimization_tutorial.ipynb
121+
BOXCRETE_OPTIMIZATION_MODE=mortar BOXCRETE_INCLUDE_COST=false \
122+
$(NBCONVERT) --output=mode_mortar_cost_false.ipynb \
123+
notebooks/prediction_and_optimization_tutorial.ipynb
124+
BOXCRETE_OPTIMIZATION_MODE=mortar BOXCRETE_INCLUDE_COST=true \
125+
$(NBCONVERT) --output=mode_mortar_cost_true.ipynb \
126+
notebooks/prediction_and_optimization_tutorial.ipynb
127+
@for nb in notebooks/*.ipynb; do \
128+
case "$$nb" in \
129+
notebooks/prediction_and_optimization_tutorial.ipynb) ;; \
130+
*) echo "=== Executing $$nb ==="; \
131+
$(NBCONVERT) "$$nb" ;; \
132+
esac \
133+
done
134+
135+
# --- E2E (Playwright) ----------------------------------------------
136+
# Mirrors .github/workflows/e2e.yml. Requires `npm ci` + a one-time
137+
# `npx playwright install --with-deps chromium` to set up browsers.
138+
test-e2e:
139+
npx playwright test --project=desktop
140+
npx playwright test --project=mobile
141+
142+
# --- Lighthouse CI -------------------------------------------------
143+
# Mirrors .github/workflows/lighthouse.yml.
144+
test-lighthouse:
145+
npx lhci autorun
146+
147+
# --- Aggregates ----------------------------------------------------
148+
test: test-py test-js test-notebook-fmt
149+
check: lint test
150+
check-all: lint test-py test-js test-notebook-fmt test-notebooks test-e2e test-lighthouse

pyproject.toml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,13 @@ dependencies = [
3030

3131
[project.optional-dependencies]
3232
dev = [
33-
"black>=23.0.0",
33+
# black is pinned (not >=) to keep formatter output bit-for-bit
34+
# identical between contributors' machines and the CI lint job.
35+
# Bumping requires updating .pre-commit-config.yaml and the
36+
# `pip install` line in .github/workflows/tests.yml in lockstep.
37+
"black==26.3.1",
3438
"flake8>=6.0.0",
39+
"pre-commit>=3.0.0",
3540
"pytest>=7.0.0",
3641
"pytest-cov>=4.0.0",
3742
"parameterized>=0.9.0",

0 commit comments

Comments
 (0)