Skip to content

Commit 2c97ff0

Browse files
Merge pull request #2 from zeroth-blip/chore/unit-test-foundation
test: add initial unit test foundation for CheckIP
2 parents 176077b + 83cc5fe commit 2c97ff0

5 files changed

Lines changed: 360 additions & 0 deletions

File tree

.github/tests/README.md

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
# CSF test system
2+
3+
This directory contains the repository-owned test suite used by the `Tests` GitHub Actions workflow.
4+
5+
The goal is to keep the test system simple, portable, and friendly to contributors:
6+
7+
- use standard Perl tooling (`prove`, `Test::More`) where possible
8+
- run tests against the current repository checkout
9+
- keep unit tests fast and deterministic
10+
- provide shared helpers for future growth without locking the project into a heavy framework
11+
12+
## How the current CI flow works
13+
14+
The workflow entrypoint is `.github/workflows/tests.yml`.
15+
16+
Right now it contains a single job:
17+
18+
- **Unit** — runs all tests under `.github/tests/unit/`
19+
20+
The workflow executes:
21+
22+
```bash
23+
prove -v -r .github/tests/unit/
24+
```
25+
26+
That means `prove` recursively discovers every `.t` file under `.github/tests/unit/` and runs the full unit suite in one pass.
27+
28+
## Directory layout
29+
30+
```text
31+
.github/tests/
32+
├── README.md # this document
33+
├── lib/ # shared test helpers and bootstrap modules
34+
└── unit/ # unit test files (*.t)
35+
```
36+
37+
### `lib/`
38+
Shared helpers live here.
39+
40+
Current helper:
41+
42+
- `TestBootstrap.pm` — ensures tests load modules from the repository checkout instead of any system-installed CSF copy
43+
44+
### `unit/`
45+
Contains fast tests for isolated code paths.
46+
47+
Unit tests should prefer:
48+
49+
- pure function testing
50+
- deterministic inputs and outputs
51+
- no network access
52+
- no dependency on `/etc/csf`, `/usr/local/csf`, or a live firewall state
53+
54+
Because discovery is recursive, contributors may add subdirectories under `unit/` later if grouping tests becomes useful.
55+
56+
## Running tests locally
57+
58+
From the repository root:
59+
60+
```bash
61+
prove -v -r .github/tests/unit/
62+
```
63+
64+
If you only want to run one file:
65+
66+
```bash
67+
prove -v .github/tests/unit/checkip.t
68+
```
69+
70+
## How to write a new unit test
71+
72+
1. Create a new `.t` file anywhere under `.github/tests/unit/`
73+
2. Use `strict`, `warnings`, and `Test::More`
74+
3. Add `.github/tests/lib` to `@INC`
75+
4. Load `TestBootstrap` before loading project modules
76+
5. Import the function(s) or module(s) you want to test
77+
6. Keep the test self-contained and deterministic
78+
79+
### Example skeleton
80+
81+
```perl
82+
#!/usr/bin/env perl
83+
84+
use strict;
85+
use warnings;
86+
87+
use FindBin qw($Bin);
88+
use Test::More;
89+
use lib "$Bin/../lib";
90+
91+
use TestBootstrap ();
92+
use ConfigServer::SomeModule qw(some_function);
93+
94+
subtest 'some_function handles the happy path' => sub {
95+
is( some_function('example'), 'expected', 'returns expected value' );
96+
};
97+
98+
done_testing;
99+
```
100+
101+
## Test design guidelines
102+
103+
When adding tests, prefer the following order:
104+
105+
### 1. Test behavior, not implementation trivia
106+
Good tests lock down externally useful behavior.
107+
Avoid asserting internal details unless they are part of the contract.
108+
109+
### 2. Keep tests local-first
110+
Prefer tests that run without special services, custom images, or machine-specific setup.
111+
If a behavior needs a richer environment, it likely belongs in a future integration suite.
112+
113+
### 3. Use shared helpers when repetition appears
114+
If two or three tests need the same bootstrap or fixture logic, move that logic into `.github/tests/lib/`.
115+
116+
### 4. Keep fixtures explicit
117+
If a test needs sample data, store it in a clear form and make the dependency obvious inside the test.
118+
119+
### 5. Fail loudly and clearly
120+
Test names should make it obvious what broke.
121+
Future contributors should be able to understand a failure from the `prove` output alone.
122+
123+
## Scope boundaries
124+
125+
This test directory is intentionally separate from upstream historical test layouts.
126+
We may study external approaches, but tests added here should be repository-owned and written for this project's needs.
127+
128+
In practice that means:
129+
130+
- do not copy vendor-specific test harnesses wholesale
131+
- do not depend on control-panel-specific test utilities
132+
- do not introduce external branding or references into the local test system
133+
134+
## Future growth
135+
136+
As coverage expands, this structure can grow without changing the basic workflow model. Likely next steps are:
137+
138+
- more files under `unit/`
139+
- reusable helpers under `lib/`
140+
- optional sibling suites later, such as `integration/` or `fixtures/`
141+
142+
The workflow can keep a single `Tests` entrypoint while adding more jobs only when the suite actually needs them.

