Skip to content

chore: harmonize CI workflow, templates, and test structure - #21

Merged
TheWitness merged 8 commits into
mainfrom
chore/ci-and-template-harmonization
Sep 21, 2026
Merged

TheWitness merged 8 commits into
mainfrom
chore/ci-and-template-harmonization

Conversation

@TheWitness

Copy link
Copy Markdown
Member

Description

Part of the fleet-wide plugin_* CI/template/test-structure harmonization
effort (see plugin_analytics#8, plugin_apcupsd#29).

Changes

  • Actions pinning: actions/checkout, shivammathur/setup-php,
    actions/upload-artifact now pinned to current release commit SHAs.
  • MySQL password logging fix: cat ~/.my.cnf printed the root password
    into the Actions log; replaced with chmod 600.
  • MySQL bootstrap hardening: quoted --defaults-file, grants added for
    both 'cactiuser'@'localhost' and 'cactiuser'@'127.0.0.1'.
  • Pest/Composer ownership: added a "Restore vendor ownership for Pest"
    step after Composer install, matching the pattern in plugin_apcupsd.
  • PHP syntax check: switched to the vendor-excluding, null-delimited
    find/xargs pattern from plugin_audit.
  • Removed the "Configure Apache" step: apache2/libapache2-mod-php were
    never installed in this workflow, so the step was dead weight.
  • Pest invocation simplified: relies on phpunit.xml's own
    <testsuites> block instead of duplicating the test directory list.
  • PHP compatibility test consolidated: old PHP-7.4-floor compatibility
    test replaced with tests/Security/PhpCompatibilityTest.php, checking for
    PHP 8.3/8.4-only syntax (this plugin's floor is PHP 8.2 per the CI matrix).
  • Removed dead tests/TestCase.php: not referenced by any
    uses(TestCase::class) call in this plugin's actual tests.
  • Issue/PR templates added, styled after Cacti/cacti's own templates.
  • copilot-instructions.md: documented the CI/dependency baselines.

Related Issue

N/A — internal fleet harmonization, not tracked against a specific issue.

How Has This Been Tested?

  • Workflow YAML was parsed and validated locally before pushing.
  • Full CI will run automatically on this PR.

Types of changes

  • Non-breaking maintenance/CI change

Checklist

  • My change follows the fleet-wide harmonization conventions being rolled
    out across plugin_* repos.

Part of the fleet-wide plugin_* harmonization effort.

- Pin actions/checkout, shivammathur/setup-php, actions/upload-artifact to
  current release commit SHAs (was floating @v4/@v2).
- Stop logging the MySQL root password in CI output (cat ~/.my.cnf ->
  chmod 600).
- Harden MySQL bootstrap: quoted --defaults-file, grants added for both
  'cactiuser'@'localhost' and 'cactiuser'@'127.0.0.1'.
- Add "Restore vendor ownership for Pest" step after Composer install.
- Switch the PHP syntax-check step to the vendor-excluding, null-delimited
  find/xargs pattern used in plugin_audit.
- Remove the unused "Configure Apache" step (apache2/libapache2-mod-php are
  not installed and not needed).
- Simplify the Pest invocation to rely solely on phpunit.xml's <testsuites>.
- No CodeQL workflow added: this repo has no JavaScript/Python/Ruby content
  (PHP only), so there is nothing for CodeQL to scan.
- Consolidate tests/Security/Php74CompatibilityTest.php into
  tests/Security/PhpCompatibilityTest.php, now checking for PHP 8.3/8.4-only
  syntax (this plugin's floor is PHP 8.2 per the CI matrix).
- Remove tests/TestCase.php and its require in tests/bootstrap-unit.php: not
  referenced by any uses(TestCase::class) call in this plugin's tests.
- Add .github/ISSUE_TEMPLATE/{bug_report,feature_request}.md and
  .github/PULL_REQUEST_TEMPLATE.md, styled after Cacti/cacti's own templates.
- Document CI/dependency baselines in copilot-instructions.md.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The new PhpCompatibilityTest.php needs the standard GPL header/clearer failure context, and the bug report issue template’s labels front matter is currently structured to apply a single combined label instead of multiple labels.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Medium severity · 2 Low severity

Open (4)
What changed in this PR

This PR continues the fleet-wide harmonization effort for plugin_* repositories by aligning this plugin’s CI workflows, templates, and test structure with the emerging shared conventions.

Changes:

  • Harmonizes CI workflows: pins Actions to commit SHAs, hardens MySQL bootstrap, simplifies Pest invocation, and removes dead workflow steps.
  • Updates test structure: removes unused tests/TestCase.php and replaces the legacy PHP-7.4 compatibility checks with a PHP 8.2-floor compatibility test.
  • Adds repository templates/documentation: introduces issue/PR templates and extends .github/copilot-instructions.md with CI/dependency baselines.
File Description
tests/​TestCase.php Removes an unused PHPUnit base class.
tests/​Security/​PhpCompatibilityTest.php Adds a PHP-floor compatibility test scanning plugin PHP sources for PHP 8.3/8.4-only constructs.
tests/​Security/​Php74CompatibilityTest.php Removes the old PHP 7.4-era compatibility checks.
tests/​bootstrap-unit.php Stops requiring the removed TestCase.php.
.github/​workflows/​plugin-ci-workflow.yml Aligns CI steps with pinned actions, safer DB bootstrap, syntax-check pattern, and simplified Pest invocation.
.github/​workflows/​codeql.yml Adds CodeQL workflow (JS/TS analysis only).
.github/​PULL_REQUEST_TEMPLATE.md Adds a PR template aligned with the fleet pattern.
.github/​ISSUE_TEMPLATE/​feature_request.md Adds a feature request template.
.github/​ISSUE_TEMPLATE/​bug_report.md Adds a bug report template.
.github/​copilot-instructions.md Documents CI/dependency baselines for Copilot guidance.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/ISSUE_TEMPLATE/bug_report.md
Comment thread tests/Security/PhpCompatibilityTest.php Outdated
Comment thread tests/Security/PhpCompatibilityTest.php
Comment thread tests/Security/PhpCompatibilityTest.php Outdated
- Document the locales/build_gettext.sh + cacti.pot workflow in
  copilot-instructions.md (Weblate owns per-language .po/.mo sync).
- Add a CI step that regenerates cacti.pot and fails if it is stale,
  ignoring the POT-Creation-Date timestamp.
- Mark locales/build_gettext.sh executable.
- Bump the CI MariaDB service image floor to 11.8 (from mariadb:10.6 /
  mysql:8.0).
- mariadb:11.8 no longer ships mysqladmin; use mariadb-admin for the
  service container healthcheck.
- Run chmod/build_gettext.sh with sudo in the i18n verification step,
  since locales/ is owned by www-data by the time it runs.
Ran the real locales/build_gettext.sh (xgettext/msgmerge/msgfmt) to
refresh the translation template against current source. Per the i18n
workflow documented in copilot-instructions.md, only the regenerated
cacti.pot is committed here; Weblate owns syncing the per-language
.po/.mo files from it.
…ibilityTest

Matches the project's PSR coding standard (short array syntax, single
quotes for non-interpolated strings) that php-cs-fixer enforces in CI.
`find` without `sort` returns filesystem/readdir-order results, which
can differ between machines (e.g. a local regen vs. a GitHub Actions
runner), producing a spurious reordering diff in cacti.pot even when
no strings actually changed. Pipe through `sort` and regenerate
cacti.pot with the now-deterministic order.
- tests/Security/PhpCompatibilityTest.php: fixed the #[Override]/#[Deprecated]
  attribute regexes to also match the fully qualified (`#[\Override]`,
  `#[\Deprecated]`) form, added a realpath() false-guard for the plugin root,
  included the relative file path in both RuntimeException messages, added
  typed-class-constant and dynamic-class-constant-fetch checks (PHP 8.3), and
  restored each()/create_function() removed-in-PHP-8.0 guards that the
  consolidation had dropped. Also excludes include/vendor/ (not just vendor/)
  from the recursive source scan.
- .github/workflows/plugin-ci-workflow.yml: the "Create MySQL Config" step's
  root password is now masked via `::add-mask::` before it's echoed into
  ~/.my.cnf, so it no longer appears in plaintext in the Actions log (chmod
  600 alone only protected the file after creation, not the command's own
  echoed source in the log).
@TheWitness

Copy link
Copy Markdown
Member Author

This PR's tests/Security/PhpCompatibilityTest.php #[Override]/#[Deprecated] regexes now also match the fully qualified form (#[\Override], #[\Deprecated]), a realpath() false-guard was added for the plugin root, both exception messages now include the relative file path, typed-class-constant and dynamic-class-constant-fetch checks (PHP 8.3) were added, and each()/create_function() (removed in PHP 8.0) guards were restored. The CI workflow's MySQL root password is now masked via ::add-mask:: before being written to ~/.my.cnf, so it no longer appears in plaintext in the Actions log. Resolving the corresponding review threads.

@TheWitness

Copy link
Copy Markdown
Member Author

Acknowledged, left as-is: verified against Cacti/cacti's own actual bug_report.md (fetched from develop) - it has the exact same labels: 'type:bug,status:needs-verification' quoted-string front matter (not a proper YAML list), so this template is genuinely byte-parity with Cacti core's own template, per the earlier fleet-wide decision to match Cacti's issue templates verbatim rather than fix them independently per-plugin. This is a pre-existing upstream quirk, not something introduced by this PR.

The each()-removed-in-PHP-8.0 check matched jQuery's $.each(/.each(
calls too, which have nothing to do with the removed PHP global
function. Exclude any each( preceded by . or > via a negative
lookbehind.
@TheWitness
TheWitness merged commit c59b114 into main Sep 21, 2026
5 checks passed
@TheWitness
TheWitness deleted the chore/ci-and-template-harmonization branch September 21, 2026 19:44
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.

3 participants