main: use friendly filename in stdin read error - #728
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change adds ChangesInput reading
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/main_test.go`:
- Around line 23-88: The readInput tests should be consolidated into a
table-driven TestReadInput with subtests covering stdin, file input, missing
files, and an empty-stdin edge case. Reuse the existing setup and assertions for
each case, preserving expected data, filename, and error behavior while keeping
the tests in internal/main_test.go.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d23ec70-8fd8-4776-82c3-1751726923de
📒 Files selected for processing (2)
internal/main.gointernal/main_test.go
The io.ReadAll failure message used infile.Name(), which returns /dev/stdin on Linux instead of the user-facing <stdin> label already used everywhere else in main(). Extract the input reading into readInput so the friendly filename (<stdin> for stdin, the real path otherwise) is used consistently in both the read error and the translation report. Fixes: coreos#726 Signed-off-by: Deepak Bhagat <deepak988088@gmail.com>
bc8ce88 to
02b0e94
Compare
|
Closing in favor of coreos/ignition#2293. Butane is merging into Ignition (see coreos/ignition#2281), so the fix for the stdin read error now lives there. ignition#2293 ports this exact change, including the readInput helper and the main_test.go table tests, so this Butane PR is no longer needed. |
Summary
In
internal/main.go, theio.ReadAllfailure path formatted its error withinfile.Name(), which returns/dev/stdinon Linux instead of the friendly<stdin>label used everywhere else in the function (and in theTranslateBytesreport path).Changes
Extracted the input-reading logic into a
readInput(input string) ([]byte, string, error)helper. It returns the friendly filename (<stdin>when reading from stdin, the real path when--input/a positional file is given) so the read error message and the translation report stay consistent.internal/main.goline 127 (now inreadInput):filenameis now<stdin>for the stdin case instead of/dev/stdin.Verification
Added
internal/main_test.gocoveringreadInput:<stdin>filename./testpasses (gofmt, go vet, full test suite, doc validation).Before:
failed to read /dev/stdin: <error>After:
failed to read <stdin>: <error>Fixes: #726
Summary by CodeRabbit
Bug Fixes
Tests