buildbot package: actually find commits for each build during ingestion

go/buildbot/buildbot_test.go

 package buildbot
 
 import (
         "bytes"
         "encoding/json"
         "fmt"
         "io"
         "net/http"
+        "path/filepath"
         "reflect"
         "testing"
 
         "github.com/golang/glog"
 
         "skia.googlesource.com/buildbot.git/go/database"
+        "skia.googlesource.com/buildbot.git/go/gitinfo"
         "skia.googlesource.com/buildbot.git/go/testutils"
+        "skia.googlesource.com/buildbot.git/go/util"
 )
 
 var (
         // testJsonInput is raw JSON data as returned from the build master.
         testJsonInput = testutils.MustReadFile("default_build.json")
 
         // testIncompleteBuild is JSON data for a not-yet-finished build.
         testIncompleteBuild = testutils.MustReadFile("unfinished_build.json")
 
         // Results for /json/builders on various masters.
@@ skipping 63 matching lines @@
         if data, ok := urlMap[url]; ok {
                 return &http.Response{
                         Body: &respBodyCloser{bytes.NewReader(data)},
                 }, nil
         }
         return nil, fmt.Errorf("No such URL in urlMap!")
 }
 
 // testGetBuild is a helper function which pretends to load JSON data from a
 // build master and decodes it into a Build object.
