Skip to content

ci: add cargo fmt/clippy/test workflow with test-modification detector#3

Merged
RagingRedRiot merged 2 commits into
mainfrom
ci/add-pr-workflow
May 25, 2026
Merged

ci: add cargo fmt/clippy/test workflow with test-modification detector#3
RagingRedRiot merged 2 commits into
mainfrom
ci/add-pr-workflow

Conversation

@RagingRedRiot

@RagingRedRiot RagingRedRiot commented May 25, 2026

Copy link
Copy Markdown
Owner

Background

There's been no automated check on the repo until now — every PR has been validated by running cargo test locally. With the repo public and branch protection requiring PRs to main, the lack of a green-check signal on the PR view is the next gap worth closing.

What this adds

A single .github/workflows/ci.yml triggered on pull_request to main (and on push to main), with four jobs running in parallel:

Job What it runs Notes
fmt cargo fmt --all --check No service container, no cache — finishes in seconds
clippy cargo clippy --all-targets --locked -- -D warnings Lints lib + bin + tests; treats every warning as an error
test cargo test --locked against a Postgres 18 service container sqlx::test provisions per-test DBs and runs migrations automatically; PG18 is required because the schema uses the built-in uuidv7() function
detect-test-modifications Diffs tests/ against the PR base; surfaces edits to existing test code as a self-updating PR comment See next section

cargo jobs share a Swatinem/rust-cache cache; the fmt job skips caching since it doesn't compile.

Why the test-modification check is informational, not blocking

Adding new tests is always welcome and never triggers the detector: --diff-filter=MDR excludes pure additions, so a new tests/rooms.rs or a new #[sqlx::test] appended to an existing file produces no notice.

The detector exists for the quiet failure mode: a PR that removes or weakens assertions inside an existing test, or that edits a helper under tests/common/ that other tests transitively depend on. Both are sometimes legitimate (a test needs to evolve with the behavior it's measuring) but they should never slip past review unnoticed.

The detector posts a PR comment listing the affected files and line counts, keyed by an HTML marker so subsequent pushes update the same comment in place. If a follow-up push rolls the test changes back, the comment is deleted. The job itself always exits 0 — it's a reviewer aid, not a gate.

Test plan

  • cargo fmt --all --check clean against current main
  • cargo clippy --all-targets --locked -- -D warnings clean locally
  • This PR exercises the workflow against itself (workflow lives on the head branch, so all four jobs run on this PR)
  • fmt, clippy, and test jobs all green on this PR
  • detect-test-modifications reports ✓ (this PR touches no files under tests/)
  • Smoke-test the detector after merge by opening a throwaway PR that removes a line from an existing test, confirming the comment shows up; then pushing a follow-up that restores the line and confirming the comment is deleted (test: smoke-test the test-modification detector (do not merge) #4)

Adds a PR-triggered GitHub Actions workflow with four jobs running in parallel:

- fmt:    cargo fmt --all --check
- clippy: cargo clippy --all-targets --locked -- -D warnings
- test:   cargo test --locked against a Postgres 18 service container (sqlx::test provisions per-test DBs and runs migrations)
- detect-test-modifications: diffs tests/ against the PR base. Files with removed/changed lines (or test helpers under tests/common/) are surfaced via a self-updating PR comment and a job summary. New tests are excluded by --diff-filter=MDR. Informational only -- never fails the check.

The test-modification check exists because adding tests is welcome but silent edits to existing test code can mask regressions; the PR comment makes those edits impossible for a reviewer to miss.
The test harness loads Config from env on every test (tests/common/mod.rs) and both admin_username and admin_credential are required fields with no serde defaults. Without them, every sqlx::test panics with `invalid config: MissingValue("admin_username")` before reaching the database. The credential value is ephemeral CI-only (each sqlx::test gets its own database).
@RagingRedRiot RagingRedRiot merged commit 0be925d into main May 25, 2026
4 checks passed
@RagingRedRiot RagingRedRiot deleted the ci/add-pr-workflow branch May 25, 2026 23:42
RagingRedRiot added a commit that referenced this pull request May 25, 2026
This reverts commit e67594a.
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