Skip to content
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

io/fs: should fs.Sub(fsys, "no such directory") return a file system that fails fstest.TestFS? #45087

Open
dchapes opened this issue Mar 17, 2021 · 2 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@dchapes
Copy link

dchapes commented Mar 17, 2021

What version of Go are you using (go version)?

$ go version
go version go1.16.2 freebsd/amd64

Does this issue reproduce with the latest release?

If you mean latest released Go, then yes.
If you mean tip, I'm not sure, but I expect so.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
[output trimmed]
GO111MODULE=""
GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="freebsd"
GOOS="freebsd"
GOVERSION="go1.16.2"
CGO_ENABLED="1"

What did you do?

    fs, err := fs.Sub(someFS, "noSuchDir")
    err = fstest.TestFS(fs)

https://play.golang.org/p/eXsqSUYHWQs

What did you expect to see?

No error returned.

What did you see instead?

It fails due to lack of an empty root directory:

    prog.go:18: TestFS found errors:
        .: Open: open .: file does not exist

This may very well be a "works as intended" issue, and if so I apologise for the noise.

FYI: I ran into this while experimenting with implementing the opposite of fs.Sub, a Prefix(fs.FS, string) (fs.FS, error).
I provided a prefixFS.Sub as an optimisation where it returns another prefixFS with the prefix adjusted or calls fs.Sub on it's underlying file system to avoid chaining multiple wrappers. I ended up also implementing an EmptyFS that I return when the sub-dir isn't within my prefix in order to keep fstest.TestFS happy.

I imagine others implementing fs.SubFS and trying to test their code would appreciate not having to make their own EmptyFS just to satisfy fstest.TestFS. Perhaps an empty file system implementation could be provided or fstest could be tweaked with respect to what it accepts as a completely emtpy file system.

@cherrymui cherrymui changed the title Should fs.Sub(fsys, "no such directory") return a file system that fails fstest.TestFS? io/fs: should fs.Sub(fsys, "no such directory") return a file system that fails fstest.TestFS? Mar 17, 2021
@cherrymui
Copy link
Member

cc @rsc
It may be intended. It would be good if someone could confirm.

@dchapes I think you can implement EmptyFS with fstest.MapFS with an empty map.

@cherrymui cherrymui added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 17, 2021
@cherrymui cherrymui added this to the Backlog milestone Mar 17, 2021
@dchapes
Copy link
Author

dchapes commented Mar 17, 2021

@cherrymui

@dchapes I think you can implement EmptyFS with fstest.MapFS with an empty map.

D'oh, you are indeed correct. Thanks for your comment!

I just verified that var fsys fstest.MapFS does indeed work correctly with fstest.TestFS as it implements a "." directory (as does var fsys embed.FS BTW). For some reason I had thought it wouldn't do that without even checking it. Oh well, I was just doing this all as an exercise with how the io/fs interfaces work in practice so my short time doing an EmptyFS wasn't wasted. If an empty file system turns out useful in practice I'd probably prefer one that didn't depend on somthing under testing/… anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

2 participants