-func testGetBuildFromMaster() (*Build, error) {
+func testGetBuildFromMaster(repo *gitinfo.GitInfo) (*Build, error) {
         httpGet = testGet
-        return GetBuildFromMaster("client.skia", "Test-Ubuntu12-ShuttleA-GTX660-x86-Release", 721)
+        return getBuildFromMaster("client.skia", "Test-Ubuntu12-ShuttleA-GTX660-x86-Release", 721, repo)
 }
 
 // TestGetBuildFromMaster verifies that we can load JSON data from the build master and
 // decode it into a Build object.
 func TestGetBuildFromMaster(t *testing.T) {
+        clearDB(t, ProdDatabaseConfig(true))

[borenet, 2014/12/11 16:34:23]

Now that ingesting a build runs findCommitsForBuild which requires both a
database and a Git repository, all tests which use testGetBuildFromMaster must
run clearDB() and set up the temp repo.

+
+        // Load the test repo.
+        tr := util.NewTempRepo()
+        defer tr.Cleanup()
+        repo, err := gitinfo.NewGitInfo(filepath.Join(tr.Dir, "testrepo"), false, true)
+        if err != nil {
+                t.Fatal(err)
+        }
+
         // Default, complete build.
-        if _, err := testGetBuildFromMaster(); err != nil {
+        if _, err := testGetBuildFromMaster(repo); err != nil {
                 t.Fatal(err)
         }
         // Incomplete build.
-        _, err := GetBuildFromMaster("client.skia", "Test-Ubuntu12-ShuttleA-GTX550Ti-x86_64-Release-Valgrind", 152)
+        _, err = getBuildFromMaster("client.skia", "Test-Ubuntu12-ShuttleA-GTX550Ti-x86_64-Release-Valgrind", 152, repo)
         if err != nil {
                 t.Fatal(err)
         }
 }
 
 // TestBuildJsonSerialization verifies that we can serialize a build to JSON
 // and back without losing or corrupting the data.
 func TestBuildJsonSerialization(t *testing.T) {
-        b1, err := testGetBuildFromMaster()
+        if err := clearDB(t, ProdDatabaseConfig(true)); err != nil {
+                t.Fatal(err)
+        }
+
+        // Load the test repo.
+        tr := util.NewTempRepo()
+        defer tr.Cleanup()
+        repo, err := gitinfo.NewGitInfo(filepath.Join(tr.Dir, "testrepo"), false, true)
         if err != nil {
                 t.Fatal(err)
         }
+
+        b1, err := testGetBuildFromMaster(repo)
+        if err != nil {
+                t.Fatal(err)
+        }
         bytes, err := json.Marshal(b1)
         if err != nil {
                 t.Fatal(err)
         }
         b2 := &Build{}
         if err := json.Unmarshal(bytes, b2); err != nil {
                 t.Fatal(err)
         }
         if !reflect.DeepEqual(b1, b2) {
                 t.Fatalf("Serialization diff:\nIn:  %v\nOut: %v", b1, b2)
         }
 }
 
+// TestFindCommitsForBuild verifies that findCommitsForBuild correctly obtains
+// the list of commits which were newly built in a given build.
+func TestFindCommitsForBuild(t *testing.T) {
+        if err := clearDB(t, ProdDatabaseConfig(true)); err != nil {
+                t.Fatal(err)
+        }
+
+        // Load the test repo.
+        tr := util.NewTempRepo()
+        defer tr.Cleanup()
+        repo, err := gitinfo.NewGitInfo(filepath.Join(tr.Dir, "testrepo"), false, true)
+        if err != nil {
+                t.Fatal(err)
+        }
+
+        // The test repo is laid out like this:
+        //
+        // *   06eb2a58139d3ff764f10232d5c8f9362d55e20f I (HEAD, master, Build #4)
+        // *   ecb424466a4f3b040586a062c15ed58356f6590e F (Build #3)
+        // |\
+        // | * d30286d2254716d396073c177a754f9e152bbb52 H
+        // | * 8d2d1247ef5d2b8a8d3394543df6c12a85881296 G (Build #2)
+        // * | 67635e7015d74b06c00154f7061987f426349d9f E
+        // * | 6d4811eddfa637fac0852c3a0801b773be1f260d D (Build #1)
+        // * | d74dfd42a48325ab2f3d4a97278fc283036e0ea4 C
+        // |/
+        // *   4b822ebb7cedd90acbac6a45b897438746973a87 B (Build #0)
+        // *   051955c355eb742550ddde4eccc3e90b6dc5b887 A
+        //
+        hashes := map[rune]string{
+                'A': "051955c355eb742550ddde4eccc3e90b6dc5b887",
+                'B': "4b822ebb7cedd90acbac6a45b897438746973a87",
+                'C': "d74dfd42a48325ab2f3d4a97278fc283036e0ea4",
+                'D': "6d4811eddfa637fac0852c3a0801b773be1f260d",
+                'E': "67635e7015d74b06c00154f7061987f426349d9f",
+                'F': "ecb424466a4f3b040586a062c15ed58356f6590e",
+                'G': "8d2d1247ef5d2b8a8d3394543df6c12a85881296",
+                'H': "d30286d2254716d396073c177a754f9e152bbb52",
+                'I': "06eb2a58139d3ff764f10232d5c8f9362d55e20f",
+        }
+
+        // Test cases. Each test case builds on the previous cases.
+        testCases := []struct {
+                GotRevision string
+                Expected    []string
+        }{
+                // 0. The first build.
+                {
+                        GotRevision: hashes['B'],
+                        Expected:    []string{hashes['B'], hashes['A']},
+                },
+                // 1. On a linear set of commits, with at least one previous build.
+                {
+                        GotRevision: hashes['D'],
+                        Expected:    []string{hashes['D'], hashes['C']},
+                },
+                // 2. The first build on a new branch.
+                {
+                        GotRevision: hashes['G'],
+                        Expected:    []string{hashes['G']},
+                },
+                // 3. After a merge.
+                {
+                        GotRevision: hashes['F'],
+                        Expected:    []string{hashes['F'], hashes['E'], hashes['H']},
+                },
+                // 4. One last "normal" build.
+                {
+                        GotRevision: hashes['I'],
+                        Expected:    []string{hashes['I']},
+                },
+                // 5. No GotRevision.
+                {
+                        GotRevision: "",
+                        Expected:    []string{},
+                },
+        }
+        for buildNum, tc := range testCases {
+                b, err := testGetBuildFromMaster(repo)
+                if err != nil {
+                        t.Fatal(err)
+                }
+                b.GotRevision = tc.GotRevision
+                b.Number = buildNum
+                c, err := findCommitsForBuild(b, repo)
+                if err != nil {
+                        t.Fatal(err)
+                }
+                if !reflect.DeepEqual(c, tc.Expected) {
+                        t.Fatalf("Commits for build do not match expectation.\nGot:  %v\nWant: %v", c, tc.Expected)
+                }
+                b.Commits = c
+                if err := b.ReplaceIntoDB(); err != nil {
+                        t.Fatal(err)
+                }
+        }
+}
+
 // dbSerializeAndCompare is a helper function used by TestDbBuild which takes
 // a Build object, writes it into the database, reads it back out, and compares
 // the structs. Returns any errors encountered including a comparison failure.
 func dbSerializeAndCompare(b1 *Build) error {
         if err := b1.ReplaceIntoDB(); err != nil {
                 return err
         }
         b2, err := GetBuildFromDB(b1.MasterName, b1.BuilderName, b1.Number)
         if err != nil {
                 return err
         }
 
         if !reflect.DeepEqual(b1, b2) {
                 for i, s := range b1.Steps {
                         if !reflect.DeepEqual(s, b2.Steps[i]) {
                                 glog.Errorf("Not equal:\n %+v\n %+v\n", s, b2.Steps[i])
                         }
                 }
                 return fmt.Errorf("Builds are not equal! Builds:\nExpected: %+v\nActual:   %+v", b1, b2)
         }
         return nil
 }
 
 // testBuildDbSerialization verifies that we can write a build to the DB and
 // pull it back out without losing or corrupting the data.
 func testBuildDbSerialization(t *testing.T, conf *database.DatabaseConfig) {
         clearDB(t, conf)
+        // Load the test repo.
+        tr := util.NewTempRepo()
+        defer tr.Cleanup()
+        repo, err := gitinfo.NewGitInfo(filepath.Join(tr.Dir, "testrepo"), false, true)
+        if err != nil {
+                t.Fatal(err)
+        }
 
         // Test case: an empty build. Tests null and empty values.
         emptyTime := 0.0
         emptyBuild := &Build{
                 Steps:   []*BuildStep{},
                 Times:   []float64{emptyTime, emptyTime},
                 Commits: []string{},
         }
 
         // Test case: a completely filled-out build.
-        buildFromFullJson, err := testGetBuildFromMaster()
+        buildFromFullJson, err := testGetBuildFromMaster(repo)
         if err != nil {
                 t.Fatal(err)
         }
-        // TODO(borenet): Remove this once we actually associate commits with builds.
-        buildFromFullJson.Commits = []string{
-                "1e7841f2a7a06065e926d6544e833f400fec9d6e",
-                "3209f6591512980197d410ac1e6139a5e4731a38",
-                "b578bd76143927ac14981c2ee235b06366b10e7f",
-        }
 
         testCases := []*Build{emptyBuild, buildFromFullJson}
         for _, b := range testCases {
                 if err = dbSerializeAndCompare(b); err != nil {
                         t.Fatal(err)
                 }
         }
 }
 
 // testUnfinishedBuild verifies that we can write a build which is not yet
 // finished, load the build back from the database, and update it when it
 // finishes.
 func testUnfinishedBuild(t *testing.T, conf *database.DatabaseConfig) {
         clearDB(t, conf)
+        // Load the test repo.
+        tr := util.NewTempRepo()
+        defer tr.Cleanup()
+        repo, err := gitinfo.NewGitInfo(filepath.Join(tr.Dir, "testrepo"), false, true)
+        if err != nil {
+                t.Fatal(err)
+        }
 
         // Obtain and insert an unfinished build.
         httpGet = testGet
-        b, err := GetBuildFromMaster("client.skia", "Test-Ubuntu12-ShuttleA-GTX550Ti-x86_64-Release-Valgrind", 152)
+        b, err := getBuildFromMaster("client.skia", "Test-Ubuntu12-ShuttleA-GTX550Ti-x86_64-Release-Valgrind", 152, repo)
         if err != nil {
                 t.Fatal(err)
         }
         if b.IsFinished() {
                 t.Fatal(fmt.Errorf("Unfinished build thinks it's finished!"))
         }
         if err := dbSerializeAndCompare(b); err != nil {
                 t.Fatal(err)
         }
 
@@ skipping 52 matching lines @@
         }
         if found {
                 t.Fatal(fmt.Errorf("Finished build was found by getUnfinishedBuilds!"))
         }
 }
 
 // testLastProcessedBuilds verifies that getLastProcessedBuilds gives us
 // the expected result.
 func testLastProcessedBuilds(t *testing.T, conf *database.DatabaseConfig) {
         clearDB(t, conf)
-        build, err := testGetBuildFromMaster()
+        // Load the test repo.
+        tr := util.NewTempRepo()
+        defer tr.Cleanup()
+        repo, err := gitinfo.NewGitInfo(filepath.Join(tr.Dir, "testrepo"), false, true)
         if err != nil {
                 t.Fatal(err)
         }
+
+        build, err := testGetBuildFromMaster(repo)
+        if err != nil {
+                t.Fatal(err)
+        }
 
         // Ensure that we get the right number for not-yet-processed
         // builder/master pair.
         builds, err := getLastProcessedBuilds()
         if err != nil {
                 t.Fatal(err)
         }
         if builds == nil || len(builds) != 0 {
                 t.Fatal(fmt.Errorf("getLastProcessedBuilds returned an unacceptable value for no builds: %v", builds))
         }
 
         // Ensure that we get the right number for a single already-processed
         // builder/master pair.
         if err := build.ReplaceIntoDB(); err != nil {
                 t.Fatal(err)
         }
         builds, err = getLastProcessedBuilds()
         if err != nil {
                 t.Fatal(err)
         }
         if builds == nil || len(builds) != 1 {
                 t.Fatal(fmt.Errorf("getLastProcessedBuilds returned incorrect number of results: %v", builds))
         }
         if builds[0].MasterName != build.MasterName || builds[0].BuilderName != build.BuilderName || builds[0].Number != build.Number {
                 t.Fatal(fmt.Errorf("getLastProcessedBuilds returned the wrong build: %v", builds[0]))
         }
 
         // Ensure that we get the correct result for multiple builders.
-        build2, err := testGetBuildFromMaster()
+        build2, err := testGetBuildFromMaster(repo)
         if err != nil {
                 t.Fatal(err)
         }
         build2.BuilderName = "Other-Builder"
         build2.Number = build.Number + 10
         if err := build2.ReplaceIntoDB(); err != nil {
                 t.Fatal(err)
         }
         builds, err = getLastProcessedBuilds()
         if err != nil {
@@ skipping 15 matching lines @@
                                 return false
                         }
                 }
                 return true
         }
         if !compareBuildLists([]*Build{build, build2}, builds) {
                 t.Fatal(fmt.Errorf("getLastProcessedBuilds returned incorrect results: %v", builds))
         }
 
         // Add "older" build, ensure that only the newer ones are returned.
