test: add unit coverage for Phase 1 modules (cloudflare, messenger, rblcheck, servercheck, serverstats)#10
Open
zeroth-blip wants to merge 8 commits into
Open
Conversation
Member
|
@zeroth-blip thanks for the PR, reviewing it we noticed some changes in the core Will you be able to refactor your unit test code, and make it similar to the current unit tests? |
Contributor
Author
|
Understood, I will look what I can do at first opportunity. |
Covers init() GD-absent fallback, charts_html() layout with and without cc_lookups, and minmaxavg() observed through graphs_html() rendering (MIN/MAX/AVG accumulation and per-bucket isolation across HOUR/DAY/WEEK/ MONTH timeframes).
Covers checktarget() target classification (country/ip_range/ip), getscope() parsing of DOMAIN/DISABLE/ANY directives in csf.cloudflare, and the LWP interaction surface of block(), whitelist(), and remove() (success body, error status-line surfacing, and the empty-id remove branch).
Covers init() returning a blessed v2 instance without touching the v1/v3 side effects, and _read_request_line() against a fake client (plain LF termination, CRLF stripping, and MAX_LINE_LENGTH cap behavior).
Covers report()'s externally observable behavior under non-verbose mode: empty-IP set returns no failures and only the endoutput marker, a fresh PUBLIC IP gets a "New ... (PUBLIC)" / "Not Checked" placeholder when no cached result exists, and non-PUBLIC IPs are silently skipped. Mocks ConfigServer::GetEthDev, slurp, and rbllookup; reloads the module between calls to reset its file-scope output buffer.
The module's surface is dominated by hardcoded reads of /etc/csf, /proc, /usr/local/cpanel, and shellouts to iptables/systemctl/chkconfig, all captured into a file-scope output lexical. Without Phase-B refactor seams this can only be smoke-tested. Asserts that the module loads cleanly, exports the documented entry points, and that getportinfo() returns 0 for out-of-range ports when /proc/net is available (skipped elsewhere).
zeroth-blip
force-pushed
the
rework/csf-testability
branch
from
4d197db to
27e774b
Compare
zeroth-blip
changed the title
Contributor
Author
|
@UptimeEnforcer — heads up: after a deeper review I decided the original Full suite now at 20 files / 107 tests, all green. PR title + body updated to match. A separate, smaller |
…w-perl CloudFlare.pm has a top-level 'use LWP::UserAgent', which fails to compile on CI runners (or any host) without libwww-perl installed. Pre-populate $INC and provide minimal package stubs so the module loads in isolation; each subtest still overrides the relevant LWP::UserAgent method with a local *glob assignment, so behaviour is unchanged.
Adds the higher-value messenger coverage that this fork was missing and
that fits the existing test infrastructure without invasive changes to
Messenger.pm:
* module public surface: can('init'), can('start'), can('messengerv2'),
$VERSION presence + numeric baseline check (catches accidental
renames/regressions)
* init(1) returns a blessed object once ConfigServer::GetEthDev is
stubbed out (no real network probing)
* init(3) returns a blessed object without dying when
/var/lib/csf/ssl/ is not writable (CI runner case)
* init(1) tolerates a populated RECAPTCHA_NAT list (spaces +
multiple comma-separated entries)
* init(1) tolerates MESSENGER6 + IPV6 enabled together
* _read_request_line accepts a line of exactly MAX_LINE_LENGTH bytes
terminated by LF (the upper-bound boundary case)
Subtests that introspect %ConfigServer::Messenger::config (cached config
matrices: DEBUG levels, MESSENGER_HTTPS_SKIPMAIL, RECAPTCHA_SITEKEY,
webserver type) are intentionally not added here: %config is a file-
lexical 'my' in this fork's Messenger.pm, so it cannot be read from a
test without an out-of-scope re-scoping change. Those checks effectively
exercise ConfigServer::Config, not Messenger, and belong in a separate
ConfigServer::Config test.
messenger.t: 4 subtests -> 10 subtests, 7 asserts -> 17 asserts.
…el installs Keeps the NOTE generic about hardcoded install-prefix paths without referencing a specific vendor by name.
zeroth-blip
force-pushed
the
rework/csf-testability
branch
from
7f187d2 to
659808a
Compare
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
2 participants
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
Pivot from invasive csf.pl refactor → additive unit test coverage.
The original commit (
4d197db) restructured ~300 lines ofcsf.plto introduce a testing seam (modulino + dispatch refactor). After review we decided that level of change in the entry point is too risky for the goal at hand and reverted it.This PR now adds 5 new test files covering Phase 1 helper modules, with no changes to production code:
cloudflare.t—ConfigServer::CloudFlaremessenger.t—ConfigServer::Messengerrblcheck.t—ConfigServer::RBLCheckservercheck.t—ConfigServer::ServerCheckserverstats.t—ConfigServer::ServerStatsTest results
All existing tests still green; +5 new test files, +107 total assertions.
Notes for maintainers
csf.plor any production code in this PR.csf.plcan follow if/when there's appetite — happy to scope it tightly (e.g. one extracted sub at a time) so it's easier to review.