proposal: testing/fstest: add MapFS.CopyFrom(fs.FS)

[myaaaaaaaaa]

Proposal Details

Since it was concluded that fs.FS is necessarily read-only, the only alternative is for every writable filesystem to implement its own form of os.CopyFS. This proposal is about doing so for MapFS.

For details, see:

• proposal: io/fs: add writable interfaces #45757 (comment)

[gabyhelp]

Related Issues and Documentation

• os: add CopyFS #62484 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[ianlancetaylor]

What is an example of a test that would want to use that method?

[myaaaaaaaaa]

I actually think that MapFS has utility outside of testing. For example:

	mapfs := fstest.MapFS{}
	mapfs.CopyFrom(os.DirFS("path/to/dir"))
	// manipulate mapfs without need for err != nil
	os.CopyFS("path/to/other/dir", mapfs)

Code that needs to deal with filesystems can generally benefit by consolidating all of their err != nil checks into the copy operation.

In the case of tests, here are some examples of code that would benefit from map's native integration with Go, compared to having to use the fs.FS interface:

//go:embed *
var embedFiles embed.FS

func TestHello(t *testing.T) {
	assert := assert.New(t)

	bucket := memblob.OpenBucket(nil)
	defer bucket.Close()
	// bucket operations...

	mapfs := fstest.MapFS{}
	mapfs.CopyFrom(bucket)
	mapfs.CopyFrom(os.DirFS("path/to/testing/tmpdir"))
	mapfs.CopyFrom(embedFiles)



	//// simple smoke test when filenames are non-deterministic
	// MapFS
	assert.Equal(3, len(mapfs))
	// fs.FS - more awkward and requires a must()
	assert.Equal(3, len(must(fs.Glob(bucket, "*"))))

	//// deterministic filenames
	// MapFS
	filenames := strings.Join(slices.Sorted(maps.Keys(mapfs)), " ")
	assert.Equal("a.txt b/b.txt c/c/c.json", filenames)
	// fs.FS - no shorthand for this due to the subdirectories
	fs.WalkDir(bucket, ".", func(path string, d fs.DirEntry, err error) error {
		// ...
		return nil
	})

	//// specific file
	// MapFS
	assert.NotNil(mapfs["a.txt"])
	// fs.FS - not exactly elegant, panics instead of failing gracefully
	must(fs.ReadFile(bucket, "a.txt"))

	//// nonexistent file
	// MapFS
	assert.Nil(mapfs["q.txt"])
	// fs.FS
	_, err := fs.ReadFile(bucket, "q.txt")
	assert.NotNil(err)
}

[timothy-king]

CopyFrom should likely return an error.

2cents: once an FS is loaded in a MapFS, it is fairly easy to edit the MapFS. Say there was a function From(fsys io.FS) (MapFS, error) that reads the FS and copies it into memory. Then CopyFrom is then just From followed by a maps.Copy. From seems like it could be a useful building block for tests.

[earthboundkid]

Say there was a function From(fsys io.FS) (MapFS, error) that reads the FS and copies it into memory. Then CopyFrom is then just From followed by a maps.Copy. From seems like it could be a useful building block for tests.

Yes, that strikes me as the correct API.

FWIW, I wrote txtar.From as part of #44158 but it got dropped from the final API change. It should probably also come back.