-        build3, err := testGetBuildFromMaster()
+        build3, err := testGetBuildFromMaster(repo)
         if err != nil {
                 t.Fatal(err)
         }
         build3.Number -= 10
         if err := build3.ReplaceIntoDB(); err != nil {
                 t.Fatal(err)
         }
         builds, err = getLastProcessedBuilds()
         if err != nil {
                 t.Fatal(err)
@@ skipping 26 matching lines @@
         if !reflect.DeepEqual(expected, actual) {
                 t.Fatal(fmt.Errorf("getLatestBuilds returned incorrect results: %v", actual))
         }
 }
 
 // testGetUningestedBuilds verifies that getUningestedBuilds works as expected.
 func testGetUningestedBuilds(t *testing.T, conf *database.DatabaseConfig) {
         // First, insert some builds into the database as a starting point.
         clearDB(t, conf)
 
-        // This builder is no longer found on the master.
-        b1, err := testGetBuildFromMaster()
+        // Load the test repo.
+        tr := util.NewTempRepo()
+        defer tr.Cleanup()
+        repo, err := gitinfo.NewGitInfo(filepath.Join(tr.Dir, "testrepo"), false, true)
         if err != nil {
                 t.Fatal(err)
         }
+
+        // This builder is no longer found on the master.
+        b1, err := testGetBuildFromMaster(repo)
+        if err != nil {
+                t.Fatal(err)
+        }
         b1.MasterName = "client.skia.compile"
         b1.BuilderName = "My-Builder"
         b1.Number = 115
         b1.Steps = []*BuildStep{}
         if err := b1.ReplaceIntoDB(); err != nil {
                 t.Fatal(err)
         }
 
         // This builder needs to load a few builds.
-        b2, err := testGetBuildFromMaster()
+        b2, err := testGetBuildFromMaster(repo)
         if err != nil {
                 t.Fatal(err)
         }
         b2.MasterName = "client.skia.android"
         b2.BuilderName = "Perf-Android-Venue8-PowerVR-x86-Release"
         b2.Number = 463
         b2.Steps = []*BuildStep{}
         if err := b2.ReplaceIntoDB(); err != nil {
                 t.Fatal(err)
         }
 
         // This builder is already up-to-date.
-        b3, err := testGetBuildFromMaster()
+        b3, err := testGetBuildFromMaster(repo)
         if err != nil {
                 t.Fatal(err)
         }
         b3.MasterName = "client.skia.fyi"
         b3.BuilderName = "Housekeeper-PerCommit"
         b3.Number = 1035
         b3.Steps = []*BuildStep{}
         if err := b3.ReplaceIntoDB(); err != nil {
                 t.Fatal(err)
         }
 
         // This builder is already up-to-date.
-        b4, err := testGetBuildFromMaster()
+        b4, err := testGetBuildFromMaster(repo)
         if err != nil {
                 t.Fatal(err)
         }
         b4.MasterName = "client.skia.android"
         b4.BuilderName = "Test-Android-Venue8-PowerVR-x86-Debug"
         b4.Number = 532
         b4.Steps = []*BuildStep{}
         if err := b4.ReplaceIntoDB(); err != nil {
                 t.Fatal(err)
         }
@@ skipping 22 matching lines @@
         }
 }
 
 // testIngestNewBuilds verifies that we can successfully query the masters and
 // the database for new and unfinished builds, respectively, and ingest them
 // into the database.
 func testIngestNewBuilds(t *testing.T, conf *database.DatabaseConfig) {
         // First, insert some builds into the database as a starting point.
         clearDB(t, conf)
 
-        // This builder needs to load a few builds.
-        b1, err := testGetBuildFromMaster()
+        // Load the test repo.
+        tr := util.NewTempRepo()
+        defer tr.Cleanup()
+        repo, err := gitinfo.NewGitInfo(filepath.Join(tr.Dir, "testrepo"), false, true)
         if err != nil {
                 t.Fatal(err)
         }
+
+        // This builder needs to load a few builds.
+        b1, err := testGetBuildFromMaster(repo)
+        if err != nil {
+                t.Fatal(err)
+        }
         b1.MasterName = "client.skia.android"
         b1.BuilderName = "Perf-Android-Venue8-PowerVR-x86-Release"
         b1.Number = 463
         b1.Steps = []*BuildStep{}
         if err := b1.ReplaceIntoDB(); err != nil {
                 t.Fatal(err)
         }
 
         // This builder has no new builds, but the last one wasn't finished
         // at its time of ingestion.
-        b2, err := testGetBuildFromMaster()
+        b2, err := testGetBuildFromMaster(repo)
         if err != nil {
                 t.Fatal(err)
         }
         b2.MasterName = "client.skia.fyi"
         b2.BuilderName = "Housekeeper-PerCommit"
         b2.Number = 1035
         b2.Finished = 0.0
         b2.Steps = []*BuildStep{}
         if err := b2.ReplaceIntoDB(); err != nil {
                 t.Fatal(err)
         }
 
         // Subsequent builders are already up-to-date.
-        b3, err := testGetBuildFromMaster()
+        b3, err := testGetBuildFromMaster(repo)
         b3.MasterName = "client.skia.fyi"
         b3.BuilderName = "Housekeeper-Nightly-RecreateSKPs"
         b3.Number = 58
         b3.Steps = []*BuildStep{}
         if err := b3.ReplaceIntoDB(); err != nil {
                 t.Fatal(err)
         }
 
-        b4, err := testGetBuildFromMaster()
+        b4, err := testGetBuildFromMaster(repo)
         if err != nil {
                 t.Fatal(err)
         }
         b4.MasterName = "client.skia.android"
         b4.BuilderName = "Test-Android-Venue8-PowerVR-x86-Debug"
         b4.Number = 532
         b4.Steps = []*BuildStep{}
         if err := b4.ReplaceIntoDB(); err != nil {
                 t.Fatal(err)
         }
 
         // IngestNewBuilds should process the above Venue8 Perf bot's builds
         // 464-466 as well as Housekeeper-PerCommit's unfinished build #1035.
-        if err := IngestNewBuilds(); err != nil {
+        if err := IngestNewBuilds(repo); err != nil {
                 t.Fatal(err)
         }
 
         // Verify that the expected builds are now in the database.
         expected := []Build{
                 Build{
                         MasterName:  b1.MasterName,
                         BuilderName: b1.BuilderName,
                         Number:      464,
                 },
@@ skipping 81 matching lines @@
         }
         testGetUningestedBuilds(t, localMySQLTestDatabaseConfig("test_user", ""))
 }
 
 func TestMySQLIngestNewBuilds(t *testing.T) {
         if testing.Short() {
                 t.Skip("Skipping MySQL tests with -short.")
         }
         testIngestNewBuilds(t, localMySQLTestDatabaseConfig("test_user", ""))
 }