[Test Coverage] Improve host-iptables-shared branch coverage#3403
Conversation
Add tests for uncovered branches in host-iptables-shared.ts: - cleanupChain: matchPredicate option - getNetworkBridgeName: empty output and error paths - getDockerBridgeGateway: empty output, invalid IPv4 format, error paths - isIp6tablesAvailable: cache hit path - disableIpv6ViaSysctl: success and failure paths - enableIpv6ViaSysctl: no-op when not disabled, re-enable path, failure path - addDnsRules: rollback when TCP rule fails, re-throws original error Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
This PR extends the unit test suite for src/host-iptables-shared.ts to improve branch coverage for security-sensitive iptables and IPv6/sysctl behaviors, without requiring Docker/iptables access at test time.
Changes:
- Added tests for previously uncovered branches in
cleanupChain,getNetworkBridgeName, andgetDockerBridgeGateway. - Added caching-path coverage for
isIp6tablesAvailable. - Added success/failure-path coverage for IPv6 sysctl toggling (
disableIpv6ViaSysctl/enableIpv6ViaSysctl) and DNS rule rollback behavior (addDnsRules).
Show a summary per file
| File | Description |
|---|---|
src/host-iptables-shared.test.ts |
Adds targeted unit tests to exercise untested branches in host-iptables-shared (bridge discovery, gateway validation, IPv6 handling, and DNS rule rollback). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 2
| it('returns null and warns when gateway is not a valid IPv4 address', async () => { | ||
| mockedExeca.mockResolvedValueOnce(execaResult({ stdout: 'not-an-ip' })); | ||
|
|
||
| const result = await getDockerBridgeGateway(); | ||
|
|
||
| expect(result).toBeNull(); | ||
| }); |
| describe('host-iptables-shared', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| testHelpers.resetIpv6State(); | ||
| }); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@copilot address review feedback |
Smoke Test Results✅ GitHub API: recent-prs.json contains 2 PR entries Result: PASS — All tests passed
|
🔥 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Overall: PARTIAL — BYOK inference ✅, pre-step data not substituted PR author:
|
Gemini Smoke Test Results
Overall Status: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
🔬 Smoke Test Results
PR: [Test Coverage] Improve host-iptables-shared branch coverage Overall: PASS (2/2 verifiable tests passed)
|
|
Smoke test Codex: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Chroot Smoke Test Results
Result: ❌ Not all tests passed — Python patch version and Node.js minor version differ between host and chroot environments.
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL —
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Summary
Extends
src/host-iptables-shared.test.tsto cover previously untested branches inhost-iptables-shared.ts, the security-critical module responsible for iptables rule management.Coverage Improvements
Before:
host-iptables.tshad 83.63% statement coverage and only 55.55% branch coverage.The following uncovered branches are now tested:
cleanupChainmatchPredicateoption path (custom line selector)getNetworkBridgeNamenull; docker error →nullgetDockerBridgeGatewaynull; invalid IPv4 format →null; docker error →nullisIp6tablesAvailabledisableIpv6ViaSysctlenableIpv6ViaSysctladdDnsRulesSecurity-Critical Paths Covered
addDnsRulesmust rollback any successfully-added rules if a later add fails, preventing inconsistent firewall state.disableIpv6ViaSysctl/enableIpv6ViaSysctllifecycle is now fully exercised, ensuring IPv6 can't be used as an unfiltered bypass path.getDockerBridgeGatewayinvalid-IP branch prevents unvalidated strings from being injected into iptables rules.isIp6tablesAvailablecache ensures consistent behavior throughout a single run.Test Notes
All tests follow existing patterns (mocked
execa,jest.clearAllMocks()inbeforeEach,testHelpers.resetIpv6State()to reset module-level state). No Docker, iptables, or network access required.