pillar: add unit tests for NIM-related packages by eriknordmark · Pull Request #5901 · lf-edge/eve

eriknordmark · 2026-05-05T14:45:42Z

Description

Commits 1–3 and 5 are test-only. The only production code change is commit 4
(c1dd0c9), which is a mechanical refactor to enable testing.

Add Go unit tests for NIM-related packages in pkg/pillar that previously
had no or minimal test coverage. Five commits, covering five packages:

Commit 1 (487661e) — dpcreconciler wpa/bond/vlan items:

genericitems/wpa_supplicant_test.go: WpaSupplicant metadata, renderWifiConfig, renderPNACConfig, path helpers, WifiConfig/PNAC8021XConfig helpers
linuxitems/bond_vlan_test.go: Bond, Vlan, isFailoverBondMode, GetMTU, NeedsRecreate, Dependencies

Commit 2 (70a6a51) — remaining dpcreconciler and netmonitor items:

genericitems/items_test.go: AdapterAddrs, Dhcpcd, NetIO, ResolvConf, SSHAuthKeys, Wwan
linuxitems/items_test.go: Adapter, Arp, IPRule, PhysIf, RFKill, Route
netmonitor/netmonitor_test.go: IsDefaultRoute, IfAttrs.Equal, IfChange.Equal, event marker methods

Commit 3 (573e688) — nireconciler item types:

nireconciler/genericitems/items_test.go: Dnsmasq, HTTPServer, IPReserve, Port, Radvd plus helpers (equalUpstreamDNSServer, equalMACToIP, EqualIPRoutes, isECONNREFUSED, path helpers, dhcpTagForHost)
nireconciler/linuxitems/items_test.go: BPDUGuard, BridgeFwdMask, Bridge, BridgePort, DummyIf, IPRule, IPSet, Sysctl, TCIngress, TCMirror, VIF, VLANBridge, VLANPort, VLANSubIf, Route plus helpers (toMaskValue, equalBoolPtr, boolPtrToString, ipVersionStr, hasDefaultDst)

Commit 4 (c1dd0c9) — netmonitor proc parser refactor (minimal production change):

Extract hardcoded /proc/net/bonding path into a procNetBondingDir constant and add it as a parameter to parseProcBondInfo and parseProcBondMemberMetrics, making them unit-testable without /proc.

Commit 5 (075f5f5) — netmonitor proc bond parser tests:

netmonitor/netmonitor_test.go: parseChurnState, isDhcpcdNotRunningErr, parseProcBondInfo, parseProcBondMemberMetrics using synthetic temp-dir bond files.

Statement coverage improvements:

dpcreconciler/genericitems: 7.1% → ~30%
dpcreconciler/linuxitems: 0.0% → ~20%
netmonitor: 0% → ~15%
nireconciler/genericitems: 16.7% → 32.9%
nireconciler/linuxitems: 0.0% → 26.6%

All new unit tests have no OS or hardware dependencies.

PR dependencies

None.

How to test and validate this PR

cd pkg/pillar
go test ./dpcreconciler/genericitems/
go test ./dpcreconciler/linuxitems/
go test ./netmonitor/
go test ./nireconciler/genericitems/
go test ./nireconciler/linuxitems/

All packages pass all tests.

Changelog notes

No user-facing changes.

PR Backports

16.0-stable: No, test-only change.
14.5-stable: No.
13.4-stable: No.

Checklist

I've provided a proper description
I've added the proper documentation
I've tested my PR on amd64 device
I've tested my PR on arm64 device
I've written the test verification instructions
I've set the proper labels to this PR
I've checked the boxes above, or I've provided a good reason why I didn't check them.

milan-zededa · 2026-05-05T14:57:26Z

+			{SSID: "HomeNet", KeyScheme: types.KeySchemeWpaPsk},
+		},
+	}
+	s := wifi.String()


The String() method is really only used for logging purposes.
Not sure if unit test is really valuable here (aside from increasing code coverage).
Every time we change some of these non-crucial methods (e.g. to change the logging output), we will have to fix potentially multiple unit tests.

milan-zededa · 2026-05-05T14:59:44Z

