Skip to content

CI quality uplift: ESLint + Vitest + coverage gating + small extras (#161) - #162

Merged
rpgmem merged 5 commits into
mainfrom
claude/ci-quality-uplift
May 12, 2026
Merged

CI quality uplift: ESLint + Vitest + coverage gating + small extras (#161)#162
rpgmem merged 5 commits into
mainfrom
claude/ci-quality-uplift

Conversation

@rpgmem

@rpgmem rpgmem commented May 12, 2026

Copy link
Copy Markdown
Owner

Closes #161.

Summary

Four commits implementing the CI quality uplift from #161. Each is independently revertable.

Sprint Commit What
S1 5df4472 ESLint 9 flat config + lint:js script + new Lint workflow. Fixed 3 real bugs surfaced by the first pass.
S2 2727b57 Vitest + jsdom infrastructure + 23 unit tests covering validateDateTime, pickHideMode, analyzeDateTimeOrder. Test surfaced and fixed a real bug (JS analyzeDateTimeOrder wasn't defaulting time_mode to 'daily' like the PHP mirror does).
S3 e6da643 Coverage job loses continue-on-error: true and gains a floor enforcer that parses clover XML and writes a step-summary table. Floor set to 50% — under the baseline so day-one PRs aren't blocked, high enough to catch a regression.
S4-extras 38d487a composer validate --strict in the audit job; new Stylelint on assets/css job (was an unused npm script); new CodeQL workflow with javascript + php matrix on PRs + push + weekly cron.

What this adds in CI

After this PR lands, every PR to main will run:

  • PHPStan (PHP 8.1) — already gating
  • Composer audit + validate — new composer validate --strict step
  • PHPUnit (PHP 8.1-8.4 matrix) — already gating
  • Coverage (PHP 8.1) — now gating with a 50% line floor (was advisory)
  • WPCS on changed PHP files — already gating
  • Verify minified assets are up to date — already gating
  • Review dependency changes — already gating
  • ESLint on assets/js — new, gating
  • Vitest on tests/js — new, gating
  • Stylelint on assets/css — new, gating
  • CodeQL (javascript + php) — new, surfaces findings in the Security tab; not yet required for merge (operator can promote later)

Real bugs caught while implementing

  1. ffc-core.js:208== instead of === in field-dependency matching (both sides are always strings; no behaviour change, but tightening matters).
  2. ffc-dynamic-fragments.js:194 — duplicate var formId shadowing earlier declaration in the same function.
  3. ffc-admin.js:91 — redundant var strings redeclare (var hoists, so the second binding was a no-op).
  4. ffc-geofence-admin.js:83analyzeDateTimeOrder() didn't default time_mode to 'daily' when omitted, drifting from its PHP counterpart in Geofence::analyze_datetime_order(). The metabox form always provides the value, so invisible at runtime — caught by the new unit test.

⚠️ Manual setup required after merge: branch protection on main

This addresses the gap that left Dependabot patch/minor PRs sitting open instead of auto-merging (e.g. #157 last week). dependabot-automerge.yml calls gh pr merge --auto --squash, and --auto only activates when branch protection requires status checks. The MCP tools available to me don't include branch-protection config, so this needs to be set in the GitHub UI:

  1. Settings → Branches → Add branch protection rule for main.
  2. Enable "Require status checks to pass before merging" and "Require branches to be up to date".
  3. Add these as required checks:
    • PHPStan (PHP 8.1)
    • Composer audit
    • PHPUnit (PHP 8.1), PHPUnit (PHP 8.2), PHPUnit (PHP 8.3), PHPUnit (PHP 8.4)
    • Coverage (PHP 8.1)
    • WPCS on changed PHP files
    • Verify minified assets are up to date
    • Review dependency changes
    • ESLint on assets/js
    • Vitest on tests/js
    • Stylelint on assets/css
  4. Optional: also require conversation resolution and a pull request before merging.

Once that's in place, the next Dependabot patch/minor will auto-merge as soon as CI is green.

Test plan

  • npm run lint:js — 0 errors, 9 unused-vars warnings (non-gating).
  • npm run test:js — 23 / 23 OK.
  • npm run lint:css — 0 violations.
  • vendor/bin/phpunit — 3881 / 9839 OK (no test changes; runs same as on main).
  • vendor/bin/phpstan analyse — no errors.
  • Coverage floor enforcement — verified inline by reading the workflow; first PR-time run will print the actual percentage on the GitHub Actions step summary.

https://claude.ai/code/session_01HiExaniSqvNBLpVTszCLNx


Generated by Claude Code

claude added 5 commits May 12, 2026 02:19
First static-analysis pass on the 28 JS source files in `assets/js/`.
Previous state: 0 lint coverage. After this commit: 0 errors, 9
unused-vars warnings (non-gating, documented for future cleanup).

`eslint.config.mjs` — ESLint 9 flat config.
  - Permissive on style (legacy code mixes `var`/`const`, single/double
    quotes, indent widths; normalising is out of scope).
  - Strict on real bugs: `no-undef`, `no-unreachable`, dupe rules,
    `no-redeclare`, `eqeqeq` (smart mode so `== null` still works),
    `valid-typeof`, `use-isnan`, `no-self-compare`, etc.
  - Globals whitelist covers jQuery, browser, WP core (`wp`, `ajaxurl`)
    and all the `ffc*` localized payloads / `FFC*` window assigns the
    plugin defines via wp_localize_script.
  - Ignores `*.min.js`, sourcemaps, node_modules, vendor, build.

`package.json` — adds `eslint@^9.36.0` + `globals@^16.4.0` devDeps and
the `lint:js` / `lint:js:fix` scripts.

`.github/workflows/lint.yml` — new `Lint` workflow, single job
`ESLint on assets/js`. Gating from day one (clean baseline).

Three real bugs fixed by this pass:
  - `ffc-core.js:208` — `==` → `===` for a string/string comparison
    in field-dependency matching (both sides were already strings; no
    behaviour change).
  - `ffc-dynamic-fragments.js:194` — removed duplicate `var formId`
    declaration that shadowed the earlier one in the same function.
  - `ffc-admin.js:91` — removed redundant `var strings` (the earlier
    declaration in the same function hoists, so the second `var` was a
    redeclare; kept as plain re-assignment with a comment explaining
    why both paths need to assign).

Minified output rebuilt via `npm run build:js`.

https://claude.ai/code/session_01HiExaniSqvNBLpVTszCLNx
Adds Vitest + jsdom + jQuery stub infrastructure and 23 unit tests
covering the pure helpers shipped via #160:

  - FFCGeofence.validateDateTime (8 tests) — phase-tagged validity
    across daily/span modes, before/during/after states, custom-
    message propagation.
  - FFCGeofence.pickHideMode (5 tests) — phase → hideMode<Phase>
    mapping with the documented fallback to hideModeBefore for
    unknown phases and the 'message' default.
  - FFCGeofenceAdmin.analyzeDateTimeOrder (10 tests) — date inversion,
    span composed inversion, daily time inversion, equal-time edge
    case, partial configs, and the short-circuit-on-date-inversion
    behaviour that mirrors the PHP helper.

Infrastructure:
  - `vitest.config.mjs` — jsdom env, includes `tests/js/**/*.test.js`,
    setup file pre-installs a chainable jQuery stub.
  - `tests/js/setup.js` — minimal jQuery stub with `.ready(cb)` calling
    the callback synchronously (handles the `jQuery(document).ready(...)`
    pattern used in the admin script). All other chain methods are no-ops
    via a Proxy fallback.
  - `tests/js/helpers.js` — `loadScript(path)` reads the IIFE file and
    evaluates it via `new Function('jQuery', '$', code)` so `window.FFC*`
    globals land on jsdom's window without needing the script-tag/
    resource-loader path.
  - `tests/js/{geofence-frontend,geofence-admin}.test.js` — the suites.

Source adjustments to make the helpers testable:
  - `ffc-geofence-admin.js` exposes `analyzeDateTimeOrder` on
    `window.FFCGeofenceAdmin` at the end of the IIFE. Zero runtime
    impact — the IIFE wires its own listeners and never reads this
    object.
  - Same file: `analyzeDateTimeOrder()` now defaults `time_mode` to
    'daily' when omitted, matching the PHP server-side helper
    `Geofence::analyze_datetime_order()`. The metabox form always
    supplies a value, so this is invisible at runtime, but it
    correctly mirrors the server contract for any future direct
    caller. The new "defaults to daily" test caught this drift.

CI: `.github/workflows/lint.yml` gains a `jstest` job running
`npm run test:js`. Gating from day one (clean baseline).

Minified output rebuilt via `npm run build:js`.

https://claude.ai/code/session_01HiExaniSqvNBLpVTszCLNx
`ci.yml` coverage job loses `continue-on-error: true` and gains a
"Enforce line-coverage floor" step that parses the clover XML's
project-level `<metrics>` element and fails the job if
`coveredstatements / statements` falls below `COVERAGE_FLOOR_LINES`.

The floor is set to 50% — comfortably below the current baseline so
day-one PRs don't surprise their authors, but high enough to catch a
genuine regression. The intent is to ratchet upward whenever a PR
moves the number, not to hold it at 50% forever.

The step also writes a small summary table to `$GITHUB_STEP_SUMMARY`
(lines pct / floor / raw statement counts), so reviewers see the
coverage delta at a glance on the PR check page without digging
through Coveralls.

The existing Coveralls upload step is preserved unchanged — Coveralls
remains the historical trend view; the floor enforcement is the gate.

This closes the "Promote to required once we have a baseline % to
enforce" TODO that has been on the workflow comment since the job
was first added.

https://claude.ai/code/session_01HiExaniSqvNBLpVTszCLNx
#161)

Three small extras that polish the CI surface:

1. `composer validate --strict --no-check-publish` runs before the
   existing `composer audit` step in the audit job. Catches the
   `composer.json` vs `composer.lock` drift that occasionally slips
   through when a contributor edits one without regenerating the
   other. `--no-check-publish` skips the package-publish-readiness
   checks (license format, etc.) that aren't relevant for a private
   WP plugin.

2. New `css` job in `lint.yml` runs the existing `npm run lint:css`
   script (stylelint with the WordPress preset). Was already wired
   in package.json but never enforced in CI; baseline is clean (0
   violations on the current CSS files), so it gates from day one.
   If a future bump exposes legacy issues, swap to
   `continue-on-error: true` and chip away.

3. New `codeql.yml` workflow. Matrix over `javascript` + `php`, runs
   on PRs to main, pushes to main, and a weekly cron (Mon 03:00 UTC)
   so newly-disclosed query packs surface on unchanged code. Uses
   the `security-extended` query suite — security-only, no
   maintainability noise. CodeQL's `analyze` step writes findings
   into the Security tab; high-severity ones can be promoted to
   required checks later via branch protection.

https://claude.ai/code/session_01HiExaniSqvNBLpVTszCLNx
CodeQL's first-party analyzer ships for javascript/typescript/python/
java/go/c#/c++/swift/kotlin/ruby only. PHP isn't in that list, so the
"Analyze (php)" job failed at init with "Did not recognize the
following languages: php".

The JS side of the matrix is the actually-useful half here (28 source
files in `assets/js/`), and the PHP side is already covered by
PHPStan + composer audit + WPCS in `ci.yml` / `phpcs.yml`. Dropping
the PHP matrix entry leaves CodeQL with the meaningful work and gets
the workflow back to green.

If CodeQL adds PHP later, the matrix can be extended.

https://claude.ai/code/session_01HiExaniSqvNBLpVTszCLNx
@rpgmem
rpgmem marked this pull request as ready for review May 12, 2026 02:45
@rpgmem
rpgmem merged commit de31043 into main May 12, 2026
16 checks passed
@rpgmem
rpgmem deleted the claude/ci-quality-uplift branch May 12, 2026 02:45
rpgmem pushed a commit that referenced this pull request May 13, 2026
Bumps:
- ffcertificate.php plugin header `Version` → 6.5.3
- FFC_VERSION constant → 6.5.3
- readme.txt Stable tag → 6.5.3

CHANGELOG: collapses the Unreleased block into a 6.5.3 (2026-05-13)
section and adds the items merged since 6.5.2 that the prior diff was
missing:

Changed:
  - thumbmarkjs 1.8.1 → 1.9.0 (this PR)
  - jQuery UI theme 1.14.1 → 1.14.2 (this PR)
  - Recruitment CSV import: CPF/RF normalisation at parse time
    (#172, shipped via #182).

Fixed:
  - Form-editor groups 7/8 toggle-off persistence (already on
    Unreleased; kept).
  - Form-editor public CSV CPF "No" reverting to "Audit" (already on
    Unreleased; kept).
  - Reregistration form: $.trim() TypeError under jQuery 4 (#185).
  - Reregistration form: [name^="fields["] selector rejected under
    jQuery 4, getFields() returned {} (#185).

Internal:
  - JS coverage uplift 7.37% → 72.84% across multiple sprints
    (#162 / #164 / #166 / #168-#171 / #174-#180 / #183-#186),
    floor ratcheted 3 → 70.
  - Coverage job timeout 15 → 20 min (#181).
  - The 9 pre-existing ESLint no-unused-vars warnings cleared (#188).
  - CLAUDE.md added documenting auto-merge convention + CI gates +
    test-infrastructure notes (#187).
rpgmem added a commit that referenced this pull request May 13, 2026
…1 → 1.14.2 (#189)

* chore(deps): bump thumbmarkjs 1.8.1 → 1.9.0 and jQuery UI theme 1.14.1 → 1.14.2

Two upstream patch bumps that keep the API/CSS surface this plugin
relies on:

thumbmarkjs 1.8.1 → 1.9.0:
- New vendored bundle: `libs/js/thumbmark-1.9.0.umd.js` (33 KB).
- Old bundle removed.
- `FFC_THUMBMARK_VERSION` bumped; `Frontend::enqueue_*` resolves the
  filename from the constant so the enqueue call updates automatically.
- Surface used by `assets/js/ffc-device-signals.js`
  (`window.ThumbmarkJS.setOption('logging', false)`,
  `stableStringify`, `getFingerprintData()`) verified present in the
  new bundle and exercised by the existing JS suites
  (device-signals-deep.test.js / device-signals-and-frontend.test.js)
  — both still pass.
- `DeviceSignalsLoggingOffTest::test_vendored_thumbmarkjs_present_at_pinned_path`
  path + redownload-URL hint updated to track the new bundle.

jQuery UI theme 1.14.1 → 1.14.2:
- `libs/css/jquery-ui-smoothness.css` replaced with the 1.14.2 release.
- The CSS payload is byte-identical between the two upstream releases;
  the diff is just the file-header comment moving to
  `v1.14.2 - 2026-01-28`. Visible change for users is the cache-bust
  version string `wp_enqueue_style` emits.
- `FFC_JQUERY_UI_VERSION` bumped to `'1.14.2'`.

Tests: 3893 PHP + 487 JS still green.

* release: 6.5.2 → 6.5.3, consolidate CHANGELOG for the maintenance cut

Bumps:
- ffcertificate.php plugin header `Version` → 6.5.3
- FFC_VERSION constant → 6.5.3
- readme.txt Stable tag → 6.5.3

CHANGELOG: collapses the Unreleased block into a 6.5.3 (2026-05-13)
section and adds the items merged since 6.5.2 that the prior diff was
missing:

Changed:
  - thumbmarkjs 1.8.1 → 1.9.0 (this PR)
  - jQuery UI theme 1.14.1 → 1.14.2 (this PR)
  - Recruitment CSV import: CPF/RF normalisation at parse time
    (#172, shipped via #182).

Fixed:
  - Form-editor groups 7/8 toggle-off persistence (already on
    Unreleased; kept).
  - Form-editor public CSV CPF "No" reverting to "Audit" (already on
    Unreleased; kept).
  - Reregistration form: $.trim() TypeError under jQuery 4 (#185).
  - Reregistration form: [name^="fields["] selector rejected under
    jQuery 4, getFields() returned {} (#185).

Internal:
  - JS coverage uplift 7.37% → 72.84% across multiple sprints
    (#162 / #164 / #166 / #168-#171 / #174-#180 / #183-#186),
    floor ratcheted 3 → 70.
  - Coverage job timeout 15 → 20 min (#181).
  - The 9 pre-existing ESLint no-unused-vars warnings cleared (#188).
  - CLAUDE.md added documenting auto-merge convention + CI gates +
    test-infrastructure notes (#187).

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

[CI] JS lint + JS tests + coverage gating + small extras

2 participants