isolate: pull exparchive filewalk code into its own func

client/cmd/isolate/exp_archive_test.go

+// 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")
+        }
+}