Merge pull request #48776 from thaJeztah/mount_improve_tests

volume/mounts: minor linting issues, touch-ups, and improve test-coverage

volume/mounts/lcow_parser_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/api/types/mount"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 )
@@ -85,16 +86,16 @@ func TestLCOWParseMountRaw(t *testing.T) {
 
 	for _, path := range valid {
 		if _, err := parser.ParseMountRaw(path, "local"); err != nil {
-			t.Errorf("ParseMountRaw(`%q`) should succeed: error %q", path, err)
+			t.Errorf("ParseMountRaw(%q) should succeed: error %q", path, err)
 		}
 	}
 
 	for path, expectedError := range invalid {
 		if mp, err := parser.ParseMountRaw(path, "local"); err == nil {
-			t.Errorf("ParseMountRaw(`%q`) should have failed validation. Err '%v' - MP: %v", path, err, mp)
+			t.Errorf("ParseMountRaw(%q) should have failed validation. Err '%v' - MP: %v", path, err, mp)
 		} else {
 			if !strings.Contains(err.Error(), expectedError) {
-				t.Errorf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error())
+				t.Errorf("ParseMountRaw(%q) error should contain %q, got %v", path, expectedError, err.Error())
 			}
 		}
 	}
@@ -102,107 +103,151 @@ func TestLCOWParseMountRaw(t *testing.T) {
 
 func TestLCOWParseMountRawSplit(t *testing.T) {
 	cases := []struct {
-		bind      string
-		driver    string
-		expType   mount.Type
-		expDest   string
-		expSource string
-		expName   string
-		expDriver string
-		expRW     bool
-		fail      bool
+		bind     string
+		driver   string
+		expected *MountPoint
+		expErr   string
 	}{
 		{
-			bind:      `c:\:/foo`,
-			driver:    "local",
-			expType:   mount.TypeBind,
-			expDest:   `/foo`,
-			expSource: `c:\`,
-			expRW:     true,
+			bind:   `c:\:/foo`,
+			driver: "local",
+			expected: &MountPoint{
+				Source:      `c:\`,
+				Destination: "/foo",
+				RW:          true,
+				Type:        mount.TypeBind,
+				Propagation: "", // Propagation is not set on LCOW.
+				Spec: mount.Mount{
+					Source:   `c:\`,
+					Target:   "/foo",
+					ReadOnly: false,
+					Type:     mount.TypeBind,
+				},
+			},
 		},
 		{
-			bind:      `c:\:/foo:ro`,
-			driver:    "local",
-			expType:   mount.TypeBind,
-			expDest:   `/foo`,
-			expSource: `c:\`,
+			bind:   `c:\:/foo:ro`,
+			driver: "local",
+			expected: &MountPoint{
+				Source:      `c:\`,
+				Destination: "/foo",
+				RW:          false,
+				Type:        mount.TypeBind,
+				Mode:        "ro",
+				Propagation: "", // Propagation is not set on LCOW.
+				Spec: mount.Mount{
+					Source:   `c:\`,
+					Target:   "/foo",
+					ReadOnly: true,
+					Type:     mount.TypeBind,
+				},
+			},
 		},
 		{
-			bind:      `c:\:/foo:rw`,
-			driver:    "local",
-			expType:   mount.TypeBind,
-			expDest:   `/foo`,
-			expSource: `c:\`,
-			expRW:     true,
+			bind:   `c:\:/foo:rw`,
+			driver: "local",
+			expected: &MountPoint{
+				Source:      `c:\`,
+				Destination: "/foo",
+				RW:          true,
+				Type:        mount.TypeBind,
+				Mode:        "rw",
+				Propagation: "", // Propagation is not set on LCOW.
+				Spec: mount.Mount{
+					Source:   `c:\`,
+					Target:   "/foo",
+					ReadOnly: false,
+					Type:     mount.TypeBind,
+				},
+			},
 		},
 		{
-			bind:      `c:\:/foo:foo`,
-			driver:    "local",
-			expType:   mount.TypeBind,
-			expDest:   `/foo`,
-			expSource: `c:\`,
-			fail:      true,
+			bind:   `c:\:/foo:foo`,
+			driver: "local",
+			expErr: `invalid volume specification: 'c:\:/foo:foo'`,
 		},
 		{
-			bind:      `name:/foo:rw`,
-			driver:    "local",
-			expType:   mount.TypeVolume,
-			expDest:   `/foo`,
-			expName:   `name`,
-			expDriver: "local",
-			expRW:     true,
+			bind:   `name:/foo:rw`,
+			driver: "local",
+			expected: &MountPoint{
+				Destination: "/foo",
+				RW:          true,
+				Name:        "name",
+				Driver:      "local",
+				Type:        mount.TypeVolume,
+				Mode:        "rw",
+				Propagation: "", // Propagation is not set on LCOW.
+				Spec: mount.Mount{
+					Source:        `name`,
+					Target:        "/foo",
+					ReadOnly:      false,
+					Type:          mount.TypeVolume,
+					VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}},
+				},
+			},
 		},
 		{
-			bind:      `name:/foo`,
-			driver:    "local",
-			expType:   mount.TypeVolume,
-			expDest:   `/foo`,
-			expName:   `name`,
-			expDriver: "local",
-			expRW:     true,
+			bind:   `name:/foo`,
+			driver: "local",
+			expected: &MountPoint{
+				Destination: "/foo",
+				RW:          true,
+				Name:        "name",
+				Driver:      "local",
+				Type:        mount.TypeVolume,
+				Mode:        "", // FIXME(thaJeztah): why is this different than an explicit "rw" ?
+				Propagation: "", // Propagation is not set on LCOW.
+				Spec: mount.Mount{
+					Source:        `name`,
+					Target:        "/foo",
+					ReadOnly:      false,
+					Type:          mount.TypeVolume,
+					VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}},
+				},
+			},
 		},
 		{
-			bind:      `name:/foo:ro`,
-			driver:    "local",
-			expType:   mount.TypeVolume,
-			expDest:   `/foo`,
-			expName:   `name`,
-			expDriver: "local",
+			bind:   `name:/foo:ro`,
+			driver: "local",
+			expected: &MountPoint{
+				Destination: "/foo",
+				RW:          false,
+				Name:        "name",
+				Driver:      "local",
+				Type:        mount.TypeVolume,
+				Mode:        "ro",
+				Propagation: "", // Propagation is not set on LCOW.
+				Spec: mount.Mount{
+					Source:        `name`,
+					Target:        "/foo",
+					ReadOnly:      true,
+					Type:          mount.TypeVolume,
+					VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}},
+				},
+			},
 		},
 		{
-			bind:    `name:/`,
-			expType: mount.TypeVolume,
-			expRW:   true,
-			fail:    true,
+			bind:   `name:/`,
+			expErr: `invalid volume specification: 'name:/': invalid mount config for type "volume": invalid specification: destination can't be '/'`,
 		},
 		{
-			bind:    `driver/name:/`,
-			expType: mount.TypeVolume,
-			expRW:   true,
-			fail:    true,
+			bind:   `driver/name:/`,
+			expErr: `invalid volume specification: 'driver/name:/'`,
 		},
 		{
-			bind:      `\\.\pipe\foo:\\.\pipe\bar`,
-			driver:    "local",
-			expType:   mount.TypeNamedPipe,
-			expDest:   `\\.\pipe\bar`,
-			expSource: `\\.\pipe\foo`,
-			expRW:     true,
-			fail:      true,
+			bind:   `\\.\pipe\foo:\\.\pipe\bar`,
+			driver: "local",
+			expErr: `invalid volume specification: '\\.\pipe\foo:\\.\pipe\bar'`,
 		},
 		{
-			bind:    `\\.\pipe\foo:/data`,
-			driver:  "local",
-			expType: mount.TypeNamedPipe,
-			expRW:   true,
-			fail:    true,
+			bind:   `\\.\pipe\foo:/data`,
+			driver: "local",
+			expErr: `invalid volume specification: '\\.\pipe\foo:/data': invalid mount config for type "npipe": Linux containers on Windows do not support named pipe mounts`,
 		},
 		{
-			bind:    `c:\foo\bar:\\.\pipe\foo`,
-			driver:  "local",
-			expType: mount.TypeNamedPipe,
-			expRW:   true,
-			fail:    true,
+			bind:   `c:\foo\bar:\\.\pipe\foo`,
+			driver: "local",
+			expErr: `invalid volume specification: 'c:\foo\bar:\\.\pipe\foo'`,
 		},
 	}
 
@@ -215,18 +260,14 @@ func TestLCOWParseMountRawSplit(t *testing.T) {
 		tc := tc
 		t.Run(tc.bind, func(t *testing.T) {
 			m, err := parser.ParseMountRaw(tc.bind, tc.driver)
-			if tc.fail {
-				assert.Check(t, is.ErrorContains(err, ""), "expected an error")
+			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.Equal(m.Destination, tc.expDest))
-			assert.Check(t, is.Equal(m.Source, tc.expSource))
-			assert.Check(t, is.Equal(m.Name, tc.expName))
-			assert.Check(t, is.Equal(m.Driver, tc.expDriver))
-			assert.Check(t, is.Equal(m.RW, tc.expRW))
-			assert.Check(t, is.Equal(m.Type, tc.expType))
+			assert.Check(t, is.DeepEqual(*m, *tc.expected, cmpopts.IgnoreUnexported(MountPoint{})))
 		})
 	}
 }

volume/mounts/linux_parser_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/api/types/mount"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 )
@@ -83,16 +84,16 @@ func TestLinuxParseMountRaw(t *testing.T) {
 
 	for _, path := range valid {
 		if _, err := parser.ParseMountRaw(path, "local"); err != nil {
-			t.Errorf("ParseMountRaw(`%q`) should succeed: error %q", path, err)
+			t.Errorf("ParseMountRaw(%q) should succeed: error %q", path, err)
 		}
 	}
 
 	for path, expectedError := range invalid {
 		if mp, err := parser.ParseMountRaw(path, "local"); err == nil {
-			t.Errorf("ParseMountRaw(`%q`) should have failed validation. Err '%v' - MP: %v", path, err, mp)
+			t.Errorf("ParseMountRaw(%q) should have failed validation. Err '%v' - MP: %v", path, err, mp)
 		} else {
 			if !strings.Contains(err.Error(), expectedError) {
-				t.Errorf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error())
+				t.Errorf("ParseMountRaw(%q) error should contain %q, got %v", path, expectedError, err.Error())
 			}
 		}
 	}
@@ -100,77 +101,146 @@ func TestLinuxParseMountRaw(t *testing.T) {
 
 func TestLinuxParseMountRawSplit(t *testing.T) {
 	cases := []struct {
-		bind      string
-		driver    string
-		expType   mount.Type
-		expDest   string
-		expSource string
-		expName   string
-		expDriver string
-		expRW     bool
-		fail      bool
+		bind     string
+		driver   string
+		expected *MountPoint
+		expErr   string
 	}{
 		{
-			bind:      "/tmp:/tmp1",
-			expType:   mount.TypeBind,
-			expDest:   "/tmp1",
-			expSource: "/tmp",
-			expRW:     true,
+			bind: "/tmp:/tmp1",
+			expected: &MountPoint{
+				Source:      "/tmp",
+				Destination: "/tmp1",
+				RW:          true,
+				Type:        mount.TypeBind,
+				Propagation: "rprivate",
+				Spec: mount.Mount{
+					Source:   "/tmp",
+					Target:   "/tmp1",
+					ReadOnly: false,
+					Type:     mount.TypeBind,
+				},
+			},
 		},
 		{
-			bind:      "/tmp:/tmp2:ro",
-			expType:   mount.TypeBind,
-			expDest:   "/tmp2",
-			expSource: "/tmp",
+			bind: "/tmp:/tmp2:ro",
+			expected: &MountPoint{
+				Source:      "/tmp",
+				Destination: "/tmp2",
+				RW:          false,
+				Type:        mount.TypeBind,
+				Mode:        "ro",
+				Propagation: "rprivate",
+				Spec: mount.Mount{
+					Source:   "/tmp",
+					Target:   "/tmp2",
+					ReadOnly: true,
+					Type:     mount.TypeBind,
+				},
+			},
 		},
 		{
-			bind:      "/tmp:/tmp3:rw",
-			expType:   mount.TypeBind,
-			expDest:   "/tmp3",
-			expSource: "/tmp",
-			expRW:     true,
+			bind: "/tmp:/tmp3:rw",
+			expected: &MountPoint{
+				Source:      "/tmp",
+				Destination: "/tmp3",
+				RW:          true,
+				Type:        mount.TypeBind,
+				Mode:        "rw",
+				Propagation: "rprivate",
+				Spec: mount.Mount{
+					Source:   "/tmp",
+					Target:   "/tmp3",
+					ReadOnly: false,
+					Type:     mount.TypeBind,
+				},
+			},
 		},
 		{
-			bind:    "/tmp:/tmp4:foo",
-			expType: mount.TypeBind,
-			fail:    true,
+			bind:   "/tmp:/tmp4:foo",
+			expErr: `invalid mode: foo`,
 		},
 		{
-			bind:    "name:/named1",
-			expType: mount.TypeVolume,
-			expDest: "/named1",
-			expName: "name",
-			expRW:   true,
+			bind: "name:/named1",
+			expected: &MountPoint{
+				Destination: "/named1",
+				RW:          true,
+				Name:        "name",
+				Type:        mount.TypeVolume,
+				Mode:        "", // FIXME(thaJeztah): why is this different than an explicit "rw" ?
+				Propagation: "",
+				CopyData:    true,
+				Spec: mount.Mount{
+					Source:   "name",
+					Target:   "/named1",
+					ReadOnly: false,
+					Type:     mount.TypeVolume,
+				},
+			},
 		},
 		{
-			bind:      "name:/named2",
-			driver:    "external",
-			expType:   mount.TypeVolume,
-			expDest:   "/named2",
-			expName:   "name",
-			expDriver: "external",
-			expRW:     true,
+			bind:   "name:/named2",
+			driver: "external",
+			expected: &MountPoint{
+				Destination: "/named2",
+				RW:          true,
+				Name:        "name",
+				Driver:      "external",
+				Type:        mount.TypeVolume,
+				Mode:        "", // FIXME(thaJeztah): why is this different than an explicit "rw" ?
+				Propagation: "",
+				CopyData:    true,
+				Spec: mount.Mount{
+					Source:        "name",
+					Target:        "/named2",
+					ReadOnly:      false,
+					Type:          mount.TypeVolume,
+					VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "external"}},
+				},
+			},
 		},
 		{
-			bind:      "name:/named3:ro",
-			driver:    "local",
-			expType:   mount.TypeVolume,
-			expDest:   "/named3",
-			expName:   "name",
-			expDriver: "local",
+			bind:   "name:/named3:ro",
+			driver: "local",
+			expected: &MountPoint{
+				Destination: "/named3",
+				RW:          false,
+				Name:        "name",
+				Driver:      "local",
+				Type:        mount.TypeVolume,
+				Mode:        "ro",
+				Propagation: "",
+				CopyData:    true,
+				Spec: mount.Mount{
+					Source:        "name",
+					Target:        "/named3",
+					ReadOnly:      true,
+					Type:          mount.TypeVolume,
+					VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}},
+				},
+			},
 		},
 		{
-			bind:    "local/name:/tmp:rw",
-			expType: mount.TypeVolume,
-			expDest: "/tmp",
-			expName: "local/name",
-			expRW:   true,
+			bind: "local/name:/tmp:rw",
+			expected: &MountPoint{
+				Destination: "/tmp",
+				RW:          true,
+				Name:        "local/name",
+				Type:        mount.TypeVolume,
+				Mode:        "rw",
+				Propagation: "",
+				CopyData:    true,
+				Spec: mount.Mount{
+					Source:   "local/name",
+					Target:   "/tmp",
+					ReadOnly: false,
+					Type:     mount.TypeVolume,
+				},
+			},
 		},
 		{
-			bind:    "/tmp:tmp",
-			expType: mount.TypeBind,
-			expRW:   true,
-			fail:    true,
+			bind:   "/tmp:tmp",
+			expErr: `invalid volume specification: '/tmp:tmp': invalid mount config for type "bind": invalid mount path: 'tmp' mount path must be absolute`,
 		},
 	}
 
