Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Windows tests #9594

Merged
merged 5 commits into from May 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Expand Up @@ -41,7 +41,7 @@ matrix:
go: 1.13.x
script:
- go build --ldflags="$(go run buildscripts/gen-ldflags.go)" -o %GOPATH%\bin\minio.exe
- for d in $(go list ./... | grep -v browser); do go test -v --timeout 20m "$d"; done
- for d in $(go list ./... | grep -v browser); do go test -v --timeout 20m "$d" || exit -1; done

before_script:
# Add an IPv6 config - see the corresponding Travis issue
Expand Down
56 changes: 36 additions & 20 deletions cmd/endpoint_test.go
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"net"
"net/url"
"path/filepath"
"reflect"
"strings"
"testing"
Expand All @@ -30,14 +31,14 @@ import (
func TestNewEndpoint(t *testing.T) {
u2, _ := url.Parse("https://example.org/path")
u4, _ := url.Parse("http://192.168.253.200/path")

rootSlashFoo, _ := filepath.Abs("/foo")
testCases := []struct {
arg string
expectedEndpoint Endpoint
expectedType EndpointType
expectedErr error
}{
{"/foo", Endpoint{URL: &url.URL{Path: "/foo"}, IsLocal: true}, PathEndpointType, nil},
{"/foo", Endpoint{URL: &url.URL{Path: rootSlashFoo}, IsLocal: true}, PathEndpointType, nil},
{"https://example.org/path", Endpoint{URL: u2, IsLocal: false}, URLEndpointType, nil},
{"http://192.168.253.200/path", Endpoint{URL: u4, IsLocal: false}, URLEndpointType, nil},
{"", Endpoint{}, -1, fmt.Errorf("empty or root endpoint is not supported")},
Expand All @@ -55,30 +56,38 @@ func TestNewEndpoint(t *testing.T) {
{"192.168.1.210:9000", Endpoint{}, -1, fmt.Errorf("invalid URL endpoint format: missing scheme http or https")},
}

for _, testCase := range testCases {
testCase := testCase
t.Run("", func(t *testing.T) {
endpoint, err := NewEndpoint(testCase.arg)
for i, test := range testCases {
t.Run(fmt.Sprint("case-", i), func(t *testing.T) {
endpoint, err := NewEndpoint(test.arg)
if err == nil {
err = endpoint.UpdateIsLocal()
}

if testCase.expectedErr == nil {
if test.expectedErr == nil {
if err != nil {
t.Errorf("error: expected = <nil>, got = %v", err)
}
} else if err == nil {
t.Errorf("error: expected = %v, got = <nil>", testCase.expectedErr)
} else if testCase.expectedErr.Error() != err.Error() {
t.Errorf("error: expected = %v, got = %v", testCase.expectedErr, err)
t.Errorf("error: expected = %v, got = <nil>", test.expectedErr)
} else if test.expectedErr.Error() != err.Error() {
t.Errorf("error: expected = %v, got = %v", test.expectedErr, err)
}

if err == nil && !reflect.DeepEqual(testCase.expectedEndpoint, endpoint) {
t.Errorf("endpoint: expected = %#v, got = %#v", testCase.expectedEndpoint, endpoint)
if err == nil {
if (test.expectedEndpoint.URL == nil) != (endpoint.URL == nil) {
t.Errorf("endpoint url: expected = %#v, got = %#v", test.expectedEndpoint.URL, endpoint.URL)
return
} else if test.expectedEndpoint.URL.String() != endpoint.URL.String() {
t.Errorf("endpoint url: expected = %#v, got = %#v", test.expectedEndpoint.URL.String(), endpoint.URL.String())
return
}
if !reflect.DeepEqual(test.expectedEndpoint, endpoint) {
t.Errorf("endpoint: expected = %#v, got = %#v", test.expectedEndpoint, endpoint)
}
}

if err == nil && testCase.expectedType != endpoint.Type() {
t.Errorf("type: expected = %+v, got = %+v", testCase.expectedType, endpoint.Type())
if err == nil && test.expectedType != endpoint.Type() {
t.Errorf("type: expected = %+v, got = %+v", test.expectedType, endpoint.Type())
}
})
}
Expand Down Expand Up @@ -129,6 +138,13 @@ func TestCreateEndpoints(t *testing.T) {
}
nonLoopBackIP := nonLoopBackIPs.ToSlice()[0]

mustAbs := func(s string) string {
s, err := filepath.Abs(s)
if err != nil {
t.Fatal(err)
}
return s
}
getExpectedEndpoints := func(args []string, prefix string) ([]*url.URL, []bool) {
var URLs []*url.URL
var localFlags []bool
Expand Down Expand Up @@ -212,17 +228,17 @@ func TestCreateEndpoints(t *testing.T) {

// FS Setup
{"localhost:9000", [][]string{{"http://localhost/d1"}}, "", Endpoints{}, -1, fmt.Errorf("use path style endpoint for FS setup")},
{":443", [][]string{{"/d1"}}, ":443", Endpoints{Endpoint{URL: &url.URL{Path: "/d1"}, IsLocal: true}}, FSSetupType, nil},
{"localhost:10000", [][]string{{"/d1"}}, "localhost:10000", Endpoints{Endpoint{URL: &url.URL{Path: "/d1"}, IsLocal: true}}, FSSetupType, nil},
{":443", [][]string{{"/d1"}}, ":443", Endpoints{Endpoint{URL: &url.URL{Path: mustAbs("/d1")}, IsLocal: true}}, FSSetupType, nil},
{"localhost:10000", [][]string{{"/d1"}}, "localhost:10000", Endpoints{Endpoint{URL: &url.URL{Path: mustAbs("/d1")}, IsLocal: true}}, FSSetupType, nil},
{"localhost:9000", [][]string{{"https://127.0.0.1:9000/d1", "https://localhost:9001/d1", "https://example.com/d1", "https://example.com/d2"}}, "", Endpoints{}, -1, fmt.Errorf("path '/d1' can not be served by different port on same address")},

// XL Setup with PathEndpointType
{":1234", [][]string{{"/d1", "/d2", "/d3", "/d4"}}, ":1234",
Endpoints{
Endpoint{URL: &url.URL{Path: "/d1"}, IsLocal: true},
Endpoint{URL: &url.URL{Path: "/d2"}, IsLocal: true},
Endpoint{URL: &url.URL{Path: "/d3"}, IsLocal: true},
Endpoint{URL: &url.URL{Path: "/d4"}, IsLocal: true},
Endpoint{URL: &url.URL{Path: mustAbs("/d1")}, IsLocal: true},
Endpoint{URL: &url.URL{Path: mustAbs("/d2")}, IsLocal: true},
Endpoint{URL: &url.URL{Path: mustAbs("/d3")}, IsLocal: true},
Endpoint{URL: &url.URL{Path: mustAbs("/d4")}, IsLocal: true},
}, XLSetupType, nil},
// DistXL Setup with URLEndpointType
{":9000", [][]string{{"http://localhost/d1", "http://localhost/d2", "http://localhost/d3", "http://localhost/d4"}}, ":9000", Endpoints{
Expand Down
2 changes: 1 addition & 1 deletion cmd/fs-v1_test.go
Expand Up @@ -279,7 +279,7 @@ func TestFSDeleteObject(t *testing.T) {
t.Fatal("Unexpected error: ", err)
}
// Test with invalid object name
if err := fs.DeleteObject(GlobalContext, bucketName, "\\"); !isSameType(err, ObjectNotFound{}) {
if err := fs.DeleteObject(GlobalContext, bucketName, "\\"); !(isSameType(err, ObjectNotFound{}) || isSameType(err, ObjectNameInvalid{})) {
t.Fatal("Unexpected error: ", err)
}
// Test with object does not exist.
Expand Down
6 changes: 6 additions & 0 deletions cmd/object-api-input-checks.go
Expand Up @@ -18,6 +18,8 @@ package cmd

import (
"context"
"runtime"
"strings"

"github.com/google/uuid"
"github.com/minio/minio-go/v6/pkg/s3utils"
Expand Down Expand Up @@ -50,6 +52,10 @@ func checkBucketAndObjectNames(ctx context.Context, bucket, object string) error
logger.LogIf(ctx, ObjectNameInvalid{Bucket: bucket, Object: object})
return ObjectNameInvalid{Bucket: bucket, Object: object}
}
if runtime.GOOS == globalWindowsOSName && strings.Contains(object, "\\") {
// Objects cannot be contain \ in Windows and is listed as `Characters to Avoid`.
return ObjectNameInvalid{Bucket: bucket, Object: object}
}
return nil
}

Expand Down
9 changes: 5 additions & 4 deletions cmd/posix.go
Expand Up @@ -110,10 +110,7 @@ func checkPathLength(pathName string) error {

// Disallow more than 1024 characters on windows, there
// are no known name_max limits on Windows.
if runtime.GOOS == "windows" {
if len(pathName) <= 1024 {
return nil
}
if runtime.GOOS == "windows" && len(pathName) > 1024 {
return errFileNameTooLong
}

Expand All @@ -130,6 +127,10 @@ func checkPathLength(pathName string) error {
switch p {
case '/':
count = 0 // Reset
case '\\':
if runtime.GOOS == globalWindowsOSName {
count = 0
}
default:
count++
if count > 255 {
Expand Down
38 changes: 20 additions & 18 deletions cmd/posix_windows_test.go
Expand Up @@ -20,6 +20,8 @@ package cmd

import (
"bytes"
"fmt"
"io/ioutil"
"os"
"testing"
)
Expand All @@ -34,20 +36,19 @@ func TestUNCPaths(t *testing.T) {
{"/a/b/c/d/e/f/g", true},
{string(bytes.Repeat([]byte("界"), 85)), true},
// Each path component must be <= 255 bytes long.
{string(bytes.Repeat([]byte("界"), 100)), false},
{string(bytes.Repeat([]byte("界"), 280)), false},
{`/p/q/r/s/t`, true},
}
// Instantiate posix object to manage a disk
var err error
err = os.Mkdir("c:\\testdisk", 0700)
dir, err := ioutil.TempDir("", "testdisk-")
if err != nil {
t.Fatal(err)
}
// Cleanup on exit of test
defer os.RemoveAll("c:\\testdisk")
defer os.RemoveAll(dir)

// Instantiate posix object to manage a disk
var fs StorageAPI
fs, err = newPosix(`c:\testdisk`)
fs, err = newPosix(dir)
if err != nil {
t.Fatal(err)
}
Expand All @@ -58,30 +59,31 @@ func TestUNCPaths(t *testing.T) {
t.Fatal(err)
}

for _, test := range testCases {
err = fs.AppendFile("voldir", test.objName, []byte("hello"))
if err != nil && test.pass {
t.Error(err)
} else if err == nil && !test.pass {
t.Error(err)
}
fs.DeleteFile("voldir", test.objName)
for i, test := range testCases {
t.Run(fmt.Sprint(i), func(t *testing.T) {
err = fs.AppendFile("voldir", test.objName, []byte("hello"))
if err != nil && test.pass {
t.Error(err)
} else if err == nil && !test.pass {
t.Error(err)
}
fs.DeleteFile("voldir", test.objName)
})
}
}

// Test to validate posix behavior on windows when a non-final path component is a file.
func TestUNCPathENOTDIR(t *testing.T) {
var err error
// Instantiate posix object to manage a disk
err = os.Mkdir("c:\\testdisk", 0700)
dir, err := ioutil.TempDir("", "testdisk-")
if err != nil {
t.Fatal(err)
}
// Cleanup on exit of test
defer os.RemoveAll("c:\\testdisk")
defer os.RemoveAll(dir)

var fs StorageAPI
fs, err = newPosix(`c:\testdisk`)
fs, err = newPosix(dir)
if err != nil {
t.Fatal(err)
}
Expand Down
5 changes: 5 additions & 0 deletions cmd/server_test.go
Expand Up @@ -28,6 +28,7 @@ import (
"net/http"
"net/url"
"reflect"
"runtime"
"strings"
"sync"
"testing"
Expand Down Expand Up @@ -120,6 +121,9 @@ func runAllTests(suite *TestSuiteCommon, c *check) {
}

func TestServerSuite(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("cannot set up server reliably on Windows")
}
testCases := []*TestSuiteCommon{
// Init and run test on FS backend with signature v4.
{serverType: "FS", signer: signerV4},
Expand Down Expand Up @@ -599,6 +603,7 @@ func (s *TestSuiteCommon) TestDeleteMultipleObjects(c *check) {
c.Assert(err, nil)
err = xml.Unmarshal(delRespBytes, &deleteResp)
c.Assert(err, nil)
c.Assert(len(deleteResp.DeletedObjects), len(delObjReq.Objects))
for i := 0; i < 10; i++ {
c.Assert(deleteResp.DeletedObjects[i], delObjReq.Objects[i])
}
Expand Down