CNF-23051: Migrate away from deprecated ioutil#422
CNF-23051: Migrate away from deprecated ioutil#422sebrandon1 wants to merge 1 commit intoopenshift:masterfrom
Conversation
|
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
Bot
added
the
lifecycle/stale
|
/remove-lifecycle stale |
openshift-ci
Bot
removed
the
lifecycle/stale
|
/lgtm |
WalkthroughReplace deprecated Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
|
@sayan-biswas can you please approve? |
sebrandon1
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@sebrandon1: This pull request references CNF-23051 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
sebrandon1
force-pushed
the
ioutil_deprecation
branch
from
9a330d1 to
5abed52
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ingvagabund, sebrandon1 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/build/controller/build/build_controller_test.go`:
- Around line 1763-1766: The test calls os.CreateTemp and immediately defers
os.Remove(registriesConf.Name()) before checking err, which can panic if
CreateTemp failed; update the code in build_controller_test.go to check the
error returned by os.CreateTemp first (i.e., if err != nil { t.Errorf(...);
return } or t.Fatalf), only then use registriesConf and establish the defer
os.Remove(registriesConf.Name()), ensuring registriesConf is non-nil when
referenced.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 92e9659b-1985-4ded-91fd-e2c42ef2d67a
📒 Files selected for processing (3)
pkg/build/controller/build/build_controller_test.gopkg/image/controller/imagestream_controller_test.gopkg/template/controller/templateinstance_controller_test.go
| registriesConf, err := os.CreateTemp("", "registries.conf") | ||
| defer os.Remove(registriesConf.Name()) | ||
| if err != nil { | ||
| t.Errorf("unexpected error creating temp file: %s", err.Error()) |
There was a problem hiding this comment.
Guard CreateTemp error before using registriesConf.
At Line 1764, registriesConf.Name() is called before checking err. If os.CreateTemp fails, this can panic on nil and mask the real failure.
💡 Suggested fix
registriesConf, err := os.CreateTemp("", "registries.conf")
-defer os.Remove(registriesConf.Name())
if err != nil {
t.Errorf("unexpected error creating temp file: %s", err.Error())
+ return
}
+defer os.Remove(registriesConf.Name())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| registriesConf, err := os.CreateTemp("", "registries.conf") | |
| defer os.Remove(registriesConf.Name()) | |
| if err != nil { | |
| t.Errorf("unexpected error creating temp file: %s", err.Error()) | |
| registriesConf, err := os.CreateTemp("", "registries.conf") | |
| if err != nil { | |
| t.Errorf("unexpected error creating temp file: %s", err.Error()) | |
| return | |
| } | |
| defer os.Remove(registriesConf.Name()) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/build/controller/build/build_controller_test.go` around lines 1763 -
1766, The test calls os.CreateTemp and immediately defers
os.Remove(registriesConf.Name()) before checking err, which can panic if
CreateTemp failed; update the code in build_controller_test.go to check the
error returned by os.CreateTemp first (i.e., if err != nil { t.Errorf(...);
return } or t.Fatalf), only then use registriesConf and establish the defer
os.Remove(registriesConf.Name()), ensuring registriesConf is non-nil when
referenced.
|
/retest |
|
/lgtm |
|
/verified by CI |
openshift-ci-robot
added
the
verified
|
@ingvagabund: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@sebrandon1: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
ioutilhas been deprecated since Go 1.16: https://go.dev/doc/go1.16#ioutilTracking issue: redhat-best-practices-for-k8s/telco-bot#52
Migration away from
ioutil:ioutil.TempFilewithos.CreateTempfor creating temporary files inbuild_controller_test.go.ioutil.NopClosertoio.NopCloserandioutil.ReadAlltoio.ReadAllinimagestream_controller_test.goandtemplateinstance_controller_test.go. [1] [2] [3]Cleanup of imports:
ioutilimports and addediowhere necessary in test files. [1] [2] [3]Summary by CodeRabbit