[gold] Remove gcs baseliner
Was replaced with simple_baseliner.
Change-Id: If60d982872d63279ff26ceef7c5d61d08f1072cb
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/234497
Reviewed-by: Ben Wagner aka dogben <benjaminwagner@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
diff --git a/golden/cmd/skiacorrectness/main.go b/golden/cmd/skiacorrectness/main.go
index 86f8b0f..547c2bd 100644
--- a/golden/cmd/skiacorrectness/main.go
+++ b/golden/cmd/skiacorrectness/main.go
@@ -82,7 +82,6 @@
appTitle = flag.String("app_title", "Skia Gold", "Title of the deployed up on the front end.")
authoritative = flag.Bool("authoritative", false, "Indicates that this instance should write changes that could be triggered on multiple instances running in parallel.")
authorizedUsers = flag.String("auth_users", login.DEFAULT_DOMAIN_WHITELIST, "White space separated list of domains and email addresses that are allowed to login.")
- baselineGSPath = flag.String("baseline_gs_path", "", "GS path, where the baseline file should be stored. If empty no file will be written. Format: <bucket>/<path>.")
btInstanceID = flag.String("bt_instance", "production", "ID of the BigTable instance that contains Git metadata")
btProjectID = flag.String("bt_project_id", "skia-public", "project id with BigTable instance")
cacheSize = flag.Int("cache_size", 1, "Approximate cachesize used to cache images and diff metrics in GiB. This is just a way to limit caching. 0 means no caching at all. Use default for testing.")
@@ -354,9 +353,8 @@
}
gsClientOpt := storage.GCSClientOptions{
- HashesGSPath: *hashesGSPath,
- BaselineGSPath: *baselineGSPath,
- Dryrun: *local,
+ HashesGSPath: *hashesGSPath,
+ Dryrun: *local,
}
gsClient, err := storage.NewGCSClient(client, gsClientOpt)
diff --git a/golden/go/baseline/baseline.go b/golden/go/baseline/baseline.go
index 9ab2f20..d8d80a1 100644
--- a/golden/go/baseline/baseline.go
+++ b/golden/go/baseline/baseline.go
@@ -50,19 +50,3 @@
}
return ret, nil
}
-
-// minCommit returns newCommit if it appears before current (or current is nil).
-func minCommit(current *tiling.Commit, newCommit *tiling.Commit) *tiling.Commit {
- if current == nil || newCommit == nil || newCommit.CommitTime < current.CommitTime {
- return newCommit
- }
- return current
-}
-
-// maxCommit returns newCommit if it appears after current (or current is nil).
-func maxCommit(current *tiling.Commit, newCommit *tiling.Commit) *tiling.Commit {
- if current == nil || newCommit == nil || newCommit.CommitTime > current.CommitTime {
- return newCommit
- }
- return current
-}
diff --git a/golden/go/baseline/gcs_baseliner/baseliner.go b/golden/go/baseline/gcs_baseliner/baseliner.go
deleted file mode 100644
index 3a1e72b..0000000
--- a/golden/go/baseline/gcs_baseliner/baseliner.go
+++ /dev/null
@@ -1,398 +0,0 @@
-package gcs_baseliner
-
-import (
- "context"
- "math"
- "sync"
- "time"
-
- lru "github.com/hashicorp/golang-lru"
- "github.com/patrickmn/go-cache"
- "go.skia.org/infra/go/gitstore"
- "go.skia.org/infra/go/skerr"
- "go.skia.org/infra/go/sklog"
- "go.skia.org/infra/go/tiling"
- "go.skia.org/infra/go/util"
- "go.skia.org/infra/go/vcsinfo"
- "go.skia.org/infra/golden/go/baseline"
- "go.skia.org/infra/golden/go/digest_counter"
- "go.skia.org/infra/golden/go/expstorage"
- "go.skia.org/infra/golden/go/shared"
- "go.skia.org/infra/golden/go/storage"
- "go.skia.org/infra/golden/go/tryjobstore"
- "go.skia.org/infra/golden/go/types"
- "golang.org/x/sync/errgroup"
-)
-
-// TODO(stephana): Tune issueCacheSize by either finding a good value that works across instance
-// or one that can be tuned.
-
-const (
- // issueCacheSize is the size of the baselines cache for issue.
- issueCacheSize = 10000
-
- // baselineExpirationTime is how long the baselines should be cached for reading.
- baselineExpirationTime = time.Minute
-)
-
-// BaselinerImpl is a helper type that provides functions to write baselines (expectations) to
-// GCS and retrieve them. Other packages use it to continuously write expectations to GCS
-// as they become available.
-type BaselinerImpl struct {
- gStorageClient storage.GCSClient
- expectationsStore expstorage.ExpectationsStore
- tryjobStore tryjobstore.TryjobStore
- vcs vcsinfo.VCS
-
- // mutex protects lastWrittenBaselines, baselineCache and currentTile
- mutex sync.RWMutex
-
- // lastWrittenBaselines maps[commit_hash]MD5_sum_of_baseline to keep track whether a baseline for
- // a specific commit has been written already and whether we need to write it again (different MD5)
- lastWrittenBaselines map[string]string
-
- // baselineCache caches the baselines of all commits of the current tile.
- // Maps commit hash to *baseline.Baseline
- baselineCache *cache.Cache
-
- // currTileInfo is the latest tileInfo we have.
- currTileInfo baseline.TileInfo
-
- // issueBaselineCache caches baselines for issue by mapping from issueID to baseline.
- issueBaselineCache *lru.Cache
-}
-
-// New creates a new instance of baseliner.Baseliner that interacts with baselines in GCS.
-func New(gStorageClient storage.GCSClient, expectationsStore expstorage.ExpectationsStore, tryjobStore tryjobstore.TryjobStore, vcs vcsinfo.VCS) (*BaselinerImpl, error) {
- c, err := lru.New(issueCacheSize)
- if err != nil {
- return nil, skerr.Fmt("Error allocating cache: %s", err)
- }
-
- return &BaselinerImpl{
- gStorageClient: gStorageClient,
- expectationsStore: expectationsStore,
- tryjobStore: tryjobStore,
- vcs: vcs,
- issueBaselineCache: c,
- lastWrittenBaselines: map[string]string{},
- baselineCache: cache.New(baselineExpirationTime, baselineExpirationTime),
- }, nil
-}
-
-// CanWriteBaseline implements the baseline.Baseliner interface.
-func (b *BaselinerImpl) CanWriteBaseline() bool {
- return (b.gStorageClient != nil) && (b.gStorageClient.Options().BaselineGSPath != "")
-}
-
-// PushMasterBaselines implements the baseline.Baseliner interface.
-func (b *BaselinerImpl) PushMasterBaselines(tileInfo baseline.TileInfo, targetHash string) (*baseline.Baseline, error) {
- defer shared.NewMetricsTimer("push_master_baselines").Stop()
- b.mutex.Lock()
- defer b.mutex.Unlock()
- if tileInfo == nil {
- tileInfo = b.currTileInfo
- }
- if tileInfo == nil {
- return nil, skerr.Fmt("Received nil tile and no previous tile defined")
- }
-
- if !b.CanWriteBaseline() {
- return nil, skerr.Fmt("Trying to write baseline while GCS path is not configured.")
- }
-
- // Calculate the baselines for the master tile.
- exps, err := b.expectationsStore.Get()
- if err != nil {
- return nil, skerr.Fmt("Unable to retrieve expectations: %s", err)
- }
-
- // Remove negatives/untriaged entries
- exps = exps.AsBaseline()
-
- // Make sure we have all commits, not just the ones that are in the tile. Currently a tile is
- // fetched in intervals. New commits might have arrived since the last tile was read. Below we
- // extrapolate the baselines of the new commits to be identical to the last commit in the tile.
- // As new data arrive in the next tile, we update the baselines for these commits.
- tileCommits := tileInfo.AllCommits()
- if len(tileCommits) == 0 {
- sklog.Warningf("tile is empty, doing nothing")
- return nil, nil
- }
-
- extraCommits, err := b.getCommitsSince(tileCommits[len(tileCommits)-1])
- if err != nil {
- return nil, skerr.Fmt("error getting commits since %v: %s", tileCommits[len(tileCommits)-1], err)
- }
-
- commits := append(extraCommits, tileCommits...)
-
- // Get the current list of files that have been written.
- lastWritten := b.lastWrittenBaselines
-
- // Write the ones to disk that have not been written
- // Maps commit hash -> md5 of expectations
- written := map[string]string{}
- var wMutex sync.Mutex
- var egroup errgroup.Group
-
- md5Sum, err := util.MD5Sum(exps)
- if err != nil {
- return nil, skerr.Wrapf(err, "calculating md5 hash of expectations")
- }
-
- var ret *baseline.Baseline
-
- for _, commit := range commits {
- // If we have written this baseline before, we mark it as written and process the next one.
- if old, ok := lastWritten[commit.Hash]; ok && md5Sum == old {
- wMutex.Lock()
- written[commit.Hash] = md5Sum
- wMutex.Unlock()
- continue
- }
-
- func(commit *tiling.Commit) {
- bLine := baseline.EmptyBaseline()
- bLine.Expectations = exps
- bLine.MD5 = md5Sum
- bLine.Issue = types.MasterBranch
-
- if commit.Hash == targetHash {
- ret = bLine
- }
-
- b.baselineCache.Set(commit.Hash, bLine, cache.DefaultExpiration)
-
- egroup.Go(func() error {
- // Write the baseline to GCS.
- _, err := b.gStorageClient.WriteBaseline(bLine, commit.Hash)
- if err != nil {
- return skerr.Fmt("Error writing baseline to GCS: %s", err)
- }
- wMutex.Lock()
- defer wMutex.Unlock()
-
- written[commit.Hash] = bLine.MD5
- return nil
- })
- }(commit)
- }
-
- if err := egroup.Wait(); err != nil {
- return nil, skerr.Fmt("Problem writing per-commit baselines to GCS: %s", err)
- }
-
- b.currTileInfo = tileInfo
- b.lastWrittenBaselines = written
- return ret, nil
-}
-
-// PushIssueBaseline implements the baseline.Baseliner interface.
-func (b *BaselinerImpl) PushIssueBaseline(issueID int64, tileInfo baseline.TileInfo, dCounter digest_counter.DigestCounter) error {
- issueExpStore := b.expectationsStore.ForIssue(issueID)
- exp, err := issueExpStore.Get()
- if err != nil {
- return skerr.Fmt("Unable to get issue expectations: %s", err)
- }
-
- tryjobs, tryjobResults, err := b.tryjobStore.GetTryjobs(issueID, nil, true, true)
- if err != nil {
- return skerr.Fmt("Unable to get TryjobResults")
- }
-
- base, err := baseline.GetBaselineForIssue(issueID, tryjobs, tryjobResults, exp, tileInfo.AllCommits())
- if err != nil {
- return skerr.Fmt("Error calculating issue baseline: %s", err)
- }
-
- // Add it to the cache.
- _ = b.issueBaselineCache.Add(issueID, base)
-
- if !b.CanWriteBaseline() {
- return skerr.Fmt("Trying to write baseline while GCS path is not configured.")
- }
-
- // Write the baseline to GCS.
- outputPath, err := b.gStorageClient.WriteBaseline(base, "")
- if err != nil {
- return skerr.Fmt("Error writing baseline to GCS: %s", err)
- }
- sklog.Infof("Baseline for issue %d written to %s.", issueID, outputPath)
- return nil
-}
-
-// FetchBaseline implements the baseline.Baseliner interface.
-func (b *BaselinerImpl) FetchBaseline(commitHash string, issueID int64, issueOnly bool) (*baseline.Baseline, error) {
- isIssue := !types.IsMasterBranch(issueID)
-
- var masterBaseline *baseline.Baseline
- var issueBaseline *baseline.Baseline
- var egroup errgroup.Group
-
- // Retrieve the baseline on master.
- egroup.Go(func() error {
- var err error
- masterBaseline, err = b.getMasterExpectations(commitHash)
- if err != nil {
- return skerr.Fmt("Could not get master baseline: %s", err)
- }
- return nil
- })
-
- if isIssue {
- egroup.Go(func() error {
- val, ok := b.issueBaselineCache.Get(issueID)
- if ok {
- issueBaseline = val.(*baseline.Baseline)
- return nil
- }
-
- var err error
- issueBaseline, err = b.gStorageClient.ReadBaseline("", issueID)
- if err != nil {
- return skerr.Fmt("Could not get baseline for issue %d: %s", issueID, err)
- }
-
- // If no baseline was found. We place an empty one.
- if issueBaseline == nil {
- issueBaseline = baseline.EmptyBaseline()
- }
-
- return nil
- })
- }
-
- if err := egroup.Wait(); err != nil {
- return nil, skerr.Fmt("Could not fetch baselines: %s", err)
- }
-
- if isIssue {
- if issueOnly {
- // Only return the portion of the baseline that would be contributed by the issue
- issueBaseline.Issue = issueID
- masterBaseline = issueBaseline
- } else {
- // Clone the retrieved baseline before we inject the issue information.
- masterBaseline = masterBaseline.Copy()
- masterBaseline.Expectations.MergeExpectations(issueBaseline.Expectations)
- masterBaseline.Issue = issueID
- }
- }
- return masterBaseline, nil
-}
-
-// getCommitSince returns all the commits have been added to the repo since the given commit.
-// The returned instances of tiling.Commit do not contain a valid Author field.
-func (b *BaselinerImpl) getCommitsSince(firstCommit *tiling.Commit) ([]*tiling.Commit, error) {
- defer shared.NewMetricsTimer("baseliner_get_commits_since").Stop()
-
- // If there is an underlying gitstore retrieve it, otherwise this function becomes a no-op.
- gitStoreBased, ok := b.vcs.(gitstore.GitStoreBased)
- if !ok {
- return []*tiling.Commit{}, nil
- }
-
- gitStore := gitStoreBased.GetGitStore()
- ctx := context.TODO()
- startTime := time.Unix(firstCommit.CommitTime, 0)
- endTime := startTime.Add(time.Second)
- branch := b.vcs.GetBranch()
- commits, err := gitStore.RangeByTime(ctx, startTime, endTime, branch)
- if err != nil {
- return nil, err
- }
-
- if len(commits) == 0 {
- return nil, skerr.Fmt("No commits found while querying for commit %s", firstCommit.Hash)
- }
-
- var target *vcsinfo.IndexCommit
- for _, c := range commits {
- if c.Hash == firstCommit.Hash {
- target = c
- }
- }
-
- if target == nil {
- return nil, skerr.Fmt("Commit %s not found in gitstore", firstCommit.Hash)
- }
-
- // Fetch all commits after the first one which we already have.
- if commits, err = gitStore.RangeN(ctx, target.Index, int(math.MaxInt32), branch); err != nil {
- return nil, err
- }
-
- ret := make([]*tiling.Commit, len(commits))
- for idx, c := range commits {
- // Note: For the task at hand we don't need to populate the Author field of tiling.Commit.
- ret[idx] = &tiling.Commit{
- Hash: c.Hash,
- CommitTime: c.Timestamp.Unix(),
- }
- }
-
- return ret[1:], nil
-}
-
-func (b *BaselinerImpl) getMasterExpectations(commitHash string) (*baseline.Baseline, error) {
- rv := func() *baseline.Baseline {
- b.mutex.RLock()
- defer b.mutex.RUnlock()
- tileInfo := b.currTileInfo
-
- // If no commit hash was given use current HEAD.
- if commitHash == "" {
- // If we have no tile yet, we cannot get the HEAD of it.
- if tileInfo == nil {
- return baseline.EmptyBaseline()
- }
- // Get the last commit that has data.
- allCommits := tileInfo.AllCommits()
- commitHash = allCommits[len(allCommits)-1].Hash
- }
-
- if base, ok := b.baselineCache.Get(commitHash); ok {
- return base.(*baseline.Baseline).Copy()
- }
- return nil
- }()
- if rv != nil {
- return rv, nil
- }
-
- // We did not find it in the cache so lets load it from GCS.
- ret, err := b.gStorageClient.ReadBaseline(commitHash, types.MasterBranch)
- if err != nil {
- return nil, err
- }
-
- // Look up the commit to see if it's valid.
- if ret == nil {
- // Load the commit and determine if it's on the current branch.
- details, err := b.vcs.Details(context.TODO(), commitHash, true)
- if err != nil {
- return nil, err
- }
-
- // Get the branch we are tracking and make sure that the commit is in that branch.
- branch := b.vcs.GetBranch()
- if !details.Branches[branch] {
- return nil, skerr.Fmt("Commit %s is not in branch %s", commitHash, branch)
- }
-
- // Make sure all expecations are up to date.
- if ret, err = b.PushMasterBaselines(nil, commitHash); err != nil {
- return nil, err
- }
- }
- // Since we fetched from GCS - go ahead and store to cache.
- b.mutex.Lock()
- defer b.mutex.Unlock()
- b.baselineCache.Set(commitHash, ret, cache.DefaultExpiration)
- return ret, nil
-}
-
-// Make sure BaselinerImpl fulfills the Baseliner interfaces
-var _ baseline.BaselineWriter = (*BaselinerImpl)(nil)
-var _ baseline.BaselineFetcher = (*BaselinerImpl)(nil)
diff --git a/golden/go/baseline/gcs_baseliner/baseliner_test.go b/golden/go/baseline/gcs_baseliner/baseliner_test.go
deleted file mode 100644
index 6df4f49..0000000
--- a/golden/go/baseline/gcs_baseliner/baseliner_test.go
+++ /dev/null
@@ -1,190 +0,0 @@
-package gcs_baseliner
-
-import (
- "testing"
-
- "github.com/stretchr/testify/mock"
- assert "github.com/stretchr/testify/require"
- "go.skia.org/infra/go/deepequal"
- "go.skia.org/infra/go/testutils/unittest"
- "go.skia.org/infra/golden/go/baseline"
- "go.skia.org/infra/golden/go/mocks"
- "go.skia.org/infra/golden/go/storage"
- three_devices "go.skia.org/infra/golden/go/testutils/data_three_devices"
- "go.skia.org/infra/golden/go/types"
-)
-
-// Test that the baseliner passes on the request to the storage.GCSClient
-// for a baseline it hasn't seen before
-func TestFetchBaselineSunnyDay(t *testing.T) {
- unittest.SmallTest(t)
-
- testCommitHash := "abcd12345"
-
- mgs := makeMockGCSStorage()
- defer mgs.AssertExpectations(t)
-
- mgs.On("ReadBaseline", testCommitHash, types.MasterBranch).Return(three_devices.MakeTestBaseline(), nil).Once()
-
- baseliner, err := New(mgs, nil, nil, nil)
- assert.NoError(t, err)
-
- b, err := baseliner.FetchBaseline(testCommitHash, types.MasterBranch, false)
- assert.NoError(t, err)
-
- deepequal.AssertDeepEqual(t, three_devices.MakeTestBaseline(), b)
-}
-
-// Test that the baseliner behaves differently when requesting a baseline
-// for a given tryjob.
-func TestFetchBaselineIssueSunnyDay(t *testing.T) {
- unittest.SmallTest(t)
-
- testCommitHash := "abcd12345"
- testIssueID := int64(1234)
-
- // These are valid, but arbitrary md5 hashes
- IotaNewDigest := types.Digest("1115fba4ce5b4cb9ffd595beb63e7389")
- KappaNewDigest := types.Digest("222d894f5b680a9f7bd74c8004b7d88d")
- LambdaNewDigest := types.Digest("3333fe3127b984e4ff39f4885ddb0d98")
-
- additionalTriages := &baseline.Baseline{
- Expectations: types.Expectations{
- "brand-new-test": map[types.Digest]types.Label{
- IotaNewDigest: types.POSITIVE,
- KappaNewDigest: types.NEGATIVE,
- },
- three_devices.BetaTest: map[types.Digest]types.Label{
- LambdaNewDigest: types.POSITIVE,
- // Change these two pre-existing digests
- three_devices.BetaGood1Digest: types.NEGATIVE,
- three_devices.BetaUntriaged1Digest: types.POSITIVE,
- },
- },
- Issue: testIssueID,
- }
-
- mgs := makeMockGCSStorage()
- defer mgs.AssertExpectations(t)
-
- // mock the master baseline
- mgs.On("ReadBaseline", testCommitHash, types.MasterBranch).Return(three_devices.MakeTestBaseline(), nil).Once()
- // mock the expectations that a user would have applied to their CL (that
- // are not live on master yet).
- mgs.On("ReadBaseline", "", testIssueID).Return(additionalTriages, nil).Once()
-
- baseliner, err := New(mgs, nil, nil, nil)
- assert.NoError(t, err)
-
- b, err := baseliner.FetchBaseline(testCommitHash, testIssueID, false)
- assert.NoError(t, err)
-
- assert.Equal(t, testIssueID, b.Issue)
- // The expectation should be the master baseline merged in with the additionalTriages
- // with additionalTriages overwriting existing expectations, if applicable.
- assert.Equal(t, types.Expectations{
- "brand-new-test": map[types.Digest]types.Label{
- IotaNewDigest: types.POSITIVE,
- KappaNewDigest: types.NEGATIVE,
- },
- // AlphaTest should be unchanged from the master baseline.
- three_devices.AlphaTest: map[types.Digest]types.Label{
- three_devices.AlphaGood1Digest: types.POSITIVE,
- },
- three_devices.BetaTest: map[types.Digest]types.Label{
- LambdaNewDigest: types.POSITIVE,
- // Note that the state caused by this set of expectations overwrites what
- // was on the master branch
- three_devices.BetaGood1Digest: types.NEGATIVE,
- three_devices.BetaUntriaged1Digest: types.POSITIVE,
- },
- }, b.Expectations)
-
- // Ensure that reading the issue branch does not impact the master branch
- b, err = baseliner.FetchBaseline(testCommitHash, types.MasterBranch, false)
- assert.NoError(t, err)
- assert.Equal(t, three_devices.MakeTestBaseline(), b)
-}
-
-func TestFetchBaselineCachingSunnyDay(t *testing.T) {
- unittest.SmallTest(t)
-
- testCommitHash := "abcd12345"
-
- mgs := makeMockGCSStorage()
- defer mgs.AssertExpectations(t)
-
- // ReadBaseline should only be called once despite multiple requests below
- mgs.On("ReadBaseline", testCommitHash, types.MasterBranch).Return(three_devices.MakeTestBaseline(), nil).Once()
-
- baseliner, err := New(mgs, nil, nil, nil)
- assert.NoError(t, err)
-
- for i := 0; i < 10; i++ {
- b, err := baseliner.FetchBaseline(testCommitHash, types.MasterBranch, false)
- assert.NoError(t, err)
- assert.NotNil(t, b)
- deepequal.AssertDeepEqual(t, three_devices.MakeTestBaseline(), b)
- }
-}
-
-// TODO(kjlubick): TestFetchBaseline which returns nil (difficult because it shells out to
-// PushMasterBaseline)
-
-// TestPushMasterBaselineSunnyDay verifies that the baseliner pulls in the latest tile
-// (which has the traces and known images) and combines it with the triage status from
-// ExpectationsStorage to create a baseline per commit.
-func TestPushMasterBaselineSunnyDay(t *testing.T) {
- unittest.SmallTest(t)
-
- mgs := makeMockGCSStorage()
- mcs := &mocks.TileInfo{}
- mes := &mocks.ExpectationsStore{}
-
- defer mgs.AssertExpectations(t)
- defer mcs.AssertExpectations(t)
- defer mes.AssertExpectations(t)
-
- mcs.On("AllCommits").Return(three_devices.MakeTestCommits())
-
- mes.On("Get").Return(three_devices.MakeTestExpectations(), nil)
-
- bl := three_devices.MakeTestExpectations().AsBaseline()
-
- mgs.On("WriteBaseline", mock.AnythingOfType("*baseline.Baseline"), mock.AnythingOfType("string")).Run(func(args mock.Arguments) {
- b := args.Get(0).(*baseline.Baseline)
- assert.NotNil(t, b)
-
- assert.Equal(t, bl, b.Expectations)
- assert.Equal(t, types.MasterBranch, b.Issue)
- }).Return("gs://test-bucket/baselines/foo-baseline.json", nil).Times(3) // once per commit
-
- baseliner, err := New(mgs, mes, nil, nil)
- assert.NoError(t, err)
-
- b, err := baseliner.PushMasterBaselines(mcs, "")
- assert.NoError(t, err)
- assert.Nil(t, b) // baseline should be nil because commit is ""
-}
-
-func makeMockGCSStorage() *mocks.GCSClient {
- mgs := mocks.GCSClient{}
- mgs.On("Options").Return(storage.GCSClientOptions{
- HashesGSPath: "gs://test-bucket/hashes",
- BaselineGSPath: "gs://test-bucket/baselines/",
- }).Maybe()
- return &mgs
-}
-
-func assertLabel(t *testing.T, b *baseline.Baseline, testName types.TestName, hash types.Digest, label types.Label) {
- test, ok := b.Expectations[testName]
- if !ok {
- assert.Failf(t, "assertLabel", "Could not find test %s in baseline %#v", testName, b)
- }
-
- lab, ok := test[hash]
- if !ok {
- assert.Failf(t, "assertLabel", "Could not find hash %s in test %#v (name %s) (baseline %#v)", hash, test, testName, b)
- }
- assert.Equal(t, label, lab)
-}
diff --git a/golden/go/baseline/types.go b/golden/go/baseline/types.go
index 6a64539..749196e 100644
--- a/golden/go/baseline/types.go
+++ b/golden/go/baseline/types.go
@@ -1,16 +1,14 @@
package baseline
import (
- "go.skia.org/infra/go/tiling"
- "go.skia.org/infra/golden/go/digest_counter"
"go.skia.org/infra/golden/go/types"
)
// Baseline captures the data necessary to verify test results on the
-// commit queue. A baseline is essentially the expectations for a set of
-// tests in a given commit range.
+// commit queue. A baseline is essentially just the positive expectations
+// for a branch.
type Baseline struct {
- // MD5 is the hash of the Expectations field.
+ // MD5 is the hash of the Expectations field. Can be used to quickly test equality.
MD5 string `json:"md5"`
// Expectations captures the "baseline expectations", that is, the Expectations
@@ -40,24 +38,6 @@
}
}
-// TODO(kjlubick): delete this once fs_baseliner lands
-type BaselineWriter interface {
- // CanWriteBaseline returns true if this instance was configured to write baseline files.
- CanWriteBaseline() bool
-
- // PushMasterBaselines writes the baselines for the master branch to GCS.
- // If tileInfo is nil the tile of the last call to PushMasterBaselines is used. If the
- // function was never called before and tileInfo is nil, an error is returned.
- // If targetHash != "" we also return the baseline for corresponding commit as the first return
- // value. Otherwise the first return value is nil.
- // It is assumed that the target commit is one of the commits that are written as part of
- // this call.
- PushMasterBaselines(tileInfo TileInfo, targetHash string) (*Baseline, error)
-
- // PushIssueBaseline writes the baseline for a Gerrit issue to GCS.
- PushIssueBaseline(issueID int64, tileInfo TileInfo, dCounter digest_counter.DigestCounter) error
-}
-
type BaselineFetcher interface {
// FetchBaseline fetches the complete baseline for the given Gerrit issue by
// loading the master baseline and the issue baseline from GCS and combining
@@ -69,21 +49,3 @@
// baselines have been removed.
FetchBaseline(commitHash string, issueID int64, issueOnly bool) (*Baseline, error)
}
-
-// TileInfo is an interface around a subset of the functionality given by types.ComplexTile.
-// Specifically, Baseliner needs a way to get information about what commits in the tile we
-// are considering.
-type TileInfo interface {
- // AllCommits returns all commits that were processed to get the data commits.
- // Its first commit should match the first commit returned when calling DataCommits.
- AllCommits() []*tiling.Commit
-
- // DataCommits returns all commits that contain data. In some busy repos, there are commits
- // that don't get tested directly because the commits are batched in with others.
- // DataCommits is a way to get just the commits where some data has been ingested.
- DataCommits() []*tiling.Commit
-
- // GetTile returns a simple tile either with or without ignored traces depending on
- // the argument.
- GetTile(is types.IgnoreState) *tiling.Tile
-}
diff --git a/golden/go/indexer/indexer.go b/golden/go/indexer/indexer.go
index de92b3f..7fdcd8f 100644
--- a/golden/go/indexer/indexer.go
+++ b/golden/go/indexer/indexer.go
@@ -13,9 +13,6 @@
"go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/tiling"
- "go.skia.org/infra/go/vcsinfo"
- "go.skia.org/infra/go/vcsinfo/bt_vcs"
- "go.skia.org/infra/golden/go/baseline"
"go.skia.org/infra/golden/go/blame"
"go.skia.org/infra/golden/go/diff"
"go.skia.org/infra/golden/go/digest_counter"
@@ -60,7 +57,6 @@
}
type searchIndexConfig struct {
- baseliner baseline.BaselineWriter
diffStore diff.DiffStore
expectationsStore expstorage.ExpectationsStore
gcsClient storage.GCSClient
@@ -151,7 +147,6 @@
}
type IndexerConfig struct {
- Baseliner baseline.BaselineWriter
DiffStore diff.DiffStore
EventBus eventbus.EventBus
ExpectationsStore expstorage.ExpectationsStore
@@ -167,11 +162,10 @@
type Indexer struct {
IndexerConfig
- pipeline *pdag.Node
- indexTestsNode *pdag.Node
- writeBaselineNode *pdag.Node
- lastIndex *SearchIndex
- mutex sync.RWMutex
+ pipeline *pdag.Node
+ indexTestsNode *pdag.Node
+ lastIndex *SearchIndex
+ mutex sync.RWMutex
}
// New returns a new Indexer instance. It synchronously indexes the initially
@@ -201,10 +195,6 @@
// ... and invoke the Blamer to calculate the blames.
blamerNode := indexTestsNode.Child(calcBlame)
- // Write baselines whenever a new tile is processed, new commits become available, or
- // when the expectations change.
- writeBaselineNode := indexTestsNode.Child(writeMasterBaseline)
-
// Parameters depend on DigestCounter.
paramsNodeInclude := pdag.NewNodeWithParents(calcParamsetsInclude, countsNodeInclude)
// These are run in parallel because they can take tens of seconds
@@ -227,7 +217,6 @@
ret.pipeline = root
ret.indexTestsNode = indexTestsNode
- ret.writeBaselineNode = writeBaselineNode
// Process the first tile and start the indexing process.
return ret, ret.start(interval)
@@ -266,17 +255,6 @@
expCh <- e.(*expstorage.EventExpectationChange).TestChanges
})
- // When the expectations of a Gerrit issue change then trigger pushing the
- // new expectations to GCS.
- ix.EventBus.SubscribeAsync(expstorage.EV_TRYJOB_EXP_CHANGED, ix.writeIssueBaseline)
-
- // When new commits have become available trigger writing the baselines. We choose size 100
- // because that is large enough to handle an unlikely torrent of commits being added to the repo.
- commitCh := make(chan []*vcsinfo.IndexCommit, 100)
- ix.EventBus.SubscribeAsync(bt_vcs.EV_NEW_GIT_COMMIT, func(e interface{}) {
- commitCh <- e.([]*vcsinfo.IndexCommit)
- })
-
// Keep building indices for different types of events. This is the central
// event loop of the indexer.
go func() {
@@ -295,10 +273,6 @@
case tn := <-expCh:
testChanges = append(testChanges, tn)
sklog.Infof("Indexer saw %d tests change", len(tn))
-
- // Catch changes in the commits.
- case <-commitCh:
- sklog.Infof("Indexer saw commits change")
}
// Drain all input channels, effectively bunching signals together that arrive in short
@@ -308,7 +282,6 @@
select {
case tn := <-expCh:
testChanges = append(testChanges, tn)
- case <-commitCh:
default:
break DrainLoop
}
@@ -323,10 +296,6 @@
} else if len(testChanges) > 0 {
// Only index the tests that have changed.
ix.indexTests(testChanges)
- } else {
- // At this point new commits have been discovered and we
- // just want to write the baselines.
- ix.writeBaselines()
}
}
}()
@@ -340,7 +309,6 @@
defer shared.NewMetricsTimer("indexer_execute_pipeline").Stop()
// Create a new index from the given tile.
sic := searchIndexConfig{
- baseliner: ix.Baseliner,
diffStore: ix.DiffStore,
expectationsStore: ix.ExpectationsStore,
gcsClient: ix.GCSClient,
@@ -349,14 +317,6 @@
return ix.pipeline.Trigger(newSearchIndex(sic, cpxTile))
}
-// writeBaselines triggers the node that causes baselines to be written to GCS.
-func (ix *Indexer) writeBaselines() {
- idx := ix.cloneLastIndex()
- if err := ix.writeBaselineNode.Trigger(idx); err != nil {
- sklog.Errorf("Error writing baselines: %s", err)
- }
-}
-
// indexTest creates an updated index by indexing the given list of expectation changes.
func (ix *Indexer) indexTests(testChanges []types.Expectations) {
// Get all the test names that had expectations changed.
@@ -385,7 +345,6 @@
func (ix *Indexer) cloneLastIndex() *SearchIndex {
lastIdx := ix.GetIndex()
sic := searchIndexConfig{
- baseliner: ix.Baseliner,
diffStore: ix.DiffStore,
expectationsStore: ix.ExpectationsStore,
gcsClient: ix.GCSClient,
@@ -421,26 +380,6 @@
return nil
}
-// writeIssueBaseline handles changes to baselines for Gerrit issues and dumps
-// the updated baseline to disk.
-func (ix *Indexer) writeIssueBaseline(evData interface{}) {
- if ix.Baseliner == nil || !ix.Baseliner.CanWriteBaseline() {
- return
- }
-
- issueID := evData.(*expstorage.EventExpectationChange).IssueID
- if issueID <= 0 {
- sklog.Errorf("Invalid issue id received for issue exp change: %d", issueID)
- return
- }
-
- idx := ix.GetIndex()
- if err := ix.Baseliner.PushIssueBaseline(issueID, idx.cpxTile, idx.dCounters[types.ExcludeIgnoredTraces]); err != nil {
- sklog.Errorf("Unable to push baseline for issue %d to GCS: %s", issueID, err)
- return
- }
-}
-
// calcDigestCountsInclude is the pipeline function to calculate DigestCounts from
// the full tile (not applying ignore rules)
func calcDigestCountsInclude(state interface{}) error {
@@ -561,29 +500,6 @@
return nil
}
-// writeMasterBaseline asynchronously writes the master baseline to GCS.
-func writeMasterBaseline(state interface{}) error {
- idx := state.(*SearchIndex)
-
- if idx.baseliner == nil {
- return nil
- }
-
- if !idx.baseliner.CanWriteBaseline() {
- sklog.Warning("Indexer tried to write baselines, but was not allowed to")
- return nil
- }
-
- // Write the baseline asynchronously.
- // TODO(kjlubick): Does this being asynchronous cause problems?
- go func() {
- if _, err := idx.baseliner.PushMasterBaselines(idx.cpxTile, ""); err != nil {
- sklog.Errorf("Error pushing master baseline to GCS: %s", err)
- }
- }()
- return nil
-}
-
// runWarmer is the pipeline function to run the warmer. It runs
// asynchronously since its results are not relevant for the searchIndex.
func runWarmer(state interface{}) error {
diff --git a/golden/go/indexer/indexer_test.go b/golden/go/indexer/indexer_test.go
index 3fbe4ea..4f0c157 100644
--- a/golden/go/indexer/indexer_test.go
+++ b/golden/go/indexer/indexer_test.go
@@ -26,14 +26,12 @@
func TestIndexerInitialTriggerSunnyDay(t *testing.T) {
unittest.SmallTest(t)
- mb := &mocks.BaselineWriter{}
mds := &mocks.DiffStore{}
mdw := &mocks.DiffWarmer{}
meb := &mock_eventbus.EventBus{}
mes := &mocks.ExpectationsStore{}
mgc := &mocks.GCSClient{}
- defer mb.AssertExpectations(t)
defer mds.AssertExpectations(t)
defer mdw.AssertExpectations(t)
defer meb.AssertExpectations(t)
@@ -43,22 +41,18 @@
ct, _, _ := makeComplexTileWithCrosshatchIgnores()
ic := IndexerConfig{
- Baseliner: mb,
DiffStore: mds,
EventBus: meb,
ExpectationsStore: mes,
GCSClient: mgc,
Warmer: mdw,
}
- wg, isAsync, asyncWrapper := testutils.AsyncHelpers()
+ wg, _, asyncWrapper := testutils.AsyncHelpers()
allTestDigests := types.DigestSlice{data.AlphaGood1Digest, data.AlphaBad1Digest, data.AlphaUntriaged1Digest,
data.BetaGood1Digest, data.BetaUntriaged1Digest}
sort.Sort(allTestDigests)
- mb.On("CanWriteBaseline").Return(true)
- isAsync(mb.On("PushMasterBaselines", ct, "")).Return(nil, nil)
-
mes.On("Get").Return(data.MakeTestExpectations(), nil)
// Return a non-empty map just to make sure things don't crash - this doesn't actually
@@ -142,12 +136,10 @@
func TestIndexerPartialUpdate(t *testing.T) {
unittest.SmallTest(t)
- mb := &mocks.BaselineWriter{}
mdw := &mocks.DiffWarmer{}
meb := &mock_eventbus.EventBus{}
mes := &mocks.ExpectationsStore{}
- defer mb.AssertExpectations(t)
defer mdw.AssertExpectations(t)
defer meb.AssertExpectations(t)
defer mes.AssertExpectations(t)
@@ -155,13 +147,10 @@
ct, fullTile, partialTile := makeComplexTileWithCrosshatchIgnores()
assert.NotEqual(t, fullTile, partialTile)
- wg, isAsync, asyncWrapper := testutils.AsyncHelpers()
+ wg, _, asyncWrapper := testutils.AsyncHelpers()
mes.On("Get").Return(data.MakeTestExpectations(), nil)
- mb.On("CanWriteBaseline").Return(true)
- isAsync(mb.On("PushMasterBaselines", ct, "")).Return(nil, nil)
-
meb.On("Publish", EV_INDEX_UPDATED, mock.AnythingOfType("*indexer.SearchIndex"), false).Return(nil)
// Make sure PrecomputeDiffs is only told to recompute BetaTest.
@@ -174,7 +163,6 @@
}))
ic := IndexerConfig{
- Baseliner: mb,
EventBus: meb,
ExpectationsStore: mes,
Warmer: mdw,
@@ -193,7 +181,6 @@
ixr.lastIndex = &SearchIndex{
searchIndexConfig: searchIndexConfig{
- baseliner: mb,
expectationsStore: mes,
warmer: mdw,
},
diff --git a/golden/go/mocks/BaselineWriter.go b/golden/go/mocks/BaselineWriter.go
deleted file mode 100644
index 0b29996..0000000
--- a/golden/go/mocks/BaselineWriter.go
+++ /dev/null
@@ -1,63 +0,0 @@
-// Code generated by mockery v1.0.0. DO NOT EDIT.
-
-package mocks
-
-import baseline "go.skia.org/infra/golden/go/baseline"
-import digest_counter "go.skia.org/infra/golden/go/digest_counter"
-import mock "github.com/stretchr/testify/mock"
-
-// BaselineWriter is an autogenerated mock type for the BaselineWriter type
-type BaselineWriter struct {
- mock.Mock
-}
-
-// CanWriteBaseline provides a mock function with given fields:
-func (_m *BaselineWriter) CanWriteBaseline() bool {
- ret := _m.Called()
-
- var r0 bool
- if rf, ok := ret.Get(0).(func() bool); ok {
- r0 = rf()
- } else {
- r0 = ret.Get(0).(bool)
- }
-
- return r0
-}
-
-// PushIssueBaseline provides a mock function with given fields: issueID, tileInfo, dCounter
-func (_m *BaselineWriter) PushIssueBaseline(issueID int64, tileInfo baseline.TileInfo, dCounter digest_counter.DigestCounter) error {
- ret := _m.Called(issueID, tileInfo, dCounter)
-
- var r0 error
- if rf, ok := ret.Get(0).(func(int64, baseline.TileInfo, digest_counter.DigestCounter) error); ok {
- r0 = rf(issueID, tileInfo, dCounter)
- } else {
- r0 = ret.Error(0)
- }
-
- return r0
-}
-
-// PushMasterBaselines provides a mock function with given fields: tileInfo, targetHash
-func (_m *BaselineWriter) PushMasterBaselines(tileInfo baseline.TileInfo, targetHash string) (*baseline.Baseline, error) {
- ret := _m.Called(tileInfo, targetHash)
-
- var r0 *baseline.Baseline
- if rf, ok := ret.Get(0).(func(baseline.TileInfo, string) *baseline.Baseline); ok {
- r0 = rf(tileInfo, targetHash)
- } else {
- if ret.Get(0) != nil {
- r0 = ret.Get(0).(*baseline.Baseline)
- }
- }
-
- var r1 error
- if rf, ok := ret.Get(1).(func(baseline.TileInfo, string) error); ok {
- r1 = rf(tileInfo, targetHash)
- } else {
- r1 = ret.Error(1)
- }
-
- return r0, r1
-}
diff --git a/golden/go/mocks/GCSClient.go b/golden/go/mocks/GCSClient.go
index 97ff013..21bf312 100644
--- a/golden/go/mocks/GCSClient.go
+++ b/golden/go/mocks/GCSClient.go
@@ -2,7 +2,6 @@
package mocks
-import baseline "go.skia.org/infra/golden/go/baseline"
import io "io"
import mock "github.com/stretchr/testify/mock"
import storage "go.skia.org/infra/golden/go/storage"
@@ -41,29 +40,6 @@
return r0
}
-// ReadBaseline provides a mock function with given fields: commitHash, issueID
-func (_m *GCSClient) ReadBaseline(commitHash string, issueID int64) (*baseline.Baseline, error) {
- ret := _m.Called(commitHash, issueID)
-
- var r0 *baseline.Baseline
- if rf, ok := ret.Get(0).(func(string, int64) *baseline.Baseline); ok {
- r0 = rf(commitHash, issueID)
- } else {
- if ret.Get(0) != nil {
- r0 = ret.Get(0).(*baseline.Baseline)
- }
- }
-
- var r1 error
- if rf, ok := ret.Get(1).(func(string, int64) error); ok {
- r1 = rf(commitHash, issueID)
- } else {
- r1 = ret.Error(1)
- }
-
- return r0, r1
-}
-
// RemoveForTestingOnly provides a mock function with given fields: targetPath
func (_m *GCSClient) RemoveForTestingOnly(targetPath string) error {
ret := _m.Called(targetPath)
@@ -78,27 +54,6 @@
return r0
}
-// WriteBaseline provides a mock function with given fields: b, commitHash
-func (_m *GCSClient) WriteBaseline(b *baseline.Baseline, commitHash string) (string, error) {
- ret := _m.Called(b, commitHash)
-
- var r0 string
- if rf, ok := ret.Get(0).(func(*baseline.Baseline, string) string); ok {
- r0 = rf(b, commitHash)
- } else {
- r0 = ret.Get(0).(string)
- }
-
- var r1 error
- if rf, ok := ret.Get(1).(func(*baseline.Baseline, string) error); ok {
- r1 = rf(b, commitHash)
- } else {
- r1 = ret.Error(1)
- }
-
- return r0, r1
-}
-
// WriteKnownDigests provides a mock function with given fields: digests
func (_m *GCSClient) WriteKnownDigests(digests types.DigestSlice) error {
ret := _m.Called(digests)
diff --git a/golden/go/mocks/TileInfo.go b/golden/go/mocks/TileInfo.go
deleted file mode 100644
index bac5379..0000000
--- a/golden/go/mocks/TileInfo.go
+++ /dev/null
@@ -1,60 +0,0 @@
-// Code generated by mockery v1.0.0. DO NOT EDIT.
-
-package mocks
-
-import mock "github.com/stretchr/testify/mock"
-import tiling "go.skia.org/infra/go/tiling"
-import types "go.skia.org/infra/golden/go/types"
-
-// TileInfo is an autogenerated mock type for the TileInfo type
-type TileInfo struct {
- mock.Mock
-}
-
-// AllCommits provides a mock function with given fields:
-func (_m *TileInfo) AllCommits() []*tiling.Commit {
- ret := _m.Called()
-
- var r0 []*tiling.Commit
- if rf, ok := ret.Get(0).(func() []*tiling.Commit); ok {
- r0 = rf()
- } else {
- if ret.Get(0) != nil {
- r0 = ret.Get(0).([]*tiling.Commit)
- }
- }
-
- return r0
-}
-
-// DataCommits provides a mock function with given fields:
-func (_m *TileInfo) DataCommits() []*tiling.Commit {
- ret := _m.Called()
-
- var r0 []*tiling.Commit
- if rf, ok := ret.Get(0).(func() []*tiling.Commit); ok {
- r0 = rf()
- } else {
- if ret.Get(0) != nil {
- r0 = ret.Get(0).([]*tiling.Commit)
- }
- }
-
- return r0
-}
-
-// GetTile provides a mock function with given fields: is
-func (_m *TileInfo) GetTile(is types.IgnoreState) *tiling.Tile {
- ret := _m.Called(is)
-
- var r0 *tiling.Tile
- if rf, ok := ret.Get(0).(func(types.IgnoreState) *tiling.Tile); ok {
- r0 = rf(is)
- } else {
- if ret.Get(0) != nil {
- r0 = ret.Get(0).(*tiling.Tile)
- }
- }
-
- return r0
-}
diff --git a/golden/go/mocks/generate.go b/golden/go/mocks/generate.go
index 152f22f..1f665f6 100644
--- a/golden/go/mocks/generate.go
+++ b/golden/go/mocks/generate.go
@@ -1,7 +1,6 @@
package mocks
//go:generate mockery -name BaselineFetcher -dir ../baseline/ -output .
-//go:generate mockery -name BaselineWriter -dir ../baseline/ -output .
//go:generate mockery -name ClosestDiffFinder -dir ../digesttools -output .
//go:generate mockery -name ComplexTile -dir ../types -output .
//go:generate mockery -name DiffStore -dir ../diff -output .
@@ -9,7 +8,6 @@
//go:generate mockery -name DigestCounter -dir ../digest_counter -output .
//go:generate mockery -name ExpectationsStore -dir ../expstorage -output .
//go:generate mockery -name GCSClient -dir ../storage -output .
-//go:generate mockery -name TileInfo -dir ../baseline -output .
//go:generate mockery -name TileSource -dir ../tilesource -output .
//go:generate mockery -name TraceStore -dir ../tracestore -output .
//go:generate mockery -name TryjobStore -dir ../tryjobstore -output .
diff --git a/golden/go/storage/gcsclient.go b/golden/go/storage/gcsclient.go
index f71cd14..57fb601 100644
--- a/golden/go/storage/gcsclient.go
+++ b/golden/go/storage/gcsclient.go
@@ -2,17 +2,14 @@
import (
"context"
- "encoding/json"
"fmt"
"io"
"net/http"
gstorage "cloud.google.com/go/storage"
"go.skia.org/infra/go/gcs"
- "go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/util"
- "go.skia.org/infra/golden/go/baseline"
"go.skia.org/infra/golden/go/types"
"google.golang.org/api/option"
)
@@ -22,10 +19,6 @@
// HashesGSPath is the bucket and path for storing the list of known digests.
HashesGSPath string
- // BaselineGSPath is the bucket and path for storing the baseline information.
- // This is considered to be a directory and will be used as such.
- BaselineGSPath string
-
// If DryRun is true, don't actually write the files (e.g. running locally)
Dryrun bool
}
@@ -35,15 +28,6 @@
// WriteKnownDigests writes the given list of digests to GCS as newline separated strings.
WriteKnownDigests(digests types.DigestSlice) error
- // WriteBaseline writes the given baseline for the given commit to GCS.
- // It returns the path of the written file in GCS (prefixed with 'gs://').
- // TODO(kjlubick): remove this when removing gcs_baseliner
- WriteBaseline(b *baseline.Baseline, commitHash string) (string, error)
-
- // ReadBaseline returns the baseline for the given issue from GCS.
- // TODO(kjlubick): remove this when removing gcs_baseliner
- ReadBaseline(commitHash string, issueID int64) (*baseline.Baseline, error)
-
// LoadKnownDigests loads the digests that have previously been written
// to GS via WriteKnownDigests. The digests should be copied to the
// provided writer 'w'.
@@ -99,74 +83,6 @@
return g.writeToPath(g.options.HashesGSPath, "text/plain", writeFn)
}
-// ReadBaseline fulfills the GCSClient interface.
-func (g *ClientImpl) WriteBaseline(b *baseline.Baseline, commitHash string) (string, error) {
- if g.options.Dryrun {
- sklog.Infof("dryrun: Writing baseline")
- outPath := g.getBaselinePath(commitHash, b.Issue)
- return "gs://" + outPath, nil
- }
- writeFn := func(w *gstorage.Writer) error {
- if err := json.NewEncoder(w).Encode(b); err != nil {
- return fmt.Errorf("Error encoding baseline to JSON: %s", err)
- }
- return nil
- }
-
- // We need a valid end commit or issue.
- if commitHash == "" && types.IsMasterBranch(b.Issue) {
- return "", skerr.Fmt("Received empty baseline commit and no issue. Cannot write baseline.")
- }
-
- outPath := g.getBaselinePath(commitHash, b.Issue)
- return "gs://" + outPath, g.writeToPath(outPath, "application/json", writeFn)
-}
-
-// ReadBaseline fulfills the GCSClient interface.
-func (g *ClientImpl) ReadBaseline(commitHash string, issueID int64) (*baseline.Baseline, error) {
- baselinePath := g.getBaselinePath(commitHash, issueID)
- bucketName, storagePath := gcs.SplitGSPath(baselinePath)
-
- ctx := context.Background()
- target := g.storageClient.Bucket(bucketName).Object(storagePath)
-
- _, err := target.Attrs(ctx)
- if err != nil {
- // If the item doesn't exist we return nil
- if err == gstorage.ErrObjectNotExist {
- return nil, nil
- }
- return nil, sklog.FmtErrorf("Error fetching attributes of baseline file: %s", err)
- }
-
- reader, err := target.NewReader(ctx)
- if err != nil {
- return nil, sklog.FmtErrorf("Error getting reader for baseline file: %s", err)
- }
- defer util.Close(reader)
-
- ret := &baseline.Baseline{}
- if err := json.NewDecoder(reader).Decode(ret); err != nil {
- return nil, sklog.FmtErrorf("Error decoding baseline file: %s", err)
- }
- return ret, nil
-}
-
-// getBaselinePath returns the baseline path in GCS for the given issueID.
-// This can also return the master baseline.
-func (g *ClientImpl) getBaselinePath(commitHash string, issueID int64) string {
- // Change the output file based on whether it's the master branch or a Gerrit issue.
- var outPath string
- if !types.IsMasterBranch(issueID) {
- outPath = fmt.Sprintf("issue_%d.json", issueID)
- } else if commitHash != "" {
- outPath = fmt.Sprintf("master_%s.json", commitHash)
- } else {
- outPath = "master.json"
- }
- return g.options.BaselineGSPath + "/" + outPath
-}
-
// LoadKnownDigests fulfills the GCSClient interface.
func (g *ClientImpl) LoadKnownDigests(w io.Writer) error {
bucketName, storagePath := gcs.SplitGSPath(g.options.HashesGSPath)
diff --git a/golden/go/storage/gcsclient_test.go b/golden/go/storage/gcsclient_test.go
index 8c6e50e..ea4a152 100644
--- a/golden/go/storage/gcsclient_test.go
+++ b/golden/go/storage/gcsclient_test.go
@@ -4,61 +4,17 @@
"bufio"
"bytes"
"fmt"
- "strings"
"testing"
"time"
assert "github.com/stretchr/testify/require"
"go.skia.org/infra/go/testutils/unittest"
- "go.skia.org/infra/go/tiling"
- "go.skia.org/infra/golden/go/baseline"
"go.skia.org/infra/golden/go/types"
)
const (
- // TEST_HASHES_GS_PATH is the bucket/path combination where the test file will be written.
- TEST_HASHES_GS_PATH = "skia-infra-testdata/hash_files/testing-known-hashes.txt"
-
- // TEST_BASELINE_GS_PATH is the root path of all baseline file in GCS.
- TEST_BASELINE_GS_PATH = "skia-infra-testdata/hash_files/testing-baselines"
-
- // Directory with testdata.
- TEST_DATA_DIR = "./testdata"
-
- // Local file location of the test data.
- TEST_DATA_PATH = TEST_DATA_DIR + "/10-test-sample-4bytes.tile"
-
- // Folder in the testdata bucket. See go/testutils for details.
- TEST_DATA_STORAGE_PATH = "gold-testdata/10-test-sample-4bytes.tile"
-)
-
-var (
- issueID = int64(5678)
-
- startCommit = &tiling.Commit{
- CommitTime: time.Now().Add(-time.Hour * 10).Unix(),
- Hash: "abb84b151a49eca5a6e107c51a1f1b7da73454bf",
- Author: "Jon Doe",
- }
- endCommit = &tiling.Commit{
- CommitTime: time.Now().Unix(),
- Hash: "51465f0ed60ce2cacff3653c7d1d70317679fc06",
- Author: "Jane Doe",
- }
-
- masterBaseline = &baseline.Baseline{
- Expectations: types.Expectations{
- "test-1": map[types.Digest]types.Label{"d1": types.POSITIVE},
- },
- Issue: types.MasterBranch,
- }
-
- issueBaseline = &baseline.Baseline{
- Expectations: types.Expectations{
- "test-3": map[types.Digest]types.Label{"d2": types.POSITIVE},
- },
- Issue: issueID,
- }
+ // hashesGCSPath is the bucket/path combination where the test file will be written.
+ hashesGCSPath = "skia-infra-testdata/hash_files/testing-known-hashes.txt"
)
func TestWritingHashes(t *testing.T) {
@@ -89,67 +45,10 @@
assert.Equal(t, knownDigests, found)
}
-func TestWritingBaselines(t *testing.T) {
- unittest.LargeTest(t)
-
- gsClient, _ := initGSClient(t)
- removePaths := []string{}
- defer func() {
- for _, path := range removePaths {
- _ = gsClient.RemoveForTestingOnly(path)
- }
- }()
-
- path, err := gsClient.WriteBaseline(masterBaseline, endCommit.Hash)
- assert.NoError(t, err)
- removePaths = append(removePaths, strings.TrimPrefix(path, "gs://"))
-
- foundBaseline, err := gsClient.ReadBaseline(endCommit.Hash, 0)
- assert.NoError(t, err)
- assert.Equal(t, masterBaseline, foundBaseline)
-
- // Add a baseline for an issue
- path, err = gsClient.WriteBaseline(issueBaseline, "")
- assert.NoError(t, err)
- removePaths = append(removePaths, strings.TrimPrefix(path, "gs://"))
-
- foundBaseline, err = gsClient.ReadBaseline("", issueID)
- assert.NoError(t, err)
- assert.Equal(t, issueBaseline, foundBaseline)
-}
-
-func TestBaselineRobustness(t *testing.T) {
- unittest.LargeTest(t)
-
- gsClient, _ := initGSClient(t)
-
- removePaths := []string{}
- defer func() {
- for _, path := range removePaths {
- _ = gsClient.RemoveForTestingOnly(path)
- }
- }()
-
- // Read the master baseline that has not been written
- foundBaseline, err := gsClient.ReadBaseline("", 5344)
- assert.NoError(t, err)
- assert.Nil(t, foundBaseline)
-
- // Test reading a non-existing baseline for an issue
- foundBaseline, err = gsClient.ReadBaseline("", 5344)
- assert.NoError(t, err)
- assert.Nil(t, foundBaseline)
-
- path, err := gsClient.WriteBaseline(masterBaseline, endCommit.Hash)
- assert.NoError(t, err)
- removePaths = append(removePaths, strings.TrimPrefix(path, "gs://"))
-}
-
func initGSClient(t *testing.T) (GCSClient, GCSClientOptions) {
timeStamp := fmt.Sprintf("%032d", time.Now().UnixNano())
opt := GCSClientOptions{
- HashesGSPath: TEST_HASHES_GS_PATH + "-" + timeStamp,
- BaselineGSPath: TEST_BASELINE_GS_PATH + "-" + timeStamp,
+ HashesGSPath: hashesGCSPath + "-" + timeStamp,
}
gsClient, err := NewGCSClient(nil, opt)
assert.NoError(t, err)
diff --git a/golden/k8s-config-templates/gold-common.json5 b/golden/k8s-config-templates/gold-common.json5
index 29b1e1d..f424c0a 100644
--- a/golden/k8s-config-templates/gold-common.json5
+++ b/golden/k8s-config-templates/gold-common.json5
@@ -1,5 +1,5 @@
{
- "BASELINE_SERVER_IMAGE": "gcr.io/skia-public/gold-baseline-server:2019-08-09T11_49_17Z-kjlubick-b195d18-clean",
+ "BASELINE_SERVER_IMAGE": "gcr.io/skia-public/gold-baseline-server:2019-08-13T00_34_10Z-kjlubick-903d961-clean",
"DIFF_SERVER_IMAGE": "gcr.io/skia-public/gold-diff-server:2019-05-10T16_57_45Z-kjlubick-a01b41f-clean",
"INGESTION_IMAGE": "gcr.io/skia-public/gold-ingestion:2019-05-09T17_23_16Z-kjlubick-e814534-clean",
"INGESTION_BT_IMAGE": "gcr.io/skia-public/gold-ingestion:2019-08-08T17_37_52Z-kjlubick-65e0aa3-clean",
