diff --git a/.github/PULL_REQUEST_TEMPLATE/default.md b/.github/PULL_REQUEST_TEMPLATE/default.md index 5d0ef05..d1af3e3 100644 --- a/.github/PULL_REQUEST_TEMPLATE/default.md +++ b/.github/PULL_REQUEST_TEMPLATE/default.md @@ -36,6 +36,7 @@ Closes # - [ ] Changes have been tested locally - [ ] This PR is single-purpose and does not mix dependency/security updates with unrelated formatting or content cleanups - [ ] All review threads are resolved or have an explicit won't-fix / follow-up disposition before merge +- [ ] Every accepted review finding that identifies a reproducible bug or edge case either lands a regression test in this PR **or** has a linked follow-up issue referenced in the thread resolution ## Additional Notes diff --git a/.github/workflows/pr-hygiene-check.yml b/.github/workflows/pr-hygiene-check.yml new file mode 100644 index 0000000..a0ccbb5 --- /dev/null +++ b/.github/workflows/pr-hygiene-check.yml @@ -0,0 +1,90 @@ +name: PR Hygiene Check + +# Flags PRs that mix dependency/security changes with unrelated app or content +# changes — which the project checklist explicitly discourages. +# +# Runs in WARNING mode: the job always exits 0 so it never blocks merge. +# Switch `ENFORCE=true` to make it a hard failure once the pattern is stable. + +on: + pull_request: + branches: ["main"] + types: [opened, synchronize, reopened, labeled, unlabeled] + +env: + # Set to true to make the check block merges instead of warn + ENFORCE: "false" + +jobs: + hygiene: + name: PR hygiene — dependency/security separation + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Check for mixed dependency/security + unrelated changes + env: + PR_TITLE: ${{ github.event.pull_request.title }} + PR_BODY: ${{ github.event.pull_request.body }} + PR_LABELS: ${{ toJson(github.event.pull_request.labels.*.name) }} + run: | + set -euo pipefail + + # Skip if the override label is present + if echo "$PR_LABELS" | grep -qi "mixed-changes-ok"; then + echo "ℹ️ Label 'mixed-changes-ok' present — skipping hygiene check." + exit 0 + fi + + BASE="${{ github.event.pull_request.base.sha }}" + HEAD="${{ github.event.pull_request.head.sha }}" + + CHANGED=$(git diff --name-only "$BASE"..."$HEAD" 2>/dev/null || \ + git diff --name-only "$BASE" "$HEAD") + + # Count package files — use grep with || true so pipefail does not + # exit when grep finds zero matches (exit 1 = no match, not an error + # for our purposes). + DEP_FILES=$(echo "$CHANGED" | { grep -cE '^package(-lock)?\.json$' || true; }) + + # Detect security/dependency signals in title only (not body) to + # avoid false-positives from maintenance PRs that mention CVEs in + # context rather than as the fix target. + SEC_SIGNAL=0 + if echo "$PR_TITLE" | grep -qiE 'cve-[0-9]|dependabot|security fix|bump .+ to [0-9]'; then + SEC_SIGNAL=1 + fi + + IS_DEP_OR_SEC=$(( DEP_FILES > 0 || SEC_SIGNAL > 0 )) + + if [ "$IS_DEP_OR_SEC" -eq 0 ]; then + echo "✅ Not a dependency/security PR — no separation check needed." + exit 0 + fi + + # Count non-package files (app/content/docs/scripts) + UNRELATED=$(echo "$CHANGED" | { grep -cvE '^package(-lock)?\.json$' || true; }) + + if [ "$UNRELATED" -gt 0 ]; then + echo "⚠️ Mixed changes detected in a dependency/security PR:" + echo " Dependency/security files: package*.json or CVE/Dependabot signal in title" + echo " Other files changed ($UNRELATED):" + echo "$CHANGED" | grep -vE '^package(-lock)?\.json$' | sed 's/^/ /' || true + echo "" + echo " Project guidelines ask for single-purpose dependency/security PRs." + echo " To suppress this warning, add the 'mixed-changes-ok' label." + echo "" + + if [ "$ENFORCE" = "true" ]; then + exit 1 + else + echo " (Running in warning mode — this check does not block merge.)" + exit 0 + fi + else + echo "✅ Dependency/security PR contains only package files — looks clean." + fi diff --git a/.github/workflows/weekly-tests.yml b/.github/workflows/weekly-tests.yml index 74bb295..ea01475 100644 --- a/.github/workflows/weekly-tests.yml +++ b/.github/workflows/weekly-tests.yml @@ -20,7 +20,7 @@ jobs: node-version: "20" cache: "npm" - - name: Install dependencies + - name: Install Node dependencies run: npm ci - name: Run tests @@ -29,15 +29,63 @@ jobs: - name: Run linting run: npm run lint + # ----------------------------------------------------------------------- + # Maintenance guardrails added in PRs #73/#76 — exercised weekly so + # toolchain drift and content-rules parity issues surface on a schedule + # rather than only when the relevant paths change. + # ----------------------------------------------------------------------- + + - name: Check content-rules trigger parity + run: python3 scripts/check-content-rules-trigger-parity.py + + - name: Validate content rules + run: ./scripts/validate-content-rules.sh + + - name: Run deploy-script regression tests + run: bash tests/deploy-script-regression.sh + + - name: Run news-bullet and stale-search regression tests + run: npm test -- --runInBand tests/news-bullet-regression.test.js tests/command-palette-stale-search.test.js + + - name: Check markdown quality for teaching content + run: | + # Only run if teaching content files exist; markdownlint errors are + # real failures — do not swallow them with || true. + if compgen -G "_teaching/**/*.md" > /dev/null 2>&1 || \ + [ -f "assets/images/teaching/README.md" ]; then + npx markdownlint-cli2 --config .markdownlint-cli2.jsonc \ + "_teaching/**/*.md" \ + "assets/images/teaching/README.md" + else + echo "ℹ️ No teaching content files found — skipping markdownlint." + fi + + # ----------------------------------------------------------------------- + # Ruby / Jekyll toolchain check + # (install rbenv + pinned Ruby so check-ruby-toolchain.sh can verify) + # ----------------------------------------------------------------------- + + - name: Set up Ruby (pinned version) + uses: ruby/setup-ruby@v1 + with: + ruby-version-file: ".ruby-version" + bundler-cache: true + + - name: Check Ruby toolchain versions + run: bash scripts/check-ruby-toolchain.sh + + - name: Jekyll build (Ruby + Node combined) + run: bundle exec jekyll build --destination _site_weekly_check + - name: Create test summary if: always() run: | echo "## Weekly Test Results" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY - if [ ${{ job.status }} == 'success' ]; then - echo "✅ All tests passed successfully!" >> $GITHUB_STEP_SUMMARY + if [ "${{ job.status }}" = "success" ]; then + echo "✅ All checks passed successfully!" >> $GITHUB_STEP_SUMMARY else - echo "❌ Some tests failed. Please check the logs above." >> $GITHUB_STEP_SUMMARY + echo "❌ Some checks failed. Please review the logs above." >> $GITHUB_STEP_SUMMARY fi echo "" >> $GITHUB_STEP_SUMMARY echo "Test run completed on $(date)" >> $GITHUB_STEP_SUMMARY diff --git a/tests/utils.test.js b/tests/utils.test.js new file mode 100644 index 0000000..2b46b85 --- /dev/null +++ b/tests/utils.test.js @@ -0,0 +1,303 @@ +/** + * Tests for assets/js/utils.js shared utilities + * + * Covers: initScrollReveal, copyToClipboard (+ fallback), safeQuery, + * safeQueryAll, and the window.Utils export shape. + */ + +describe("utils.js", () => { + beforeEach(() => { + // Reset DOM and window state + document.body.innerHTML = ""; + delete window.Utils; + delete window.isMacPlatform; + delete window.updatePlatformSpecificElements; + delete window.copyEmail; + jest.resetModules(); + jest.clearAllMocks(); + }); + + // --------------------------------------------------------------------------- + // Module load / export shape + // --------------------------------------------------------------------------- + describe("window.Utils export", () => { + it("exports all expected utility functions", () => { + require("../assets/js/utils.js"); + const expected = [ + "isMacPlatform", + "updatePlatformSpecificElements", + "createModal", + "copyToClipboard", + "debounce", + "throttle", + "safeQuery", + "safeQueryAll", + "initAccessibleButton", + ]; + expected.forEach((fn) => { + expect(typeof window.Utils[fn]).toBe("function"); + }); + }); + + it("maintains backwards-compat aliases on window", () => { + require("../assets/js/utils.js"); + expect(typeof window.isMacPlatform).toBe("function"); + expect(typeof window.updatePlatformSpecificElements).toBe("function"); + expect(typeof window.copyEmail).toBe("function"); + }); + }); + + // --------------------------------------------------------------------------- + // initScrollReveal + // --------------------------------------------------------------------------- + describe("initScrollReveal()", () => { + it("reveals blocks immediately when IntersectionObserver is unavailable", () => { + // Remove IO from jsdom (it may be defined by the environment) + const original = global.IntersectionObserver; + delete global.IntersectionObserver; + + document.body.innerHTML = ` +
one
+
two
+ `; + + // Load the module — DOMContentLoaded fires synchronously in jsdom after + // require() in this context, but we can also invoke it via the event. + require("../assets/js/utils.js"); + document.dispatchEvent(new Event("DOMContentLoaded")); + + const blocks = document.querySelectorAll("[data-animate-block]"); + blocks.forEach((block) => { + expect(block.classList.contains("is-inview")).toBe(true); + }); + + if (original) global.IntersectionObserver = original; + }); + + it("sets up an observer when IntersectionObserver is available", () => { + const observeMock = jest.fn(); + const disconnectMock = jest.fn(); + const observerInstances = []; + + const unobserveMock = jest.fn(); + global.IntersectionObserver = jest.fn().mockImplementation((cb) => { + const instance = { + observe: observeMock, + unobserve: unobserveMock, + disconnect: disconnectMock, + _cb: cb, + }; + observerInstances.push(instance); + return instance; + }); + + // Set DOM before requiring so the DOMContentLoaded handler sees exactly 1 block + document.body.innerHTML = `
content
`; + + require("../assets/js/utils.js"); + document.dispatchEvent(new Event("DOMContentLoaded")); + + expect(global.IntersectionObserver).toHaveBeenCalled(); + + // The block we set up must have been passed to observe + const block = document.querySelector("[data-animate-block]"); + expect(observeMock).toHaveBeenCalledWith(block); + + // Simulate intersection — block should get .is-inview + observerInstances[observerInstances.length - 1]._cb([ + { isIntersecting: true, target: block }, + ]); + expect(block.classList.contains("is-inview")).toBe(true); + }); + + it("is a no-op when there are no [data-animate-block] elements", () => { + const observeMock = jest.fn(); + global.IntersectionObserver = jest + .fn() + .mockImplementation(() => ({ observe: observeMock })); + + document.body.innerHTML = "

no blocks

"; + require("../assets/js/utils.js"); + document.dispatchEvent(new Event("DOMContentLoaded")); + + expect(global.IntersectionObserver).not.toHaveBeenCalled(); + expect(observeMock).not.toHaveBeenCalled(); + }); + }); + + // --------------------------------------------------------------------------- + // copyToClipboard / fallback + // --------------------------------------------------------------------------- + describe("copyToClipboard()", () => { + let button; + + beforeEach(() => { + button = document.createElement("button"); + button.setAttribute("data-text", "hello world"); + const icon = document.createElement("i"); + icon.classList.add("fa-copy"); + button.appendChild(icon); + document.body.appendChild(button); + require("../assets/js/utils.js"); + }); + + it("uses navigator.clipboard when available", async () => { + const writeText = jest.fn().mockResolvedValue(undefined); + Object.defineProperty(navigator, "clipboard", { + value: { writeText }, + configurable: true, + writable: true, + }); + + window.Utils.copyToClipboard(button); + await Promise.resolve(); // flush microtasks + + expect(writeText).toHaveBeenCalledWith("hello world"); + }); + + it("falls back to execCommand when navigator.clipboard is unavailable", () => { + Object.defineProperty(navigator, "clipboard", { + value: undefined, + configurable: true, + writable: true, + }); + + // jsdom may not define execCommand; define it so we can spy on it + if (!document.execCommand) { + document.execCommand = () => false; + } + const execCommand = jest + .spyOn(document, "execCommand") + .mockReturnValue(true); + + window.Utils.copyToClipboard(button); + + expect(execCommand).toHaveBeenCalledWith("copy"); + execCommand.mockRestore(); + }); + + it("falls back to execCommand when clipboard.writeText rejects", async () => { + const writeText = jest.fn().mockRejectedValue(new Error("denied")); + Object.defineProperty(navigator, "clipboard", { + value: { writeText }, + configurable: true, + writable: true, + }); + + if (!document.execCommand) { + document.execCommand = () => false; + } + const execCommand = jest + .spyOn(document, "execCommand") + .mockReturnValue(true); + + window.Utils.copyToClipboard(button); + await Promise.resolve(); // let writeText reject + await Promise.resolve(); // let catch handler run + + expect(execCommand).toHaveBeenCalledWith("copy"); + execCommand.mockRestore(); + }); + + it("does nothing if no text is available", () => { + const noTextBtn = document.createElement("button"); + document.body.appendChild(noTextBtn); + // Should not throw + expect(() => window.Utils.copyToClipboard(noTextBtn)).not.toThrow(); + }); + + it("reads text from data-clipboard-text attribute", () => { + const altBtn = document.createElement("button"); + altBtn.setAttribute("data-clipboard-text", "alt text"); + document.body.appendChild(altBtn); + + const writeText = jest.fn().mockResolvedValue(undefined); + Object.defineProperty(navigator, "clipboard", { + value: { writeText }, + configurable: true, + writable: true, + }); + + window.Utils.copyToClipboard(altBtn); + expect(writeText).toHaveBeenCalledWith("alt text"); + }); + + it("uses the explicit text argument when provided", () => { + const writeText = jest.fn().mockResolvedValue(undefined); + Object.defineProperty(navigator, "clipboard", { + value: { writeText }, + configurable: true, + writable: true, + }); + + window.Utils.copyToClipboard(button, "explicit text"); + expect(writeText).toHaveBeenCalledWith("explicit text"); + }); + }); + + // --------------------------------------------------------------------------- + // safeQuery / safeQueryAll + // --------------------------------------------------------------------------- + describe("safeQuery()", () => { + beforeEach(() => { + require("../assets/js/utils.js"); + }); + + it("returns the matching element for a valid selector", () => { + document.body.innerHTML = `
hi
`; + const el = window.Utils.safeQuery(".target"); + expect(el).not.toBeNull(); + expect(el.textContent).toBe("hi"); + }); + + it("returns null for a valid selector with no match", () => { + document.body.innerHTML = ""; + expect(window.Utils.safeQuery(".no-such-element")).toBeNull(); + }); + + it("returns null and emits a warning for an invalid selector", () => { + const result = window.Utils.safeQuery(":::invalid:::"); + expect(result).toBeNull(); + expect(console.warn).toHaveBeenCalled(); + }); + + it("searches within a provided context element", () => { + document.body.innerHTML = ` +
yes
+ no + `; + const ctx = document.getElementById("ctx"); + const el = window.Utils.safeQuery(".inner", ctx); + expect(el.textContent).toBe("yes"); + }); + }); + + describe("safeQueryAll()", () => { + beforeEach(() => { + require("../assets/js/utils.js"); + }); + + it("returns all matching elements for a valid selector", () => { + document.body.innerHTML = ` +
  • a
    • +
  • b
    • +
  • c
    • + `; + const results = window.Utils.safeQueryAll(".item"); + expect(results.length).toBe(3); + }); + + it("returns an empty NodeList for a valid selector with no matches", () => { + document.body.innerHTML = ""; + const results = window.Utils.safeQueryAll(".nothing"); + expect(results.length).toBe(0); + }); + + it("returns an empty array and emits a warning for an invalid selector", () => { + const results = window.Utils.safeQueryAll(":::bad:::"); + expect(Array.isArray(results)).toBe(true); + expect(results.length).toBe(0); + expect(console.warn).toHaveBeenCalled(); + }); + }); +});