Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions .github/workflows/plugin-ci-workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ jobs:
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
path: cacti/plugins/flowview
# patch-coverage.php diffs against the base branch below.
fetch-depth: 0

- name: Install PHP ${{ matrix.php }}
uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2
Expand Down Expand Up @@ -179,13 +181,23 @@ jobs:
run: echo -n "${{ env.CACTI }}" | sudo tee ${{ github.workspace }}/cacti/plugins/flowview/tests/.cacti-version > /dev/null

- name: Run Pest Unit Tests
env:
COMPOSER_ROOT_VERSION: 1.3.0-dev
run: |
cd ${{ github.workspace }}/cacti
include/vendor/bin/pest --configuration=plugins/flowview/phpunit.xml \
--coverage-clover=plugins/flowview/coverage/clover.xml

# Whole-file coverage is meaningless here: most of the plugin only runs
# inside a live Cacti. What is enforceable is that a change covers the
# lines it adds.
- name: Enforce coverage of changed lines
if: github.event_name == 'pull_request'
env:
BASE_REF: ${{ github.event.pull_request.base.sha }}
run: |
cd ${{ github.workspace }}/cacti/plugins/flowview
git config --global --add safe.directory ${{ github.workspace }}/cacti/plugins/flowview
php tests/bin/patch-coverage.php coverage/clover.xml "$BASE_REF" 100

- name: Upload coverage report
if: always()
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,4 @@ config.php
locales/po/*.mo
locales/LC_MESSAGES/*.mo
.omc/
locales/po/*.po
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

--- develop ---

* dev: Enforce patch coverage of changed lines in CI and remove the inert COMPOSER_ROOT_VERSION env from the Pest step
* issue#110: Add NAT (postNAT source/destination IP and port) support to the core raw flow schema, filters, and DNS resolution; see flowview_upgrade_nat_columns.php to backfill existing partitions
* security: Add a version-safe CSP nonce (`plugin_flowview_csp_nonce()`) to every inline `<script>` tag so pages stay compatible with Cacti's Content-Security-Policy nonce enforcement, while falling back cleanly on older Cacti releases that lack the `CactiSecureHeaders` class
* chore: Harmonize CI workflow, issue/PR templates, and PHP-compatibility test structure with the shared Cacti plugin baseline
Expand Down
198 changes: 198 additions & 0 deletions tests/bin/patch-coverage.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
| |
| This program is free software; you can redistribute it and/or |
| modify it under the terms of the GNU General Public License |
| as published by the Free Software Foundation; either version 2 |
| of the License, or (at your option) any later version. |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
| http://www.cacti.net/ |
+-------------------------------------------------------------------------+
*/

/*
* Report line coverage for the lines a branch changes.
*
* Whole-file coverage is not a useful gate here: most of the plugin only runs
* inside a live Cacti, so the repository figure would sit near zero no matter
* how well a change is tested. What a reviewer wants to know is whether the
* lines this branch adds are exercised, which is what this measures.
*
* Usage: php tests/bin/patch-coverage.php <clover.xml> <base-ref> [min-percent]
*
* Exits 1 if coverage is below the threshold, 2 on bad input.
*/

if ($argc < 3) {
fwrite(STDERR, "usage: patch-coverage.php <clover.xml> <base-ref> [min-percent]\n");

exit(2);
}

$clover_path = $argv[1];
$base_ref = $argv[2];
$minimum = isset($argv[3]) ? (float) $argv[3] : 100.0;

if (!is_readable($clover_path)) {
fwrite(STDERR, "cannot read coverage report: $clover_path\n");

exit(2);
}

/**
* Line numbers each measured file changed, keyed by repository-relative path.
*
* Only added and modified lines count. Deletions have nothing left to cover,
* and context lines were not part of this change.
*
* Paths stay repository-relative so the report can be produced in a container
* and evaluated on the host, where the absolute paths differ.
*
* @param string $base_ref Git ref to diff against.
*
* @return array<string, array<int, bool>>
*/
function changed_lines($base_ref) {
$command = 'git diff --no-ext-diff --unified=0 --no-color --diff-filter=AM ' . escapeshellarg($base_ref) . '...HEAD -- "*.php"';
$output = [];
$status = 0;

$last_line = exec($command, $output, $status);

if ($last_line === false || $status !== 0) {
fwrite(STDERR, "git diff failed\n");

exit(2);
}

$diff = implode("\n", $output);

$changed = [];
$file = null;

foreach (explode("\n", $diff) as $line) {
if (strncmp($line, '+++ b/', 6) === 0) {
$file = substr($line, 6);

if (strncmp($file, 'tests/', 6) === 0) {
$file = null;

continue;
}

$changed[$file] = [];
} elseif (strncmp($line, '@@', 2) === 0 && $file !== null) {
if (preg_match('/\+(\d+)(?:,(\d+))?/', $line, $match)) {
$start = (int) $match[1];
$count = isset($match[2]) ? (int) $match[2] : 1;

for ($i = 0; $i < $count; $i++) {
$changed[$file][$start + $i] = true;
}
}
}
}

return $changed;
}

$changed = changed_lines($base_ref);
$clover = simplexml_load_file($clover_path);

if ($clover === false) {
fwrite(STDERR, "cannot parse coverage report: $clover_path\n");

exit(2);
}

$covered = 0;
$total = 0;
$missing = [];
$measured = [];

foreach ($clover->xpath('//file') as $file) {
$path = (string) $file['name'];
$relative = null;

foreach (array_keys($changed) as $candidate) {
if ($path === $candidate || substr($path, -strlen('/' . $candidate)) === '/' . $candidate) {
$relative = $candidate;

break;
}
}

if ($relative === null) {
continue;
}

$measured[$relative] = true;

foreach ($file->line as $line) {
$number = (int) $line['num'];

// Only statement lines are measurable; method markers double-count.
if ((string) $line['type'] !== 'stmt' || !isset($changed[$relative][$number])) {
continue;
}

$total++;

if ((int) $line['count'] > 0) {
$covered++;
} else {
$missing[] = $relative . ':' . $number;
}
}
}

/*
* Production PHP entry points that cannot safely be loaded into the isolated
* unit process (CLI/daemon/web entry points that chdir + include auth.php,
* do pcntl signal handling, or execute at the top level) belong here, each
* with a one-line justification. Keep the exception explicit: any newly
* changed production PHP file must either appear in Clover or be added here.
*
* Empty by default; add entries per repository as the need arises.
*/
$unmeasured_allowlist = [
];
$unmeasured = array_values(array_diff(array_keys($changed), array_keys($measured)));
$unexpected_unmeasured = array_values(array_diff($unmeasured, $unmeasured_allowlist));

if ($unmeasured !== []) {
print "Changed production PHP files absent from Clover:\n " . implode("\n ", $unmeasured) . "\n";
}

if ($unexpected_unmeasured !== []) {
print "FAIL: changed production PHP files are not measured or allowlisted:\n "
. implode("\n ", $unexpected_unmeasured) . "\n";

exit(1);
}

if ($total === 0) {
print "Patch coverage: no measured lines changed.\n";

exit(0);
}

$percent = ($covered / $total) * 100;

printf("Patch coverage: %.2f%% (%d/%d lines)\n", $percent, $covered, $total);

if ($missing !== []) {
print "Uncovered changed lines:\n " . implode("\n ", $missing) . "\n";
}

if ($percent + 0.005 < $minimum) {
printf("FAIL: below the %.2f%% minimum.\n", $minimum);

exit(1);
}

exit(0);
Loading