-
Notifications
You must be signed in to change notification settings - Fork 1
chore: update modules #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
1e2f7bb to
35bfed0
Compare
8043614 to
e188e8b
Compare
Update tools and modules to latest versions, addressing dependabot alerts.
e188e8b to
9cd6283
Compare
Add go version and env debug for issue raising.
293ac91 to
8eceb77
Compare
Add a standalone test for the os.ReadDir issue.
8eceb77 to
dd233a7
Compare
04a90ed to
c7d884b
Compare
Update all module dependencies and go to the latest versions.
c7d884b to
f240a6b
Compare
Update GitHub Actions workflows to use the latest versions of actions to address issue with golang 1.24 compatibility.
|
Waiting on a release with golang/go#75157 in it, to fix the tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few suggestions on how the test cases can be updated to not run into issue golang/go#75157. It should work without needing to skip those tests.
pkg/gopro/osfs_test.go
Outdated
| require.ErrorIs(t, err, syscall.ENOTDIR) | ||
| require.Equal(t, []fs.DirEntry{}, dirs) | ||
| var expectedDirs []fs.DirEntry | ||
| require.Equal(t, expectedDirs, dirs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider this modified test case:
-require.Equal(t, []fs.DirEntry{}, dirs)
+require.Equal(t, 0, len(dirs))It still checks that the number of directories returned is as expected, zero, but it's not sensitive to whether the slice happens to be empty and non-nil, or nil.
The documentation at https://pkg.go.dev/io/fs#ReadDirFS.ReadDir states only that ReadDir "returns a list of directory entries". The documentation doesn't specify that a slice is nil or non-nil.
Since you're implementing your own interface, you could choose to add logic to detect when the slice has 0 elements and consistently make it a nil slice, but I suspect that behavior isn't going to be helpful to most callers that would at most range over the slice, and ranging over a nil slice behaves no different than ranging over an empty slice. Also see the Go style suggestion at https://go.dev/s/style#declaring-empty-slices:
When designing interfaces, avoid making a distinction between a nil slice and a non-nil, zero-length slice
pkg/gopro/osfs_test.go
Outdated
| dirs, err := os.ReadDir(name) | ||
| require.ErrorIs(t, err, syscall.ENOTDIR) | ||
| var expectedDirs []fs.DirEntry | ||
| require.Equal(t, expectedDirs, dirs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize this test is probably added in the context of issue golang/go#75157. Also noting here that https://pkg.go.dev/os#ReadDir doesn't specify whether the returned slice is nil or not. Its documented behavior only promises that "If an error occurs reading the directory, ReadDir returns the entries it was able to read before the error, along with the error."
Switch to using require.Empty for slice checks to avoid failure due to inconsistent behaviour of ReadDir between platforms
ee5dc4b to
125572c
Compare
Update modules to latest versions, addressing dependabot alerts.