Repository navigation
Respect Graph Template Test Data Source in graph automation - #128
Merged
Merged
Conversation
Gate add_graphs.php invocations on test_data_sources() so a Graph Template with Test Data Source enabled is not graphed on devices whose data source returns no valid data. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Overwriting GLOBALS[config] wiped library_path and broke SetupWiringTest when the full suite runs in one process. The gate tests do not assert on the command path, so the override is unnecessary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new SNMP gates can crash automation because the required library is not loaded, while the tests overwrite shared configuration without restoring it, breaking subsequent metadata assertions.
2 open issues
What changed in this PR
This PR makes MikroTik graph automation honor Cacti’s per-template Test Data Source setting, skipping graph creation when enabled validation fails.
Changes:
- Add validation gates for device, summary-template, and data-query graphs, with debug messages for skipped graphs.
- Add unit tests covering creation and skipping in all three paths.
- Extend prepared-query test stubs to use database fixtures while preserving default results.
| File | Description |
|---|---|
tests/Unit/PollerGraphsTest.php |
Adds tests for the three graph-creation validation gates. |
tests/bootstrap-unit.php |
Enables fixture responses for prepared database queries. |
poller_graphs.php |
Checks data sources before invoking graph creation. |
Review effort: Balanced
… gate Load poller_graphs.php via require (guarded by MIKROTIK_POLLER_GRAPHS_NO_MAIN) so Xdebug attributes coverage to the real file, and exercise the skip branch of mikrotik_gt_graph()/mikrotik_dq_graphs(). Revert the gate in the unused (commented-out) summary path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…urces test_data_source() calls cacti_snmp_session()/cacti_snmp_session_get() from lib/snmp.php. Without it, enabling Test Data Source on an SNMP Graph Template aborts automation with a fatal undefined-function error. The include is in the untestable CLI bootstrap, so it is excluded from coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
xmacan
approved these changes
Nov 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The MikroTik graph automation (
poller_graphs.php) created graphs by shelling out to Cacti'scli/add_graphs.php, which callscreate_complete_graph_from_template()directly and never tests the data source. As a result, the automation ignored the per–Graph Template "Test Data Source" setting (graph_templates.test_source) that Cacti's GUI graph creation honors viacreate_save_graph()→test_data_sources().In practice this created graphs (for example Login Users / Processes from the Host Resources MIB) on MikroTik/RouterOS devices that expose those OIDs but return no valid data for them.
Fix
Gate every
add_graphs.phpinvocation on core'stest_data_sources(), mirroringcreate_save_graph():mikrotik_gt_graph()— device/health template graphsadd_summary_graphs()mikrotik_dq_graphs()— data-query graphscggraphs usetest_data_sources($graph_template_id, $host_id);dsgraphs usetest_data_sources($graph_template_id, $host_id, $query_id, $snmp_index).When a template's "Test Data Source" is off (the default),
test_data_sources()returns true and behavior is unchanged. When it is on and the data source returns no valid data, the graph is skipped and a debug line is emitted.Testing
php -lclean.tests/Unit/PollerGraphsTest.php) cover both the skip and create branches ofmikrotik_gt_graph(),mikrotik_dq_graphs(), andadd_summary_graphs()— all pass; existing suites remain green.db_fetch_*_prepared()honormikrotik_test_mock_db()fixtures (default return values unchanged).