Fix runtime dependency handling and verify production scanner path - #344
Closed
somethingwithproof wants to merge 3 commits into
Closed
Fix runtime dependency handling and verify production scanner path#344somethingwithproof wants to merge 3 commits into
somethingwithproof wants to merge 3 commits into
Conversation
somethingwithproof
added
bug
QA
19 tasks
Member
Author
|
Superseded by #345. Plugin directories carry no composer.json, so the fail-closed bootstrap here is the wrong direction; the Docker production-path harness will come back as its own PR. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new dependency bootstrap can still fatally error in the presence of an unreadable autoload.php, and the new production probe has a concrete argument/row-validation bug that should be fixed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens MacTrack’s runtime dependency handling so missing Composer installs fail closed with actionable diagnostics (instead of fatal errors), and adds Docker-based CI coverage that exercises the real production scanner/poller path against a live SNMP agent.
Changes:
- Introduce a centralized
DependencyBootstraphelper and wire it intosetup.phpandmactrack_resolver.phpto report missing/unloadable Composer dependencies and exit cleanly. - Add a dedicated runtime test suite + 100% coverage gate for the new runtime/bootstrap code, plus a production-path Docker E2E that runs the real scanner and poller and asserts persisted scan state.
- Update CI workflows to run the new Docker suites and install locked dependencies in clean checkouts.
File summaries
| File | Description |
|---|---|
| tests/Unit/test_device_type_sql_safety.php | Updates string-signature assertions to detect the new dependency bootstrap integration. |
| tests/Pest/Unit/Runtime/fixtures/working-autoload.php | Fixture autoloader that defines a required class for positive-path tests. |
| tests/Pest/Unit/Runtime/fixtures/throwing-autoload.php | Fixture autoloader that throws to test failure reporting. |
| tests/Pest/Unit/Runtime/fixtures/empty-autoload.php | Fixture autoloader with no classes to test “required class missing” handling. |
| tests/Pest/Unit/Runtime/DependencyBootstrapTest.php | Unit tests validating dependency bootstrap behavior and messaging. |
| tests/e2e/snmpd.conf | SNMP agent config for production-path Docker E2E. |
| tests/e2e/mactrack_production_probe.php | Seeds test device/type and asserts scanner/poller persistence against SNMP fixture. |
| tests/e2e/Dockerfile | Moves E2E image to PHP 8.3, adds unzip and Composer binary. |
| tests/e2e/docker-compose.yml | Adds an SNMP agent service and healthcheck dependency for E2E. |
| tests/e2e/bootstrap-mactrack.sh | Installs locked dependencies and runs scanner + poller + missing-deps resolver checks. |
| tests/docker/run-tests.sh | Convenience wrapper to build/run the Docker test image. |
| tests/docker/Dockerfile | Docker test runner for full Pest suite + runtime 100% coverage + composer validate. |
| src/Runtime/DependencyBootstrap.php | New centralized runtime Composer bootstrap with error reporting and required-class checks. |
| setup.php | Uses DependencyBootstrap to block enablement when runtime dependencies aren’t usable. |
| phpunit.runtime.xml | PHPUnit config for runtime-only suite and coverage include for src/Runtime. |
| mactrack_resolver.php | Uses DependencyBootstrap to fail closed (non-fatally) when Composer deps are absent/unloadable. |
| composer.json | Adds PSR-4 autoload mapping for new runtime code + runtime coverage script. |
| CHANGELOG.md | Notes issue #343 change. |
| .github/workflows/plugin-ci-workflow.yml | Adds production-path job; improves MySQL init command usage. |
| .github/workflows/code-quality.yml | Adds Docker runtime coverage job. |
| .dockerignore | Avoids sending .git, cache, and vendor/ into Docker builds. |
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+22
to
+26
| if (!is_file($autoloadPath)) { | ||
| $reporter('Mactrack requires Composer dependencies. Run composer install --no-dev in the plugin directory.'); | ||
|
|
||
| return false; | ||
| } |
Comment on lines
+43
to
+60
| $deviceId = filter_var($argv[2] ?? null, FILTER_VALIDATE_INT); | ||
|
|
||
| if ($deviceId === false) { | ||
| fwrite(STDERR, "Production probe requires a numeric device id\n"); | ||
| exit(2); | ||
| } | ||
|
|
||
| $device = db_fetch_row_prepared( | ||
| 'SELECT snmp_status, snmp_sysDescr, device_type_id, last_rundate, last_runmessage | ||
| FROM mac_track_devices WHERE device_id = ?', | ||
| [$deviceId] | ||
| ); | ||
|
|
||
| $passed = (int) ($device['snmp_status'] ?? 0) === HOST_UP | ||
| && str_contains((string) ($device['snmp_sysDescr'] ?? ''), 'Linux') | ||
| && (int) ($device['device_type_id'] ?? 0) > 0 | ||
| && ($device['last_rundate'] ?? '0000-00-00 00:00:00') !== '0000-00-00 00:00:00'; | ||
|
|
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.
Fixes #343
What this does
develop, MariaDB, and a live Net-SNMP agentmactrack_scanner.phpentry point and the realpoller_mactrack.phpscheduler-to-worker path, then verifies persisted scan stateValidation
tests/docker/run-tests.sh: 14 tests passed; new runtime code 100.00% covered; Composer validation passedtests/e2e/run-mactrack-e2e.sh: both lock files installed inside Docker, then Cacti installation, plugin installation, live SNMP scan, poller subprocess scan, database assertions, and missing-dependency failure path passed on PHP 8.3actionlint: passedgit diff --check: passedCI follow-up
The clean GitHub runner exposed two assumptions hidden by an existing local worktree: no archive extractor in the image, and incomplete committed Cacti autoload artifacts. The image now includes
unzip, and the bootstrap installs both Cacti and MacTrack lock files before execution. The same production test passes locally from clean dependency state; the pushed commits rerun GitHub CI.