Repository: moby/moby
File: daemon/volume/mounts/linux_parser_test.go (Go)

Code before:
					Type:     mount.TypeVolume,
				},
			},
		},
		{
			mount: mount.Mount{
				Source: "/invalid/source/path",
				Target: "/data/anonymous-read-only-volume",
				Type:   mount.TypeBind,
			},
			expErr: `invalid mount config for type "bind": bind source path does not exist: /invalid/source/path`,
		},
		{
			mount: mount.Mount{
				Target: "/data/invalid-type",
				Type:   "invalid",
			},
			expErr: `invalid mount config for type "invalid": mount type unknown`,
		},
	}

	parser := NewLinuxParser()

	for _, tc := range tests {
		m, err := parser.ParseMountSpec(tc.mount)
		if tc.expErr != "" {
			assert.Check(t, is.Nil(m))
			assert.Check(t, is.Error(err, tc.expErr))
			continue
		}

		assert.NilError(t, err)
		assert.Check(t, is.DeepEqual(*m, *tc.expected, cmpopts.IgnoreUnexported(MountPoint{})))
	}
}

// TestLinuxParseMountSpecBindWithFileinfoError makes sure that the parser returns
// the error produced by the fileinfo provider.
//
// Some extra context for the future in case of changes and possible wtf are we
// testing this for:
//
// Currently this "fileInfoProvider" returns (bool, bool, error)
// The 1st bool is "does this path exist"
// The 2nd bool is "is this path a dir"
// Then of course the error is an error.
//
// The issue is the parser was ignoring the error and only looking at the
// "does this path exist" boolean, which is always false if there is an error.
// Then the error returned to the caller was a (slightly, maybe) friendlier
// error string than what comes from `os.Stat`

Review comment (suggestion):
If you're updating; perhaps would make sense to use subtests here; adding a `doc` field to the test-table to describe the test.


Very quick write-up I made locally (including changes I mentioned above); feel free to copy/paste if useful;


```go
func TestLinuxValidateMounts(t *testing.T) {
	tmpDir := t.TempDir()

	tests := []struct {
		doc      string
		mount    mount.Mount
		expected *MountPoint
		expErr   string
	}{
		{
			doc: "read-write",
			mount: mount.Mount{
				Source: tmpDir,
				Target: "/tmp1",
				Type:   mount.TypeBind,
			},
			expected: &MountPoint{
				Source:      tmpDir,
				Destination: "/tmp1",
				RW:          true,
				Type:        mount.TypeBind,
				Propagation: "rprivate",
				Spec: mount.Mount{
					Source: tmpDir,
					Target: "/tmp1",
					Type:   mount.TypeBind,
				},
			},
		},
		{
			doc: "read-only",
			mount: mount.Mount{
				Target:   "/data/anonymous-read-only-volume",
				ReadOnly: true,
				Type:     mount.TypeVolume,
			},
			expected: &MountPoint{
				Destination: "/data/anonymous-read-only-volume",
				Type:        mount.TypeVolume,
				CopyData:    true,
				Spec: mount.Mount{
					Target:   "/data/anonymous-read-only-volume",
					ReadOnly: true,
					Type:     mount.TypeVolume,
				},
			},
		},
		{
			doc: "invalid source path",
			mount: mount.Mount{
				Source: filepath.Join(tmpDir, "no-such-path"),
				Target: "/data/anonymous-read-only-volume",
				Type:   mount.TypeBind,
			},
			expErr: `invalid mount config for type "bind": bind source path does not exist: ` + filepath.Join(tmpDir, "no-such-path"),
		},
		{
			mount: mount.Mount{
				Target: "/data/invalid-type",
				Type:   "invalid",
			},
			expErr: `invalid mount config for type "invalid": mount type unknown`,
		},
	}

	parser := NewLinuxParser()

	for _, tc := range tests {
		t.Run(tc.doc, func(t *testing.T) {
			m, err := parser.ParseMountSpec(tc.mount)
			if tc.expErr != "" {
				assert.Check(t, is.Nil(m))
				assert.Check(t, is.Error(err, tc.expErr))
				return
			}

			assert.NilError(t, err)
			assert.Check(t, is.DeepEqual(*m, *tc.expected, cmpopts.IgnoreUnexported(MountPoint{})))
		})
	}
}
```



Code after:
				Type:     mount.TypeVolume,
			},
			expected: &MountPoint{
				Destination: "/data/anonymous-read-only-volume",
				Type:        mount.TypeVolume,
				CopyData:    true,
				Spec: mount.Mount{
					Target:   "/data/anonymous-read-only-volume",
					ReadOnly: true,
					Type:     mount.TypeVolume,
				},
			},
		},
		{
			doc: "invalid source path",
			mount: mount.Mount{
				Source: "/tmp/definitely-no-such-path",
				Target: "/data/anonymous-read-only-volume",
				Type:   mount.TypeBind,
			},
			expErr: `invalid mount config for type "bind": bind source path does not exist: /tmp/definitely-no-such-path`,
		},
		{
			doc: "invalid mount type",
			mount: mount.Mount{
				Target: "/data/invalid-type",
				Type:   "invalid",
			},
			expErr: `invalid mount config for type "invalid": mount type unknown`,
		},
	}

	parser := NewLinuxParser()

	for _, tc := range tests {
		t.Run(tc.doc, func(t *testing.T) {
			if tc.skipWindows && runtime.GOOS == "windows" {
				t.Skip("skip on Windows")
			}
			m, err := parser.ParseMountSpec(tc.mount)
			if tc.expErr != "" {
				assert.Check(t, is.Nil(m))
				assert.Check(t, is.Error(err, tc.expErr))
				return
			}

			assert.NilError(t, err)
			assert.Check(t, is.DeepEqual(*m, *tc.expected, cmpopts.IgnoreUnexported(MountPoint{})))
		})
	}
}