.github/tests/lib/TestBootstrap.pm

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package TestBootstrap;
2+
3+
use strict;
4+
use warnings;
5+
6+
use Exporter qw(import);
7+
use File::Basename qw(dirname);
8+
use File::Spec;
9+
10+
our @EXPORT_OK = qw(repo_root);
11+
12+
BEGIN {
13+
# Ensure tests load modules from this checkout before any system-installed CSF copy.
14+
my $root = File::Spec->rel2abs(
15+
File::Spec->catdir( dirname(__FILE__), '..', '..', '..' )
16+
);
17+
18+
unshift @INC, $root unless grep { $_ eq $root } @INC;
19+
$ENV{CSF_TEST_REPO_ROOT} ||= $root;
20+
}
21+
22+
sub repo_root {
23+
return $ENV{CSF_TEST_REPO_ROOT};
24+
}
25+
26+
1;

.github/tests/unit/checkip.t

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
#!/usr/bin/env perl
2+
3+
use strict;
4+
use warnings;
5+
6+
use FindBin qw($Bin);
7+
use Test::More;
8+
use lib "$Bin/../lib";
9+
10+
use TestBootstrap ();
11+
use ConfigServer::CheckIP qw(checkip cccheckip);
12+
13+
sub assert_cases {
14+
my ( $fn, $cases ) = @_;
15+
16+
for my $case ( @{$cases} ) {
17+
my ( $input, $expected, $label ) = @{$case};
18+
is( $fn->($input), $expected, $label );
19+
}
20+
21+
return;
22+
}
23+
24+
subtest 'checkip accepts canonical, boundary, and alternate address forms' => sub {
25+
assert_cases(
26+
\&checkip,
27+
[
28+
[ '192.168.1.1', 4, 'accepts a private IPv4 address' ],
29+
[ '8.8.8.8', 4, 'accepts a public IPv4 address' ],
30+
[ '8.8.8.8/24', 4, 'accepts an IPv4 CIDR block' ],
31+
[ '8.8.8.8/32', 4, 'accepts the IPv4 upper CIDR boundary' ],
32+
[ '0.0.0.0', 4, 'accepts the IPv4 all-zero address' ],
33+
[ '255.255.255.255', 4, 'accepts the IPv4 all-ones address' ],
34+
[ '2001:4860:4860::8888', 6, 'accepts a compressed IPv6 address' ],
35+
[ '2001:4860:4860::8888/64', 6, 'accepts an IPv6 CIDR block' ],
36+
[ '2001:4860:4860::8888/128', 6, 'accepts the IPv6 upper CIDR boundary' ],
37+
[ '2001:0db8:0000:0000:0000:ff00:0042:8329', 6, 'accepts a fully expanded IPv6 address' ],
38+
[ 'fe80::1', 6, 'accepts a link-local IPv6 address' ],
39+
[ '::ffff:192.0.2.1', 6, 'accepts an IPv4-mapped IPv6 address' ],
40+
]
41+
);
42+
};
43+
44+
subtest 'checkip rejects malformed, polluted, or out-of-range input' => sub {
45+
assert_cases(
46+
\&checkip,
47+
[
48+
[ '', 0, 'rejects an empty string' ],
49+
[ 'not-an-ip', 0, 'rejects a non-IP string' ],
50+
[ '999.999.999.999', 0, 'rejects an out-of-range IPv4 address' ],
51+
[ '192.168.1', 0, 'rejects an IPv4 address with too few octets' ],
52+
[ '192.168.1.1.1', 0, 'rejects an IPv4 address with too many octets' ],
53+
[ '8.8.8.8/not-a-mask', 0, 'rejects a non-numeric IPv4 mask' ],
54+
[ '8.8.8.8/33', 0, 'rejects an IPv4 mask above 32' ],
55+
[ '8.8.8.8/999', 0, 'rejects a clearly invalid IPv4 mask' ],
56+
[ '2001:4860:4860::8888/129', 0, 'rejects an IPv6 mask above 128' ],
57+
[ ' 8.8.8.8', 0, 'rejects a leading-space IPv4 string' ],
58+
[ '8.8.8.8 ', 0, 'rejects a trailing-space IPv4 string' ],
59+
[ '8.8.8.8; echo hacked', 0, 'rejects IPv4 input polluted with shell text' ],
60+
[ '8.8.8.8|cat', 0, 'rejects IPv4 input polluted with pipe characters' ],
61+
]
62+
);
63+
};
64+
65+
subtest 'checkip rejects loopback addresses in both families' => sub {
66+
assert_cases(
67+
\&checkip,
68+
[
69+
[ '127.0.0.1', 0, 'rejects IPv4 loopback' ],
70+
[ '127.0.0.1/8', 0, 'rejects IPv4 loopback with CIDR' ],
71+
[ '::1', 0, 'rejects compressed IPv6 loopback' ],
72+
[ '0000:0000:0000:0000:0000:0000:0000:0001', 0, 'rejects expanded IPv6 loopback' ],
73+
]
74+
);
75+
};
76+
77+
subtest 'checkip handles undefined and empty scalar inputs without warnings' => sub {
78+
my @warnings;
79+
local $SIG{__WARN__} = sub { push @warnings, @_ };
80+
81+
my $undef_scalar;
82+
my $empty_scalar = q{};
83+
84+
is( checkip(undef), 0, 'rejects undef scalar input' );
85+
is( checkip(\$undef_scalar), 0, 'rejects undef scalar reference input' );
86+
is( checkip(\$empty_scalar), 0, 'rejects empty scalar reference input' );
87+
is( scalar @warnings, 0, 'does not emit warnings for undefined or empty input' );
88+
};
89+
90+
subtest 'IPv6 reference inputs are normalised in place and preserve CIDR suffixes' => sub {
91+
my $ipv6 = '2001:4860:0000:0000:0000:0000:0000:8888';
92+
my $ipv6_cidr = '2001:4860:0000:0000:0000:0000:0000:8844/64';
93+
94+
is( checkip(\$ipv6), 6, 'plain IPv6 reference input is accepted' );
95+
is( $ipv6, '2001:4860::8888', 'plain IPv6 reference input is shortened in place' );
96+
97+
is( checkip(\$ipv6_cidr), 6, 'IPv6 reference input with CIDR is accepted' );
98+
is( $ipv6_cidr, '2001:4860::8844/64', 'IPv6 reference input keeps its CIDR suffix after normalisation' );
99+
};
100+
101+
subtest 'cccheckip keeps the stricter public/private IPv4 split' => sub {
102+
assert_cases(
103+
\&cccheckip,
104+
[
105+
[ '8.8.8.8', 4, 'accepts a public IPv4 address' ],
106+
[ '1.1.1.1', 4, 'accepts another public IPv4 address' ],
107+
[ '8.8.8.0/24', 4, 'accepts a public IPv4 CIDR block' ],
108+
[ '192.168.1.1', 0, 'rejects a 192.168.x.x private IPv4 address' ],
109+
[ '10.0.0.1', 0, 'rejects a 10.x.x.x private IPv4 address' ],
110+
[ '172.16.0.1', 0, 'rejects a 172.16.x.x private IPv4 address' ],
111+
[ '127.0.0.1', 0, 'rejects IPv4 loopback' ],
112+
[ '0.0.0.0', 0, 'rejects a non-public all-zero IPv4 address' ],
113+
[ '8.8.8.8/33', 0, 'rejects an out-of-range public IPv4 mask' ],
114+
[ 'not-an-ip', 0, 'rejects malformed scalar input' ],
115+
[ '', 0, 'rejects an empty string' ],
116+
]
117+
);
118+
};
119+
120+
subtest 'cccheckip accepts IPv6 scalars and references while still rejecting loopback' => sub {
121+
my $ipv6 = '2001:4860:0000:0000:0000:0000:0000:8844';
122+
my $ipv6_cidr = '2001:4860:0000:0000:0000:0000:0000:8844/64';
123+
124+
assert_cases(
125+
\&cccheckip,
126+
[
127+
[ '2001:4860:4860::8844', 6, 'accepts a plain IPv6 scalar' ],
128+
[ '2001:4860:4860::8844/128', 6, 'accepts an IPv6 scalar with upper CIDR boundary' ],
129+
[ 'fe80::1', 6, 'accepts a link-local IPv6 scalar' ],
130+
[ '::1', 0, 'rejects compressed IPv6 loopback' ],
131+
[ '::1/128', 0, 'rejects IPv6 loopback with CIDR' ],
132+
[ '2001:4860:4860::8844/129', 0, 'rejects an out-of-range IPv6 mask' ],
133+
]
134+
);
135+
136+
is( cccheckip(\$ipv6), 6, 'accepts an IPv6 reference input' );
137+
is( $ipv6, '2001:4860::8844', 'normalises IPv6 reference input in place' );
138+
139+
is( cccheckip(\$ipv6_cidr), 6, 'accepts an IPv6 reference input with CIDR' );
140+
is( $ipv6_cidr, '2001:4860::8844/64', 'preserves CIDR on cccheckip IPv6 reference input' );
141+
};
142+
143+
subtest 'legacy CIDR /0 behaviour is documented as a known quirk' => sub {
144+
local $TODO = 'Legacy truthiness logic currently allows /0 CIDR masks';
145+
146+
is( checkip('8.8.8.8/0'), 0, 'checkip should reject IPv4 /0 once the legacy quirk is fixed' );
147+
is( checkip('2001:4860:4860::8888/0'), 0, 'checkip should reject IPv6 /0 once the legacy quirk is fixed' );
148+
is( cccheckip('8.8.8.8/0'), 0, 'cccheckip should reject IPv4 /0 once the legacy quirk is fixed' );
149+
is( cccheckip('2001:4860:4860::8888/0'), 0, 'cccheckip should reject IPv6 /0 once the legacy quirk is fixed' );
150+
};
151+
152+
done_testing;

