Skip to content

Conversation

@remo-lab
Copy link
Contributor

Need for Change:

This PR fixes a missing error check in a unit test.
In internal/fileutil/fileutil_test.go, the test case TestFileExists/file-with-content was calling os.WriteFile without checking its returned error. If the write failed (for example due to permission issues, disk errors, or a read-only filesystem), the test would continue assuming the file was written successfully. This could lead to misleading failures or false positives later in the test.
Other tests in the same file already validate os.WriteFile errors, so this change also improves consistency.

##Type of change:

  • Bug fix

##Changes Made:
File: internal/fileutil/fileutil_test.go
Line: 54
Wrapped os.WriteFile with require.NoError(t, ...) to ensure the test fails immediately if the write operation fails.

##Impact:
-Prevents silent failures in the test
-Produces clearer and more accurate test results
-Improves consistency with existing test patterns
-No functional or behavioral changes to production code

##Steps to Reproduce
-In internal/fileutil/fileutil_test.go (line 54), os.WriteFile is called without checking its error return.
-Run the test:
go test -v ./internal/fileutil -run TestFileExists/file-with-content
-Create a small test case (or environment) where the target directory is read-only or unwritable. In this scenario, os.WriteFile fails, but the test continues because the error is ignored.
After the fix, the test fails immediately when os.WriteFile returns an error.
Impact: Ignoring the error can cause tests to pass or fail for misleading reasons wh

@remo-lab remo-lab requested a review from a team as a code owner December 17, 2025 02:52
@remo-lab remo-lab force-pushed the fix/fileutil-test-check-writefile-error branch from 520f61b to 2b6665a Compare December 17, 2025 02:56
@remo-lab
Copy link
Contributor Author

remo-lab commented Dec 17, 2025

Hi @yacovm @denyeart 👋
This PR fixes a small issue in internal/fileutil/fileutil_test.go where os.WriteFile was called without checking its error return value.

If the write fails (e.g. due to permissions or disk issues), the test could continue silently and produce misleading results. The change simply adds proper error handling, consistent with other tests in the same file.

Whenever you have time, I’d really appreciate a review. Thanks!

Copy link
Contributor

@denyeart denyeart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, good catch!

@denyeart denyeart merged commit 850ed38 into hyperledger:main Dec 17, 2025
25 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants