Skip to content

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

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

chore: harmonize CI workflow, templates, and test structure#369
TheWitness merged 6 commits into
developfrom
chore/ci-and-template-harmonization

Conversation

@TheWitness

@TheWitness TheWitness commented Sep 21, 2026

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.

@TheWitness
chore: harmonize CI workflow, templates, and test structure
4986793 
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.
@TheWitness
TheWitness requested review from bmfmancini and xmacan and a lite review from Copilot and removed request for Copilot September 21, 2026 15:35
@TheWitness
chore: add i18n build/verify workflow and bump MariaDB floor to 11.8
02d3679 
- 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).
Copilot AI lite review requested due to automatic review settings September 21, 2026 16:01
Copilot AI reviewed Sep 21, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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

Critical and moderate CI, fixture, seeding, localization, and Playwright issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
What changed in this PR

This PR harmonizes CI workflows, testing structure, localization tooling, E2E checks, and repository templates.

Changes:

  • Updates CI action pinning, MariaDB setup, Composer/Pest handling, and CodeQL configuration.
  • Adds fixtures, Playwright tooling, gettext verification, and syslog test-data seeding.
  • Adds GitHub templates and documents CI and dependency conventions.
File Description
tests/​Fixtures/​saved_template_editor.php Adds saved-search editor rendering fixture.
tests/​Fixtures/​compact_workspace.php Adds compact workspace browser fixture.
tests/​E2E/​run-orb-docker-e2e.sh Updates E2E paths and Playwright setup.
tests/​E2E/​playwright/​run-e2e.sh Updates Playwright execution paths.
tests/​E2E/​playwright/​package.json Defines the Playwright dependency.
tests/​E2E/​playwright/​browser-check.js Adds browser login and page checks.
tests/​browser/​compact_workspace.cjs Uses the renamed fixture path.
tests/​bin/​populate_syslog_incoming.sh Seeds integration-test data.
locales/​build_gettext.sh Adds gettext catalog generation tooling.
.github/​workflows/​plugin-ci-workflow.yml Harmonizes integration CI and database setup.
.github/​workflows/​codeql.yml Updates CodeQL actions and permissions.
.github/​PULL_REQUEST_TEMPLATE.md Adds a standardized PR template.
.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, and localization baselines.

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

Comment thread .github/workflows/plugin-ci-workflow.yml Outdated
@TheWitness
fix: mariadb:11.8 healthcheck and i18n step permissions
a3437d0 
- 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.
@TheWitness
fix: make locales/build_gettext.sh string extraction order deterministic
b6ae78f 
`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.
@TheWitness
fix: Address Copilot review findings on the harmonization PR
4293aeb 
- 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
fix: Address remaining Copilot review findings
5406b4e 
- CHANGELOG.md: added an entry documenting the CI/template harmonization
  change (was previously missing per review feedback).
- locales/po/cacti.pot (+ .po/.mo): regenerated via locales/build_gettext.sh
  so the new "Verify translation template is up to date" CI check actually
  passes against the current source tree instead of comparing against a
  stale committed POT.
- locales/build_gettext.sh: added --from-code=UTF-8 to the xgettext
  invocation - without it, xgettext aborts outright on any non-ASCII string
  in the source (e.g. a "…" character), silently leaving the previous
  cacti.pot in place and defeating the up-to-date check entirely.
- tests/Security/PhpCompatibilityTest.php: dropped the PHP 8.3-only checks
  (kept only the PHP 8.4-only ones) and corrected the floor comment, since
  this repo's actual CI matrix only runs PHP 8.3/8.4 (no 8.2) - it tracks
  Cacti's `develop` branch, not `1.2.x`.
- tests/Security/PhpCompatibilityTest.php: renamed the local
  plugin_test_read_source_file() helper to schema_test_read_source_file(),
  matching this repo's own naming convention (schema_ prefix for non-
  lifecycle helpers).
- tests/Helpers/ThresholdScenario.php: fixed a flush-left statement inside
  inMaintenance() to use tab indentation, matching the surrounding code.
- README.md / tests/README.md / copilot-instructions.md: corrected stale
  PHP-floor references (7.4/8.1 -> 8.2) and a stale test-filename reference
  (Php82CompatibilityTest.php -> the consolidated PhpCompatibilityTest.php)
  left over from the harmonization pass.
@TheWitness

TheWitness commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

Fixed: the real root cause was xgettext aborting outright on the non-ASCII … character in lib/syslog_dashboard.php:1622 (no --from-code specified), which silently left the previous stale cacti.pot in place every time the script ran - added --from-code=UTF-8 to locales/build_gettext.sh's xgettext invocation and regenerated/committed the corrected cacti.pot.

@TheWitness
TheWitness merged commit c4c3ed4 into develop Sep 21, 2026
7 checks passed
@TheWitness
TheWitness deleted the chore/ci-and-template-harmonization branch September 21, 2026 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@xmacan xmacan xmacan approved these changes
@bmfmancini bmfmancini Awaiting requested review from bmfmancini

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@TheWitness @xmacan