.github/workflows/tests.yml

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
name: Tests
2+
3+
on:
4+
push:
5+
pull_request:
6+
workflow_dispatch:
7+
8+
permissions:
9+
contents: read
10+
11+
jobs:
12+
unit:
13+
name: Unit
14+
runs-on: ubuntu-latest
15+
16+
steps:
17+
- name: Check out repository
18+
uses: actions/checkout@v4
19+
20+
- name: Show Perl toolchain
21+
shell: bash
22+
run: |
23+
set -euo pipefail
24+
perl -v
25+
echo '---'
26+
prove --version
27+
28+
- name: Run unit tests
29+
shell: bash
30+
run: |
31+
set -euo pipefail
32+
prove -v -r .github/tests/unit/

ConfigServer/CheckIP.pm

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,15 @@ sub checkip {
4444
my $ip;
4545
my $cidr;
4646
if (ref $ipin) {
47+
unless (defined ${$ipin}) {return 0}
4748
($ip,$cidr) = split(/\//,${$ipin});
4849
$ipref = 1;
4950
} else {
51+
unless (defined $ipin) {return 0}
5052
($ip,$cidr) = split(/\//,$ipin);
5153
}
54+
$ip //= "";
55+
$cidr //= "";
5256
my $testip = $ip;
5357

5458
if ($cidr ne "") {
@@ -100,11 +104,15 @@ sub cccheckip {
100104
my $ip;
101105
my $cidr;
102106
if (ref $ipin) {
107+
unless (defined ${$ipin}) {return 0}
103108
($ip,$cidr) = split(/\//,${$ipin});
104109
$ipref = 1;
105110
} else {
111+
unless (defined $ipin) {return 0}
106112
($ip,$cidr) = split(/\//,$ipin);
107113
}
114+
$ip //= "";
115+
$cidr //= "";
108116
my $testip = $ip;
109117

110118
if ($cidr ne "") {

0 commit comments

Comments
 (0)