Skip to content

Commit

Permalink
pkg/path: use a subtest helper to test for each OS value
Browse files Browse the repository at this point in the history 
Using a subtest per OS helps so that "return" or "t.Skip" only affect
the OS we're currently iterating on rather than all of them.
This was already a problem with TestClean, whose t.Skip meant
that we were only actually testing for os == Unix.

A number of tests also conditionally appended to the test case list,
such as appending extra Windows-only test cases when os == Windows.
When doing that, append into a new local slice,
as otherwise the append would also affect the next OS values.

TestClean was broken once it tested all OS values as well;
it modified some test case values when os == Windows,
so make a copy of the test case slice first to ensure that's OK.

While here, TestVolumeName only cares about Windows; simplify.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: I9ce635a3034153618a779dd31db030ae76ce5fdc
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1170044
Reviewed-by: Paul Jolly <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
  • Loading branch information
@mvdan
mvdan committed Sep 28, 2023
1 parent 55c6db6 commit 1227a83
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 35 deletions.
4 changes: 2 additions & 2 deletions pkg/path/match_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func errp(e error) string {
}

func TestMatch(t *testing.T) {
for _, os := range []OS{Unix, Windows, Plan9} {
testEachOS(t, []OS{Unix, Windows, Plan9}, func(t *testing.T, os OS) {
for _, tt := range matchTests {
pattern := tt.pattern
s := tt.s
Expand All @@ -114,5 +114,5 @@ func TestMatch(t *testing.T) {
pattern, s, os, ok, errp(err), tt.match, errp(tt.err))
}
}
}
})
}
71 changes: 38 additions & 33 deletions pkg/path/path_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,20 @@
package path

import (
"fmt"
"reflect"
"runtime"
"testing"
)

func testEachOS(t *testing.T, list []OS, fn func(t *testing.T, os OS)) {
for _, os := range list {
t.Run(fmt.Sprintf("OS=%s", os), func(t *testing.T) {
fn(t, os)
})
}
}

type PathTest struct {
path, result string
}
Expand Down Expand Up @@ -101,8 +110,8 @@ var wincleantests = []PathTest{
}

func TestClean(t *testing.T) {
tests := cleantests
for _, os := range []OS{Unix, Windows, Plan9} {
testEachOS(t, []OS{Unix, Windows, Plan9}, func(t *testing.T, os OS) {
tests := append([]PathTest{}, cleantests...) // TODO: replace with slices.Clone
if os == Windows {
for i := range tests {
tests[i].result = FromSlash(tests[i].result, os)
Expand Down Expand Up @@ -132,7 +141,7 @@ func TestClean(t *testing.T) {
t.Errorf("Clean(%q): %v allocs, want zero", test.result, allocs)
}
}
}
})
}

func TestFromAndToSlash(t *testing.T) {
Expand Down Expand Up @@ -185,7 +194,7 @@ var winsplitlisttests = []SplitListTest{
}

func TestSplitList(t *testing.T) {
for _, os := range []OS{Unix, Windows, Plan9} {
testEachOS(t, []OS{Unix, Windows, Plan9}, func(t *testing.T, os OS) {
sep := getOS(os).ListSeparator

tests := []SplitListTest{
Expand All @@ -201,7 +210,7 @@ func TestSplitList(t *testing.T) {
t.Errorf("SplitList(%#q, %q) = %#q, want %#q", test.list, os, l, test.result)
}
}
}
})
}

type SplitTest struct {
Expand Down Expand Up @@ -230,21 +239,20 @@ var winsplittests = []SplitTest{
}

func TestSplit(t *testing.T) {
for _, os := range []OS{Windows, Unix} {
var splittests []SplitTest
splittests = unixsplittests
testEachOS(t, []OS{Unix, Windows}, func(t *testing.T, os OS) {
tests := unixsplittests
if os == Windows {
splittests = append(splittests, winsplittests...)
tests = append(tests, winsplittests...)
}
for _, test := range splittests {
for _, test := range tests {
pair := Split(test.path, os)
d, f := pair[0], pair[1]
if d != test.dir || f != test.file {
t.Errorf("Split(%q, %q) = %q, %q, want %q, %q",
test.path, os, d, f, test.dir, test.file)
}
}
}
})
}

type JoinTest struct {
Expand Down Expand Up @@ -308,17 +316,18 @@ var winjointests = []JoinTest{
}

func TestJoin(t *testing.T) {
for _, os := range []OS{Unix, Windows} {
testEachOS(t, []OS{Unix, Windows}, func(t *testing.T, os OS) {
tests := jointests
if os == Windows {
jointests = append(jointests, winjointests...)
tests = append(tests, winjointests...)
}
for _, test := range jointests {
for _, test := range tests {
expected := FromSlash(test.path, os)
if p := Join(test.elem, os); p != expected {
t.Errorf("join(%q, %q) = %q, want %q", test.elem, os, p, expected)
}
}
}
})
}

type ExtTest struct {
Expand All @@ -334,13 +343,13 @@ var exttests = []ExtTest{
}

func TestExt(t *testing.T) {
for _, os := range []OS{Unix, Windows} {
testEachOS(t, []OS{Unix, Windows}, func(t *testing.T, os OS) {
for _, test := range exttests {
if x := Ext(test.path, os); x != test.ext {
t.Errorf("Ext(%q, %q) = %q, want %q", test.path, os, x, test.ext)
}
}
}
})
}

var basetests = []PathTest{
Expand Down Expand Up @@ -370,7 +379,7 @@ var winbasetests = []PathTest{

func TestBase(t *testing.T) {
tests := basetests
for _, os := range []OS{Unix, Windows} {
testEachOS(t, []OS{Unix, Windows}, func(t *testing.T, os OS) {
if os == Windows {
// make unix tests work on windows
for i := range tests {
Expand All @@ -384,7 +393,7 @@ func TestBase(t *testing.T) {
t.Errorf("Base(%q, %q) = %q, want %q", test.path, os, s, test.result)
}
}
}
})
}

var dirtests = []PathTest{
Expand Down Expand Up @@ -415,7 +424,7 @@ var windirtests = []PathTest{
}

func TestDir(t *testing.T) {
for _, os := range []OS{Unix, Windows} {
testEachOS(t, []OS{Unix, Windows}, func(t *testing.T, os OS) {
tests := dirtests
if os == Windows {
// make unix tests work on windows
Expand All @@ -430,7 +439,7 @@ func TestDir(t *testing.T) {
t.Errorf("Dir(%q, %q) = %q, want %q", test.path, os, s, test.result)
}
}
}
})
}

type IsAbsTest struct {
Expand Down Expand Up @@ -465,7 +474,7 @@ var winisabstests = []IsAbsTest{
}

func TestIsAbs(t *testing.T) {
for _, os := range []OS{Unix, Windows} {
testEachOS(t, []OS{Unix, Windows}, func(t *testing.T, os OS) {
var tests []IsAbsTest
if os == Windows {
tests = append(tests, winisabstests...)
Expand All @@ -491,7 +500,7 @@ func TestIsAbs(t *testing.T) {
t.Errorf("IsAbs(%q, %q) = %v, want %v", test.path, os, r, test.isAbs)
}
}
}
})
}

type RelTests struct {
Expand Down Expand Up @@ -552,7 +561,7 @@ var winreltests = []RelTests{
}

func TestRel(t *testing.T) {
for _, os := range []OS{Unix, Windows} {
testEachOS(t, []OS{Unix, Windows}, func(t *testing.T, os OS) {
tests := append([]RelTests{}, reltests...)
if os == Windows {
for i := range tests {
Expand All @@ -575,7 +584,7 @@ func TestRel(t *testing.T) {
t.Errorf("Rel(%q, %q, %q)=%q, want %q", test.root, test.path, os, got, test.want)
}
}
}
})
}

type VolumeNameTest struct {
Expand Down Expand Up @@ -609,14 +618,10 @@ var volumenametests = []VolumeNameTest{
}

func TestVolumeName(t *testing.T) {
for _, os := range []OS{Unix, Windows} {
if os != Windows {
return
}
for _, v := range volumenametests {
if vol := VolumeName(v.path, os); vol != v.vol {
t.Errorf("VolumeName(%q, %q)=%q, want %q", v.path, os, vol, v.vol)
}
os := Windows
for _, v := range volumenametests {
if vol := VolumeName(v.path, os); vol != v.vol {
t.Errorf("VolumeName(%q, %q)=%q, want %q", v.path, os, vol, v.vol)
}
}
}

0 comments on commit 1227a83

Please sign in to comment.