@@ -183,18 +253,14 @@ func TestLinuxParseMountRawSplit(t *testing.T) {
 		tc := tc
 		t.Run(tc.bind, func(t *testing.T) {
 			m, err := parser.ParseMountRaw(tc.bind, tc.driver)
-			if tc.fail {
-				assert.Check(t, is.ErrorContains(err, ""), "expected an error")
+			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.Equal(m.Destination, tc.expDest))
-			assert.Check(t, is.Equal(m.Source, tc.expSource))
-			assert.Check(t, is.Equal(m.Name, tc.expName))
-			assert.Check(t, is.Equal(m.Driver, tc.expDriver))
-			assert.Check(t, is.Equal(m.RW, tc.expRW))
-			assert.Check(t, is.Equal(m.Type, tc.expType))
+			assert.Check(t, is.DeepEqual(*m, *tc.expected, cmpopts.IgnoreUnexported(MountPoint{})))
 		})
 	}
 }

volume/mounts/windows_parser.go

@@ -6,7 +6,6 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
-	"runtime"
 	"strings"
 
 	"github.com/docker/docker/api/types/mount"
@@ -145,7 +144,7 @@ func windowsValidMountMode(mode string) bool {
 func windowsValidateNotRoot(p string) error {
 	p = strings.ToLower(strings.ReplaceAll(p, `/`, `\`))
 	if p == "c:" || p == `c:\` {
-		return fmt.Errorf("destination path cannot be `c:` or `c:\\`: %v", p)
+		return fmt.Errorf(`destination path (%v) cannot be 'c:' or 'c:\'`, p)
 	}
 	return nil
 }
@@ -447,7 +446,7 @@ func (p *windowsParser) DefaultPropagationMode() mount.Propagation {
 }
 
 func (p *windowsParser) ConvertTmpfsOptions(opt *mount.TmpfsOptions, readOnly bool) (string, error) {
-	return "", fmt.Errorf("%s does not support tmpfs", runtime.GOOS)
+	return "", errors.New("windows does not support tmpfs")
 }
 
 func (p *windowsParser) DefaultCopyMode() bool {
@@ -459,7 +458,7 @@ func (p *windowsParser) IsBackwardCompatible(m *MountPoint) bool {
 }
 
 func (p *windowsParser) ValidateTmpfsMountDestination(dest string) error {
-	return errors.New("platform does not support tmpfs")
+	return errors.New("windows does not support tmpfs")
 }
 
 func (p *windowsParser) HasResource(m *MountPoint, absolutePath string) bool {

volume/mounts/windows_parser_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/api/types/mount"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 )
@@ -44,8 +45,8 @@ func TestWindowsParseMountRaw(t *testing.T) {
 		`..\`:                              "invalid volume specification: ",
 		`c:\:..\`:                          "invalid volume specification: ",
 		`c:\:d:\:xyzzy`:                    "invalid volume specification: ",
-		`c:`:                               "cannot be `c:`",
-		`c:\`:                              "cannot be `c:`",
+		`c:`:                               "cannot be 'c:'",
+		`c:\`:                              "cannot be 'c:'",
 		`c:\notexist:d:`:                   `source path does not exist: c:\notexist`,
 		`c:\windows\system32\ntdll.dll:d:`: `source path must be a directory`,
 		`name<:d:`:                         `invalid volume specification`,
@@ -92,16 +93,16 @@ func TestWindowsParseMountRaw(t *testing.T) {
 
 	for _, path := range valid {
 		if _, err := parser.ParseMountRaw(path, "local"); err != nil {
-			t.Errorf("ParseMountRaw(`%q`) should succeed: error %q", path, err)
+			t.Errorf("ParseMountRaw(%q) should succeed: error %q", path, err)
 		}
 	}
 
 	for path, expectedError := range invalid {
 		if mp, err := parser.ParseMountRaw(path, "local"); err == nil {
-			t.Errorf("ParseMountRaw(`%q`) should have failed validation. Err '%v' - MP: %v", path, err, mp)
+			t.Errorf("ParseMountRaw(%q) should have failed validation. Err '%v' - MP: %v", path, err, mp)
 		} else {
 			if !strings.Contains(err.Error(), expectedError) {
-				t.Errorf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error())
+				t.Errorf("ParseMountRaw(%q) error should contain %q, got %v", path, expectedError, err.Error())
 			}
 		}
 	}
@@ -109,114 +110,167 @@ func TestWindowsParseMountRaw(t *testing.T) {
 
 func TestWindowsParseMountRawSplit(t *testing.T) {
 	cases := []struct {
-		bind      string
-		driver    string
-		expType   mount.Type
-		expDest   string
-		expSource string
-		expName   string
-		expDriver string
-		expRW     bool
-		fail      bool
+		bind     string
+		driver   string
+		expected *MountPoint
+		expErr   string
 	}{
 		{
-			bind:      `c:\:d:`,
-			driver:    "local",
-			expType:   mount.TypeBind,
-			expDest:   `d:`,
-			expSource: `c:\`,
-			expRW:     true,
+			bind:   `c:\:d:`,
+			driver: "local",
+			expected: &MountPoint{
+				Source:      `c:\`,
+				Destination: `d:`,
+				RW:          true,
+				Type:        mount.TypeBind,
+				Spec: mount.Mount{
+					Source:   `c:\`,
+					Target:   `d:`,
+					ReadOnly: false,
+					Type:     mount.TypeBind,
+				},
+			},
 		},
 		{
-			bind:      `c:\:d:\`,
-			driver:    "local",
-			expType:   mount.TypeBind,
-			expDest:   `d:\`,
-			expSource: `c:\`,
-			expRW:     true,
+			bind:   `c:\:d:\`,
+			driver: "local",
+			expected: &MountPoint{
+				Source:      `c:\`,
+				Destination: `d:\`,
+				RW:          true,
+				Type:        mount.TypeBind,
+				Spec: mount.Mount{
+					Source:   `c:\`,
+					Target:   `d:\`,
+					ReadOnly: false,
+					Type:     mount.TypeBind,
+				},
+			},
 		},
 		{
-			bind:      `c:\:d:\:ro`,
-			driver:    "local",
-			expType:   mount.TypeBind,
-			expDest:   `d:\`,
-			expSource: `c:\`,
+			bind: `c:\:d:\:ro`,
+			expected: &MountPoint{
+				Source:      `c:\`,
+				Destination: `d:\`,
+				RW:          false,
+				Type:        mount.TypeBind,
+				Mode:        "ro",
+				Spec: mount.Mount{
+					Source:   `c:\`,
+					Target:   `d:\`,
+					ReadOnly: true,
+					Type:     mount.TypeBind,
+					// BindOptions: &mount.BindOptions{},
+				},
+			},
 		},
 		{
-			bind:      `c:\:d:\:rw`,
-			driver:    "local",
-			expType:   mount.TypeBind,
-			expDest:   `d:\`,
-			expSource: `c:\`,
-			expRW:     true,
+			bind: `c:\:d:\:rw`,
+			expected: &MountPoint{
+				Source:      `c:\`,
+				Destination: `d:\`,
+				RW:          true,
+				Type:        mount.TypeBind,
+				Mode:        "rw",
+				Spec: mount.Mount{
+					Source:   `c:\`,
+					Target:   `d:\`,
+					ReadOnly: false,
+					Type:     mount.TypeBind,
+				},
+			},
 		},
 		{
-			bind:      `c:\:d:\:foo`,
-			driver:    "local",
-			expType:   mount.TypeBind,
-			expDest:   `d:\`,
-			expSource: `c:\`,
-			fail:      true,
+			bind:   `c:\:d:\:foo`,
+			expErr: `invalid volume specification: 'c:\:d:\:foo'`,
 		},
 		{
-			bind:      `name:d::rw`,
-			driver:    "local",
-			expType:   mount.TypeVolume,
-			expDest:   `d:`,
-			expName:   `name`,
-			expDriver: "local",
-			expRW:     true,
+			bind:   `name:d::rw`,
+			driver: "local",
+			expected: &MountPoint{
+				Destination: `d:`,
+				RW:          true,
+				Name:        `name`,
+				Driver:      `local`,
+				Type:        mount.TypeVolume,
+				Mode:        `rw`,
+				Spec: mount.Mount{
+					Source:        `name`,
+					Target:        `d:`,
+					ReadOnly:      false,
+					Type:          mount.TypeVolume,
+					VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}},
+				},
+			},
 		},
 		{
-			bind:      `name:d:`,
-			driver:    "local",
-			expType:   mount.TypeVolume,
-			expDest:   `d:`,
-			expName:   `name`,
-			expDriver: "local",
-			expRW:     true,
+			bind:   `name:d:`,
+			driver: "local",
+			expected: &MountPoint{
+				Destination: `d:`,
+				RW:          true,
+				Name:        `name`,
+				Driver:      `local`,
+				Type:        mount.TypeVolume,
+				Mode:        ``, // FIXME(thaJeztah): why is this different than an explicit "rw" ?
+				Spec: mount.Mount{
+					Source:        `name`,
+					Target:        `d:`,
+					ReadOnly:      false,
+					Type:          mount.TypeVolume,
+					VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}},
+				},
+			},
 		},
 		{
-			bind:      `name:d::ro`,
-			driver:    "local",
-			expType:   mount.TypeVolume,
-			expDest:   `d:`,
-			expName:   `name`,
-			expDriver: "local",
+			bind:   `name:d::ro`,
+			driver: "local",
+			expected: &MountPoint{
+				Destination: `d:`,
+				RW:          false,
+				Name:        `name`,
+				Driver:      `local`,
+				Type:        mount.TypeVolume,
+				Mode:        `ro`,
+				Spec: mount.Mount{
+					Source:        `name`,
+					Target:        `d:`,
+					ReadOnly:      true,
+					Type:          mount.TypeVolume,
+					VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}},
+				},
+			},
 		},
 		{
-			bind:    `name:c:`,
-			expType: mount.TypeVolume,
-			expRW:   true,
-			fail:    true,
+			bind:   `name:c:`,
+			expErr: `invalid volume specification: 'name:c:': invalid mount config for type "volume": destination path (c:) cannot be 'c:' or 'c:\'`,
 		},
 		{
-			bind:    `driver/name:c:`,
-			expType: mount.TypeVolume,
-			expRW:   true,
-			fail:    true,
+			bind:   `driver/name:c:`,
+			expErr: `invalid volume specification: 'driver/name:c:'`,
 		},
 		{
-			bind:      `\\.\pipe\foo:\\.\pipe\bar`,
-			driver:    "local",
-			expType:   mount.TypeNamedPipe,
-			expDest:   `\\.\pipe\bar`,
-			expSource: `\\.\pipe\foo`,
-			expRW:     true,
+			bind: `\\.\pipe\foo:\\.\pipe\bar`,
+			expected: &MountPoint{
+				Source:      `\\.\pipe\foo`,
+				Destination: `\\.\pipe\bar`,
+				RW:          true,
+				Type:        mount.TypeNamedPipe,
+				Spec: mount.Mount{
+					Source:   `\\.\pipe\foo`,
+					Target:   `\\.\pipe\bar`,
+					ReadOnly: false,
+					Type:     mount.TypeNamedPipe,
+				},
+			},
 		},
 		{
-			bind:    `\\.\pipe\foo:c:\foo\bar`,
-			driver:  "local",
-			expType: mount.TypeNamedPipe,
-			expRW:   true,
-			fail:    true,
+			bind:   `\\.\pipe\foo:c:\foo\bar`,
+			expErr: `invalid volume specification: '\\.\pipe\foo:c:\foo\bar': invalid mount config for type "npipe": 'c:\foo\bar' is not a valid pipe path`,
 		},
 		{
-			bind:    `c:\foo\bar:\\.\pipe\foo`,
-			driver:  "local",
-			expType: mount.TypeNamedPipe,
-			expRW:   true,
-			fail:    true,
+			bind:   `c:\foo\bar:\\.\pipe\foo`,
+			expErr: `invalid volume specification: 'c:\foo\bar:\\.\pipe\foo': invalid mount config for type "bind": bind source path does not exist: c:\foo\bar`,
 		},
 	}
 
@@ -229,18 +283,14 @@ func TestWindowsParseMountRawSplit(t *testing.T) {
 		tc := tc
 		t.Run(tc.bind, func(t *testing.T) {
 			m, err := parser.ParseMountRaw(tc.bind, tc.driver)
-			if tc.fail {
-				assert.Check(t, is.ErrorContains(err, ""), "expected an error")
+			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.Equal(m.Destination, tc.expDest))
-			assert.Check(t, is.Equal(m.Source, tc.expSource))
-			assert.Check(t, is.Equal(m.Name, tc.expName))
-			assert.Check(t, is.Equal(m.Driver, tc.expDriver))
-			assert.Check(t, is.Equal(m.RW, tc.expRW))
-			assert.Check(t, is.Equal(m.Type, tc.expType))
+			assert.Check(t, is.DeepEqual(*m, *tc.expected, cmpopts.IgnoreUnexported(MountPoint{})))
 		})
 	}
 }