+	}
+	ethAdapter2 := mockAdapter{ifName: "eth0", wType: types.WirelessTypeNone}
+	wifiAdapter2 := mockAdapter{ifName: "eth0", wType: types.WirelessTypeWifi}
+	if !pnacDeps[0].MustSatisfy(ethAdapter2) {


So even if we just change the order of dependencies (which does not matter), we will have to fix these slice indices in unit tests..

milan-zededa · 2026-05-05T15:44:27Z

FWIW, I have prepared proper bond and vlan integration tests using my new test framework, which I will present next week.
They should provide pretty good coverage, so not sure if we need these unit tests just to "patch the coverage".

milan-zededa · 2026-05-05T15:48:41Z

These coverage-focused AI-generated tests remind of the infamous config properties count-check: https://github.com/lf-edge/eve/blob/master/pkg/pillar/types/global_test.go#L146-L262
Does it help to capture any regressions? No
Does it annoy developers? Yes

eriknordmark · 2026-05-05T18:17:53Z

FWIW, I have prepared proper bond and vlan integration tests using my new test framework, which I will present next week. They should provide pretty good coverage, so not sure if we need these unit tests just to "patch the coverage".

That was actually going to be my question to you, but you answered it before I could ask it.

FWIW I have another thing to explore which is to produce eden tests which drive nim, including the startup code and handling of /config/ input. But if I do this to drive coverage it might not be that useful - checking for the semantics is more important. Working on that a bit for zedagent.

codecov · 2026-05-05T19:30:12Z

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.90%. Comparing base (2281599) to head (da1b2c1).
⚠️ Report is 644 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/pillar/netmonitor/linux.go	66.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5901      +/-   ##
==========================================
- Coverage   19.52%   18.90%   -0.63%     
==========================================
  Files          19      474     +455     
  Lines        3021    85692   +82671     
==========================================
+ Hits          590    16196   +15606     
- Misses       2310    67955   +65645     
- Partials      121     1541    +1420

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Update pkg/pillar/docs/nim.md to document subscriptions, publications and behaviours added to NIM since the original write-up: the lps DPC source, EdgeNodeClusterStatus, KubeUserServices, VaultStatus and EnrolledCertificate gating of 802.1x, the resolver-cache goroutine, and the PNAC and bond metric publications. Add a startup-matrix discussion of how lastconfig, bootstrap-config.pb and /config/DevicePortConfig/*.json interact. Add pkg/pillar/docs/nim-eden-test-plan.md which catalogs existing unit and eden coverage (including PR lf-edge#5901), enumerates uncovered code paths in cmd/nim and supporting packages, and proposes a phased set of eden tests that exercise startup ingestion, DPC verification and fallback against real SDN-induced connectivity loss, persistence across reboot, LPS overrides, the resolver cache, PNAC/bond metrics, WLAN with cipher decryption, cluster static IP, KubeUserServices ACLs, LOC probing and flowlog reconciliation. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Add pkg/pillar/docs/nim-eden-test-plan.md which catalogs existing unit and eden coverage (including PR lf-edge#5901), enumerates uncovered code paths in cmd/nim and supporting packages, and proposes a phased set of eden tests that exercise startup ingestion, DPC verification and fallback against real SDN-induced connectivity loss, persistence across reboot, LPS overrides, the resolver cache, PNAC/bond metrics, WLAN with cipher decryption, cluster static IP, KubeUserServices ACLs, LOC probing and flowlog reconciliation. Note in the document that Phase 1 of this plan is being implemented in lf-edge/eden#1165 (open, draft); script names there may differ from the names suggested here, so per-test entries should be revisited once that PR merges. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Add Go unit tests for three dpcreconciler item types that previously had no coverage: - genericitems/wpa_supplicant_test.go: tests for WpaSupplicant metadata methods, renderWifiConfig, renderPNACConfig, and all WifiConfig/PNAC8021XConfig helpers. Covers 12 new functions. - linuxitems/bond_vlan_test.go: tests for Bond and Vlan metadata methods, Equal, GetMTU, NeedsRecreate, Dependencies, and the isFailoverBondMode helper. Covers 10 new functions. Statement coverage improvements: genericitems 7.1%→16.9% (+9.8pp), linuxitems 0.0%→6.5% (+6.5pp). Also add `nd` to .codespellrc ignore-words-list. The netmonitor linux.go uses `nd` as the dhcpcd neighbor-discovery key prefix (e.g. nd0_prefix_information0_prefix); codespell otherwise flags it as "and" or "2nd". Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Add Go unit tests for six genericitems types (AdapterAddrs, Dhcpcd metadata, NetIO, ResolvConf, SSHAuthKeys, Wwan), six linuxitems types (Adapter, Arp, IPRule, PhysIf, RFKill, Route), and netmonitor pure-logic methods (Route.IsDefaultRoute, IfAttrs.Equal, IfChange.Equal, isNetworkEvent markers). Coverage improvements: genericitems 16.9% -> 28.2% (+11.3 pp) linuxitems 6.5% -> 24.5% (+18.0 pp) netmonitor 6.2% -> 7.0% (+0.8 pp) All tests are pure Go with no OS or hardware dependencies. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Add white-box unit tests for all depgraph Item types in nireconciler/genericitems (Dnsmasq, HTTPServer, IPReserve, Port, Radvd) and nireconciler/linuxitems (BPDUGuard, BridgeFwdMask, Bridge, BridgePort, DummyIf, IPRule, IPSet, Route, Sysctl, TCIngress, TCMirror, VIF, VLANBridge, VLANPort, VLANSubIf). Coverage: genericitems 16.7%→32.9%, linuxitems 0%→26.6%. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Allow parseProcBondInfo and parseProcBondMemberMetrics to accept a bondingDir parameter instead of hardcoding /proc/net/bonding, enabling unit tests to supply a temp directory with synthetic bond files. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Add unit tests for parseChurnState, isDhcpcdNotRunningErr, parseProcBondInfo, and parseProcBondMemberMetrics. The first two are pure functions; the latter two write synthetic /proc/net/bonding files to t.TempDir() and pass the temp directory via the newly injected bondingDir parameter. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-actions Bot requested a review from milan-zededa May 5, 2026 14:46

milan-zededa reviewed May 5, 2026

View reviewed changes

github-actions Bot requested a review from milan-zededa May 5, 2026 15:38

eriknordmark changed the title ~~pillar: add unit tests for wpa, bond, vlan items~~ pillar: add unit tests for NIM-related packages May 5, 2026

eriknordmark and others added 5 commits May 10, 2026 10:41

eriknordmark force-pushed the checkpoint branch from da1b2c1 to 7236bb0 Compare May 10, 2026 08:42

eriknordmark mentioned this pull request May 10, 2026

Onboarded EVE never POSTs EdgeNodeCerts; attestation never completes (regression on master, ~2026-05-06) #5936

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pillar: add unit tests for NIM-related packages#5901

pillar: add unit tests for NIM-related packages#5901
eriknordmark wants to merge 5 commits intolf-edge:masterfrom
eriknordmark:checkpoint

eriknordmark commented May 5, 2026 •

edited

Loading

Uh oh!

milan-zededa May 5, 2026 •

edited

Loading

Uh oh!

milan-zededa May 5, 2026

Uh oh!

milan-zededa commented May 5, 2026 •

edited

Loading

Uh oh!

milan-zededa commented May 5, 2026

Uh oh!

eriknordmark commented May 5, 2026

Uh oh!

codecov Bot commented May 5, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

eriknordmark commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

PR dependencies

How to test and validate this PR

Changelog notes

PR Backports

Checklist

Uh oh!

milan-zededa May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

milan-zededa May 5, 2026

Choose a reason for hiding this comment

Uh oh!

milan-zededa commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

milan-zededa commented May 5, 2026

Uh oh!

eriknordmark commented May 5, 2026

Uh oh!

codecov Bot commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

eriknordmark commented May 5, 2026 •

edited

Loading

milan-zededa May 5, 2026 •

edited

Loading

milan-zededa commented May 5, 2026 •

edited

Loading

codecov Bot commented May 5, 2026 •

edited

Loading