Skip to content

ci: add PR gate workflow, structure/rules checker, and algorithm tests - #29

Merged
utk2103 merged 1 commit into
mainfrom
ci/pr-checks
Sep 28, 2026
Merged

utk2103 merged 1 commit into
mainfrom
ci/pr-checks

Conversation

@utk2103

@utk2103 utk2103 commented Sep 28, 2026

Copy link
Copy Markdown
Owner

What this adds

A GitHub Actions workflow that runs on every PR against main, so review starts from a green or red signal instead of a manual read-through.

.github/workflows/pr-check.yml — three steps: the gate, compileall scripts, then unittest discover tests.

.github/check_pr.py — diffs the PR against its base branch and enforces the parts of CONTRIBUTING.md that can be checked mechanically.

Hard failures:

  • New top-level entry outside the whitelist, or any move/rename/delete of a tracked file
  • A subdirectory or wrong extension inside scripts/, notebooks/, docs/, tests/
  • A third-party import that isn't in requirements.txt (with an alias map for cv2, PIL, bs4, yaml, sklearn, dotenv)
  • A script that moves or deletes files with no --dry-run flag
  • A notebook whose saved output contains a local path like /Users/...

Warnings only (non-blocking):

  • A script with real logic and no assert and no test naming it
  • More than 1 MB of saved notebook output

The two warning rules are deliberately not errors: four scripts already on main break them, and blocking beginners on pre-existing debt is the wrong trade. Flip them to errors once that backlog is clear.

Run it locally the same way CI does:

python .github/check_pr.py --base main
python .github/check_pr.py --selftest   # covers the parsing helpers

tests/test_algorithms.py — 12 unittest cases for the pure functions in the search and sort scripts: binarySearch (found / missing / empty), linearSearch (found / first match / missing), mergeSort (in place, duplicates and negatives, 0–2 elements), twoSum (pair indices, unsorted input, no pair). They load scripts by path because 01.TwoSum.py isn't a valid module name, and suppress stdout because two of the scripts print at import.

check.py, toh.py, multithreading.py and qr_code_generator.py are untested here — the first three call input() or sleep at import time, so they need refactoring into functions first. Separate PR.

Drive-by fix

requirements.txt was corrupted on main: the qrcode[pil] line had been appended as UTF-16LE into a UTF-8 file, putting a NUL byte after every character. pip install -r requirements.txt fails on it, and the gate can't read the file either. Stripped the NULs; the line now reads normally.

Verification

$ python -m unittest discover tests
Ran 35 tests in 0.003s
OK

$ python .github/check_pr.py --selftest
selftest ok

$ python .github/check_pr.py --base origin/main
4 changed file(s), 0 error(s), 0 warning(s).

The gate was also dry-run against every existing file in scripts/ and tests/: 0 errors, 4 warnings (the untested scripts named above).

🤖 Generated with Claude Code

Adds a GitHub Actions workflow that runs on every pull request against
main, so a review starts from a green or red signal instead of a manual
read-through.

.github/check_pr.py is the gate. It diffs the PR against the base branch
and enforces the parts of CONTRIBUTING.md that can be checked
mechanically:

- structure: no new top-level entries, no moves, renames or deletions,
  no subdirectories or stray extensions inside scripts/, notebooks/,
  docs/ and tests/
- third-party imports must be declared in requirements.txt
- scripts that move or delete files must offer --dry-run
- notebooks must not carry saved output containing local paths

Missing tests and oversized notebook output are warnings, not errors, so
the existing files that break those two rules do not block contributors
today. `--selftest` covers the helpers.

tests/test_algorithms.py adds 12 unittest cases for the pure functions in
the search and sort scripts. They are loaded by path because 01.TwoSum.py
is not an importable module name. The remaining scripts call input() or
sleep at import time and need a refactor before they can be tested.

Also strips the NUL bytes from the qrcode[pil] line in requirements.txt,
which was appended as UTF-16 into a UTF-8 file and made the whole file
unreadable to pip.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@utk2103 utk2103 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Value add imo

@utk2103
utk2103 merged commit efaa52c into main Sep 28, 2026
1 check passed
@utk2103
utk2103 deleted the ci/pr-checks branch September 28, 2026 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant