isolate: pull exparchive filewalk code into its own func

client/cmd/isolate/exp_archive_test.go

Index: client/cmd/isolate/exp_archive_test.go
diff --git a/client/cmd/isolate/exp_archive_test.go b/client/cmd/isolate/exp_archive_test.go
new file mode 100644
index 0000000000000000000000000000000000000000..be45df9ee583286471103b309ed033e5f2c40098
--- /dev/null
+++ b/client/cmd/isolate/exp_archive_test.go
@@ -0,0 +1,267 @@
+// Copyright 2017 The LUCI Authors. All rights reserved.
+// Use of this source code is governed under the Apache License, Version 2.0
+// that can be found in the LICENSE file.
+
+package main
+
+import (
+	"errors"
+	"os"
+	"reflect"
+	"testing"
+)
+
+// basicFileInfo implements some of os.FileInfo, and panics if unexpected parts
+// of that interface are called.
+type basicFileInfo struct {
+	size  int64
+	mode  os.FileMode
+	isDir bool
+
+	os.FileInfo
+}
+
+func (bfi basicFileInfo) Size() int64       { return bfi.size }
+func (bfi basicFileInfo) Mode() os.FileMode { return bfi.mode }
+func (bfi basicFileInfo) IsDir() bool       { return bfi.isDir }
+
+type file struct {
+	path string
+	bfi  basicFileInfo
+}
+
+func symlink(path string, size int64) file {
+	return file{
+		path: path,
+		bfi: basicFileInfo{
+			size:  size,
+			mode:  os.ModeSymlink,
+			isDir: false,
+		},
+	}
+}
+
+func regularFile(path string, size int64) file {
+	return file{
+		path: path,
+		bfi: basicFileInfo{
+			size:  size,
+			mode:  0,
+			isDir: false,
+		},
+	}
+}
+
+func directory(path string) file {
+	return file{
+		path: path,
+		bfi: basicFileInfo{
+			size:  0,
+			mode:  os.ModeDir,
+			isDir: true,
+		},
+	}
+}
+
+var errBang = errors.New("bang")
+
+func TestWalkFn(t *testing.T) {
+	type testCase struct {
+		name      string
+		files     []file
+		walkFnErr error
+		want      partitionedDeps
+		wantErr   error
+	}
+
+TestCases:
+	for _, tc := range []testCase{
+		{
+			name: "partitions files",
+			files: []file{

[mithro, 2017/06/29 07:25:18]

I think this test would technically have dir("/rootDir") before the file items
below? None of these are going to hit the "xxx.IsDir() == true" on line 112.

[mcgreevy, 2017/06/30 01:24:19]

I don't know what you mean by "this test would technically have dir("/rootDir")
before the file items below?".

line 112 is covered by the "does nothing for directories" test case, now moved
into its own test function.

+				symlink("/rootDir/patha", 1e3),
+				regularFile("/rootDir/pathb", 10e3),
+				regularFile("/rootDir/pathc", 100e3),
+			},
+			want: partitionedDeps{
+				links: itemGroup{
+					items: []*Item{
+						{
+							Path:    "/rootDir/patha",
+							RelPath: "patha",
+							Mode:    os.ModeSymlink,
+							Size:    1e3,
+						},
+					},
+					size: 1e3,
+				},
+				archiveFiles: itemGroup{
+					items: []*Item{
+						{
+							Path:    "/rootDir/pathb",
+							RelPath: "pathb",
+							Mode:    0,
+							Size:    10e3,
+						},
+					},
+					size: 10e3,
+				},
+				indivFiles: itemGroup{
+					items: []*Item{
+						{
+							Path:    "/rootDir/pathc",
+							RelPath: "pathc",
+							Mode:    0,
+							Size:    100e3,
+						},
+					},
+					size: 100e3,
+				},
+			},
+		},
+		{
+			name: "handles zero-size files",
+			files: []file{
+				symlink("/rootDir/patha", 0),
+				regularFile("/rootDir/pathb", 0),
+			},
+			want: partitionedDeps{
+				links: itemGroup{
+					items: []*Item{
+						{
+							Path:    "/rootDir/patha",
+							RelPath: "patha",
+							Mode:    os.ModeSymlink,
+							Size:    0,
+						},
+					},
+					size: 0,
+				},
+				archiveFiles: itemGroup{
+					items: []*Item{
+						{
+							Path:    "/rootDir/pathb",
+							RelPath: "pathb",
+							Mode:    0,
+							Size:    0,
+						},
+					},
+					size: 0,
+				},
+			},
+		},
+		{
+			name: "aggregates link sizes",
+			files: []file{
+				symlink("/rootDir/patha", 1),
+				symlink("/rootDir/pathb", 2),
+			},
+			want: partitionedDeps{
+				links: itemGroup{
+					items: []*Item{
+						{
+							Path:    "/rootDir/patha",
+							RelPath: "patha",
+							Mode:    os.ModeSymlink,
+							Size:    1,
+						},
+						{
+							Path:    "/rootDir/pathb",
+							RelPath: "pathb",
+							Mode:    os.ModeSymlink,
+							Size:    2,
+						},
+					},
+					size: 3,
+				},
+			},
+		},
+		{
+			name: "aggregates large file sizes",
+			files: []file{
+				regularFile("/rootDir/patha", 100e3),
+				regularFile("/rootDir/pathb", 200e3),
+			},
+			want: partitionedDeps{
+				indivFiles: itemGroup{
+					items: []*Item{
+						{
+							Path:    "/rootDir/patha",
+							RelPath: "patha",
+							Mode:    0,
+							Size:    100e3,
+						},
+						{
+							Path:    "/rootDir/pathb",
+							RelPath: "pathb",
+							Mode:    0,
+							Size:    200e3,
+						},
+					},
+					size: 300e3,
+				},
+			},
+		},
+		{
+			name: "aggregates small file sizes",
+			files: []file{
+				regularFile("/rootDir/patha", 10e3),
+				regularFile("/rootDir/pathb", 20e3),
+			},
+			want: partitionedDeps{
+				archiveFiles: itemGroup{
+					items: []*Item{
+						{
+							Path:    "/rootDir/patha",
+							RelPath: "patha",
+							Mode:    0,
+							Size:    10e3,
+						},
+						{
+							Path:    "/rootDir/pathb",
+							RelPath: "pathb",
+							Mode:    0,
+							Size:    20e3,
+						},
+					},
+					size: 30e3,
+				},
+			},
+		},
+		{
+			name:      "returns errors unchanged",
+			files:     []file{regularFile("/rootDir/patha", 1)},
+			walkFnErr: errBang,
+			want:      partitionedDeps{},
+			wantErr:   errBang,
+		},
+		{
+			name:    "does nothing for directories",
+			files:   []file{directory("/rootDir/patha")},
+			want:    partitionedDeps{},
+			wantErr: nil,
+		},
+	} {
+		pw := partitioningWalker{rootDir: "/rootDir"}
+		for _, f := range tc.files {
+			err := pw.walkFn(f.path, f.bfi, tc.walkFnErr)
+			if err != tc.wantErr {
+				t.Errorf("partitioning deps(%s): walkFn got err %v; want %v", tc.name, err, tc.wantErr)
+				continue TestCases
+			}
+		}
+		if got, want := pw.parts, tc.want; !reflect.DeepEqual(got, want) {
+			t.Errorf("partitioning deps(%s): got %#v; want %#v", tc.name, got, want)
+
+		}
+	}
+
+}
+
+func TestWalkFn_BadRelpath(t *testing.T) {
+	pw := partitioningWalker{rootDir: "/rootDir"}
+	f := regularFile("./patha", 1)
+	if err := pw.walkFn(f.path, f.bfi, nil); err == nil {
+		t.Errorf("testing bad relpath: walkFn returned nil err; want non-nil err")
+	}
+}