mock all inventory commands in evalmgr tests#5615
Conversation
rucoder
requested review from
Copilot and
rene
and removed request for
eriknordmark
There was a problem hiding this comment.
Pull request overview
Updates evalmgr inventory-related tests to be hermetic by mocking all external inventory commands, avoiding dependencies on host-installed tooling.
Changes:
- Add empty mock scripts for system inventory commands (
lspci,lsusb,lsmod,dmidecode,dmesg) and prepend the temp mock dir toPATH. - Invoke mock command setup/cleanup from
inventory_test.go. - Expand the inventory test expected file list to include newly collected outputs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/pillar/cmd/evalmgr/test_helpers_test.go | Extends mock command setup to include system inventory commands and adjusts PATH during tests. |
| pkg/pillar/cmd/evalmgr/inventory_test.go | Uses the mock setup/cleanup and updates expected inventory files list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Prepend the temp directory to PATH so exec.Command finds our mocks first | ||
| savedPath = os.Getenv("PATH") | ||
| os.Setenv("PATH", tmpDir+":"+savedPath) | ||
|
|
||
| return tmpDir, nil | ||
| } | ||
|
|
||
| // cleanupMockCommands removes mock command scripts | ||
| // cleanupMockCommands removes mock command scripts and restores PATH | ||
| func cleanupMockCommands(tmpDir string) error { | ||
| if tmpDir == "" { | ||
| return nil | ||
| } | ||
| // Restore original PATH | ||
| if savedPath != "" { | ||
| os.Setenv("PATH", savedPath) | ||
| savedPath = "" | ||
| } |
There was a problem hiding this comment.
PATH restoration uses savedPath != "" as a sentinel. If the original PATH is legitimately empty, setupMockCommands sets PATH to tmpDir+":" but cleanupMockCommands will skip restoring it, leaving PATH polluted (and pointing at a removed temp dir). Track whether PATH was modified (e.g., separate bool) and restore unconditionally; also consider using os.PathListSeparator instead of a hard-coded ":" when prepending.
| // Setup mock commands so inventory collection produces empty files | ||
| tmpDir, err := setupMockCommands(fs) | ||
| if err != nil { | ||
| t.Fatalf("Failed to setup mock commands: %v", err) | ||
| } | ||
| t.Cleanup(func() { | ||
| cleanupMockCommands(tmpDir) | ||
| }) |
There was a problem hiding this comment.
This comment says inventory collection will produce empty files, but setupMockCommands only makes the system commands empty; iommu-groups.sh and spec.sh mocks still write output ("executed: ..."), and cmdline.txt is read from the host /proc/cmdline. Please reword the comment to reflect that the goal is to avoid relying on host tools/paths, not that all files are empty.
rucoder
force-pushed
the
rucoder/eval-test-fix
branch
from
964a840 to
7a10fb1
Compare
|
I see cmd/evalmgr tests are now passing. However, we got the following timeout: https://github.com/lf-edge/eve/actions/runs/22136656814/job/63990013573?pr=5615#step:6:4511 |
@zeljko-zededa and I discussed this last time I saw this issue (Dec 10th 2025) and we're setting all of our hope into the runner proxy ... |
|
@rene I'll push fixes for copilot comments and we can retry the whole test cycle |
setupMockCommands only mocked iommu-groups.sh and spec.sh but not the system commands (lspci, lsusb, lsmod, dmidecode, dmesg) used by CollectInventory. Create mock scripts for all commands in a single loop, prepend the temp directory to PATH so exec.Command finds them, and call setupMockCommands/cleanupMockCommands from inventory tests. Consolidate genScript and genEmptyScript into a single genMockScript helper. Add missing files to the expected files list in the test. Bump WaitForCondition timeout from 1s to 5s in runEvaluationCycle Phase 1 to avoid flaky failures when inventory collection is slow. Signed-off-by: Mikhail Malyshev <mike.malyshev@gmail.com>
rucoder
force-pushed
the
rucoder/eval-test-fix
branch
from
7a10fb1 to
3e96a1a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5615 +/- ##
==========================================
+ Coverage 19.52% 29.49% +9.96%
==========================================
Files 19 18 -1
Lines 3021 2417 -604
==========================================
+ Hits 590 713 +123
+ Misses 2310 1552 -758
- Partials 121 152 +31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@rene I' think we can merge |
Description
setupMockCommands only mocked iommu-groups.sh and spec.sh but not the system commands (lspci, lsusb, lsmod, dmidecode, dmesg) used by CollectInventory. Add empty mock scripts for these commands and prepend the temp directory to PATH so exec.Command finds them. Call setupMockCommands/cleanupMockCommands from inventory tests and add the missing files to the expected files list.
PR dependencies
None
How to test and validate this PR
go tests should pass
Changelog notes
None
PR Backports
No
Checklist
And the last but not least:
check them.
Please, check the boxes above after submitting the PR in interactive mode.