Repository: tetratelabs/wazero
Pull request #1233 (2023-03-13 18:30:19)

fix: --experimental-workdir test case on Windows
Interesting issue due to a lot of weird interactions between
path handling and system defaults.

The test will pass outside of CI (e.g. on my local machine),
because usually you will run this on C:\, which is the root
of your filesystem.

However, GitHub Actions mounts the test filesystem on D:,
causing the path handling routines to miss it.

You can simulate this even if you don't have a second drive:
assuming you have `wazero` under `%USERPROFILE%\Devel\wazero`

    subst X: %USERPROFILE%\Devel
    cd X:\wazero
    make test

However, we are ignoring path information because:

```go
func WithWorkdir(ctx context.Context, workdir string) context.Context {
	// Ensure if used on windows, the input path is translated to a POSIX one.
	workdir = workdir[len(filepath.VolumeName(workdir)):]
```

strips the Volume

The other weird bit happens in the test itself:

```
--mount=/:/ --experimental-workdir=$bearDir
```

What is happening here is that we are mount the root of our FS
to / (thereby exposing the entire FS to the test) and then
telling the guest to set the CWD to `$bearDir`.

However, on Windows we don't have a unique `/` (well technically
we do; see [UNC Paths][0] but that's another story) but each `/` has a volume attached.

So, on Windows we shouldn't `--mount=/:/` but rather one root volume
(in fact, the other tests work because they do `--mount=$bearDir:/`)
i.e. X:\ or, more generally `VolumeName($bearDir)`

Finally, although `WithWorkdir` is stripping the volume name,
we should probably pass a path without the volume name or at least ensuring
that that CWD makes sense together with the host mount directive; i.e.

when we pass `--mount=x:\:/` we should either pass `--workdir=\wazero\...`
or ensure that the volume name matches `--workdir=x:\wazero\...` and
return an error otherwise.

For now I am only fixing the test.

[0]: https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats

Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>



Patch:
diff --git a/internal/sysfs/dirfs.go b/internal/sysfs/dirfs.go
index a02d2c15..81281780 100644
--- a/internal/sysfs/dirfs.go
+++ b/internal/sysfs/dirfs.go
@@ -16,7 +16,7 @@ func NewDirFS(dir string) FS {
 }
 
 func ensureTrailingPathSeparator(dir string) string {
-	if dir[len(dir)-1] != os.PathSeparator {
+	if !os.IsPathSeparator(dir[len(dir)-1]) {
 		return dir + string(os.PathSeparator)
 	}
 	return dir
@@ -139,6 +139,9 @@ func (d *dirFS) Truncate(path string, size int64) error {
 func (d *dirFS) join(path string) string {
 	switch path {
 	case "", ".", "/":
+		if d.cleanedDir == "/" {
+			return "/"
+		}
 		// cleanedDir includes an unnecessary delimiter for the root path.
 		return d.cleanedDir[:len(d.cleanedDir)-1]
 	}


Test patch:
diff --git a/cmd/wazero/wazero_test.go b/cmd/wazero/wazero_test.go
index 43440f05..a55ba5d2 100644
--- a/cmd/wazero/wazero_test.go
+++ b/cmd/wazero/wazero_test.go
@@ -345,9 +345,13 @@ func TestRun(t *testing.T) {
 			expectedStdout: "pooh\n",
 		},
 		{
-			name:           "GOARCH=wasm GOOS=js workdir",
-			wasm:           wasmCatGo,
-			wazeroOpts:     []string{"--mount=/:/", fmt.Sprintf("--experimental-workdir=%s", bearDir)},
+			name: "GOARCH=wasm GOOS=js workdir",
+			wasm: wasmCatGo,
+			wazeroOpts: []string{
+				// --mount=X:\:/ on Windows, --mount=/:/ everywhere else
+				"--mount=" + filepath.VolumeName(bearDir) + string(os.PathSeparator) + ":/",
+				fmt.Sprintf("--experimental-workdir=%s", bearDir[len(filepath.VolumeName(bearDir)):]),
+			},
 			wasmArgs:       []string{"bear.txt"},
 			expectedStdout: "pooh\n",
 		},
@@ -486,10 +490,6 @@ func TestRun(t *testing.T) {
 	for _, tt := range append(tests, cryptoTest) {
 		tc := tt
 
-		if runtime.GOOS == "windows" && tc.name == "GOARCH=wasm GOOS=js workdir" {
-			continue // TODO: Adrian fix this before next RC
-		}
-
 		if tc.wasm == nil {
 			// We should only skip when the runtime is a scratch image.
 			require.False(t, platform.CompilerSupported())
diff --git a/internal/sysfs/dirfs_test.go b/internal/sysfs/dirfs_test.go
index 67019698..07576113 100644
--- a/internal/sysfs/dirfs_test.go
+++ b/internal/sysfs/dirfs_test.go
@@ -39,6 +39,20 @@ func TestNewDirFS(t *testing.T) {
 	})
 }
 
+func TestDirFS_join(t *testing.T) {
+	testFS := NewDirFS("/").(*dirFS)
+	require.Equal(t, "/", testFS.join(""))
+	require.Equal(t, "/", testFS.join("."))
+	require.Equal(t, "/", testFS.join("/"))
+	require.Equal(t, "/tmp", testFS.join("tmp"))
+
+	testFS = NewDirFS(".").(*dirFS)
+	require.Equal(t, ".", testFS.join(""))
+	require.Equal(t, ".", testFS.join("."))
+	require.Equal(t, ".", testFS.join("/"))
+	require.Equal(t, "."+string(os.PathSeparator)+"tmp", testFS.join("tmp"))
+}
+
 func TestDirFS_String(t *testing.T) {
 	testFS := NewDirFS(".")