testing/fstest: TestFS fails on valid FS if it contains any symlinks

[caarlos0]

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

$ go version
go version devel +8869086d8f Thu Feb 4 04:46:49 2021 +0000 darwin/amd64

Also tried with go 1.16 rc1

Does this issue reproduce with the latest release?

Yes.

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

go env Output

$ goGO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/carlos/Library/Caches/go-build"
GOENV="/Users/carlos/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/carlos/Developer/Go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/carlos/Developer/Go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/carlos/Developer/forks/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/carlos/Developer/forks/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="devel +8869086d8f Thu Feb 4 04:46:49 2021 +0000"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/carlos/Developer/forks/go/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/cz/k46fpqfd547_mn0ssjq7b5rr0000gn/T/go-build4008414779=/tmp/go-build -gno-record-gcc-switches -fno-common" env

What did you do?

Example reproducible:

package main

import (
	"os"
	"path/filepath"
	"testing"
	"testing/fstest"
)

func TestTestFS(t *testing.T) {
	tmp := t.TempDir()
	tmpfs := os.DirFS(tmp)

	if err := os.WriteFile(filepath.Join(tmp, "hello"), []byte("hello, world\n"), 0644); err != nil {
		t.Fatal(err)
	}

	if err := os.Symlink(filepath.Join(tmp, "hello"), filepath.Join(tmp, "hello.link")); err != nil {
		t.Fatal(err)
	}

	if err := fstest.TestFS(tmpfs, "hello", "hello.link"); err != nil {
		t.Fatal(err)
	}
}

Seems like it compares the stat of the file vs the stat of the link to the file... which fails (as it should).

Maybe I shouldn't use it against anything but MapFS? I'm not sure.

What did you expect to see?

Test should pass.

What did you see instead?

        hello.link: mismatch:
        	entry = hello.link IsDir=false Type=L---------
        	file.Stat() = hello.link IsDir=false Type=----------
        hello.link: mismatch:
        	entry.Info() = hello.link IsDir=false Mode=Lrwxr-xr-x Size=79 ModTime=2021-02-05 00:46:02.12900541 -0300 -03
        	file.Stat() = hello.link IsDir=false Mode=-rw-r--r-- Size=13 ModTime=2021-02-05 00:46:02.128923494 -0300 -03

[robpike]

A symlink is not a file in the sense of the FS interface, so a decision must be made about what a symlink's behavior should be. One can suggest that the symlink should evaluated, and that the resulting object be referenced, but one can also argue that a symlink is an error because it does not honor the simple FS semantics. The second case is what we see here.

I am not sure what the right answer is, but whatever it is, it should be documented. There is no documentation today and that should be addressed once the decision is made.

[caarlos0]

Understood.

For context, I got this while looking into replacing afero.Fs for fs.FS in one the packages I maintain.

The main usage of afero.Fs there was to avoid creating thousands of files in testdata to validate our behavior. So "in production" we use the real FS implementation and when running tests a MemFS (although we could use a real FS on a temp dir as well).

Maybe that's not a use case fs.FS was intent for, but then I agree it should be documented.
And would also be a bit sad because it would be really nice to not use 3rd parties for that...

[bcmills]

one can also argue that a symlink is an error because it does not honor the simple FS semantics.

I don't think it makes sense for os.DirFS to lie about the types of directory entries or filter out symlink entries, especially given that nothing fundamentally prevents io/fs from gaining an LstatFS extension interface in the future.

For now, I think fstest.TestFS should skip the Stat consistency check for any entry that has a type with ModeSymlink set.

[rsc]

I agree about skipping the Stat consistency check if ModeSymlink is set in the directory listing.
Mailed https://golang.org/cl/290009.

[gopherbot]

Change https://golang.org/cl/290009 mentions this issue: testing/fstest: avoid symlink-induced failures in tester