[gold] goldctl and backend fixes
- Added goldctl flags for testing
- Synchronized backend and front-end URLs and data structures
- Factored out shared package to hold code used by the frontend
and the backend and to reduce some dependencies not needed by goldctl
(still many dependencies remain)
Bug: skia:8293
Change-Id: I77130c33da38973c680a8e3f1ad867420b0d9c91
Reviewed-on: https://skia-review.googlesource.com/c/178869
Commit-Queue: Stephan Altmueller <stephana@google.com>
Reviewed-by: Ben Wagner <benjaminwagner@google.com>
diff --git a/gold-client/cmd/goldctl/cmd_imgtest.go b/gold-client/cmd/goldctl/cmd_imgtest.go
index 45b6418..132719c 100644
--- a/gold-client/cmd/goldctl/cmd_imgtest.go
+++ b/gold-client/cmd/goldctl/cmd_imgtest.go
@@ -7,7 +7,7 @@
"github.com/spf13/cobra"
"go.skia.org/infra/gold-client/go/goldclient"
"go.skia.org/infra/golden/go/jsonio"
- "go.skia.org/infra/golden/go/search"
+ "go.skia.org/infra/golden/go/shared"
)
// imgTestEnv is the environment for the imgtest command ant its sub-commands.
@@ -22,6 +22,7 @@
flagWorkDir string
flagPassFailStep bool
flagFailureFile string
+ flagURL string
// Flags used by imgtest:add
flagTestName string
@@ -105,6 +106,7 @@
cmd.Flags().StringVarP(&i.flagPatchsetID, "patchset", "", "", "Gerrit patchset number if this is a trybot run. ")
cmd.Flags().StringVarP(&i.flagJobID, "jobid", "", "", "Job ID if this is a tryjob run. Current the BuildBucket id.")
cmd.Flags().StringVarP(&i.flagFailureFile, "failure-file", "", "", "Path to the file where to write failure information")
+ cmd.Flags().StringVarP(&i.flagURL, "url", "", "", "URL of the Gold instance. Used for testing, if empty the URL will be derived from the value of 'instance'")
if !optional {
_ = cmd.MarkFlagRequired("instance")
@@ -125,12 +127,13 @@
keyMap, err := readKeysFile(i.flagKeysFile)
ifErrLogExit(cmd, err)
- validation := search.Validation{}
+ validation := shared.Validation{}
issueID := validation.Int64Value("issue", i.flagIssueID, 0)
patchsetID := validation.Int64Value("patchset", i.flagPatchsetID, 0)
jobID := validation.Int64Value("jobid", i.flagJobID, 0)
ifErrLogExit(cmd, validation.Errors())
+ // Define the meta data of the result that is shared by all tests.
gr := &jsonio.GoldResults{
GitHash: i.flagCommit,
Key: keyMap,
@@ -139,15 +142,23 @@
BuildBucketID: jobID,
}
- goldClient, err := goldclient.NewCloudClient(i.flagWorkDir, i.flagInstanceID, i.flagPassFailStep, gr)
+ config := &goldclient.GoldClientConfig{
+ InstanceID: i.flagInstanceID,
+ WorkDir: i.flagWorkDir,
+ PassFailStep: i.flagPassFailStep,
+ OverrideGoldURL: i.flagURL,
+ }
+ goldClient, err := goldclient.NewCloudClient(config, gr)
ifErrLogExit(cmd, err)
pass, err := goldClient.Test(i.flagTestName, i.flagPNGFile)
ifErrLogExit(cmd, err)
if !pass {
+ logErrf(cmd, "Test: %s FAIL\n", i.flagTestName)
os.Exit(1)
}
+ logInfof(cmd, "Test: %s PASS\n", i.flagTestName)
os.Exit(0)
}
diff --git a/gold-client/go/goldclient/goldclient.go b/gold-client/go/goldclient/goldclient.go
index cd0fc6e..05b2fb3 100644
--- a/gold-client/go/goldclient/goldclient.go
+++ b/gold-client/go/goldclient/goldclient.go
@@ -20,20 +20,35 @@
"go.skia.org/infra/go/httputils"
"go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/util"
+ "go.skia.org/infra/golden/go/baseline"
"go.skia.org/infra/golden/go/diff"
"go.skia.org/infra/golden/go/jsonio"
+ "go.skia.org/infra/golden/go/shared"
"go.skia.org/infra/golden/go/types"
- "go.skia.org/infra/golden/go/web"
)
const (
- resultPrefix = "dm-json-v1"
- imagePrefix = "dm-images-v1"
- knownHashesURLPath = "_/hashes"
+ // resultPrefix is the path prefix in the GCS bucket that holds JSON result files
+ resultPrefix = "dm-json-v1"
- resultStateFile = "result-state.json"
- jsonTempFileName = "dm.json"
- fetchTempFileName = "hashes.txt"
+ // imagePrefix is the path prefix in the GCS bucket that holds images.
+ imagePrefix = "dm-images-v1"
+
+ // knownHashesURLPath is path on the Gold instance to retrieve the known image hashes that do
+ // not need to be uploaded anymore.
+ knownHashesURLPath = "json/hashes"
+
+ // goldURLTmp constructs the URL of the Gold instance from the instance id
+ goldURLTmpl = "https://%s-gold.skia.org"
+
+ // bucketNameTmpl constructs the name of the ingestion bucket from the instance id
+ bucketNameTmpl = "skia-gold-%s"
+
+ // resultStateFile is the name of the file that holds the state in the work directory between calls
+ resultStateFile = "result-state.json"
+
+ // jsonTempFileName is the temporary file that is created to upload results via gsutil.
+ jsonTempFileName = "gsutil_dm.json"
)
// md5Regexp is used to check whether strings are MD5 hashes.
@@ -64,23 +79,34 @@
httpClient *http.Client
}
+// GoldClientConfig is a config structure to configure GoldClient instances
+type GoldClientConfig struct {
+ // WorkDir is a temporary directory that caches data for one run with multiple calls to GoldClient
+ WorkDir string
+
+ // InstanceID is the id of the backend Gold instance
+ InstanceID string
+
+ // PassFailStep indicates whether each call to Test(...) should return a pass/fail value.
+ PassFailStep bool
+
+ // OverrideGoldURL is optional and allows to override the GoldURL for testing.
+ OverrideGoldURL string
+}
+
// NewCloudClient returns an implementation of the GoldClient that relies on the Gold service.
// Arguments:
-// - workDir : is a temporary work directory that needs to be available during the entire
-// testrun (over multiple calls to 'Test')
-// - instanceID: is the id of the Gold instance.
-// - passFailStep: indicates whether each individual call to Test needs to return pass fail.
// - goldResults: A populated instance of jsonio.GoldResults that contains configuration
// shared by all tests.
//
// If a new instance is created for each call to Test, the arguments of the first call are
// preserved. They are cached in a JSON file in the work directory.
-func NewCloudClient(workDir, instanceID string, passFailStep bool, goldResult *jsonio.GoldResults) (GoldClient, error) {
+func NewCloudClient(config *GoldClientConfig, goldResult *jsonio.GoldResults) (GoldClient, error) {
// Make sure the workdir was given and exists.
- if workDir == "" {
+ if config.WorkDir == "" {
return nil, skerr.Fmt("No 'workDir' provided to NewCloudClient")
}
- workDir, err := filepath.Abs(workDir)
+ workDir, err := filepath.Abs(config.WorkDir)
if err != nil {
return nil, err
}
@@ -102,7 +128,7 @@
httpClient: httputils.DefaultClientConfig().Client(),
}
- if err := ret.initResultState(instanceID, passFailStep, goldResult); err != nil {
+ if err := ret.initResultState(config, goldResult); err != nil {
return nil, skerr.Fmt("Error initializing result in cloud GoldClient: %s", err)
}
@@ -165,7 +191,7 @@
// initResultState assembles the information that needs to be uploaded based on previous calls
// to the function and new arguments.
-func (c *cloudClient) initResultState(instanceID string, passFailStep bool, goldResult *jsonio.GoldResults) error {
+func (c *cloudClient) initResultState(config *GoldClientConfig, goldResult *jsonio.GoldResults) error {
// Load the state from the workdir.
var err error
c.resultState, err = loadStateFromJson(c.getResultStateFile())
@@ -180,7 +206,7 @@
// Create a new instance of result state. Setting freshState to true indicates that this needs
// to be stored to disk once a test has been added successfully.
- c.resultState, err = newResultState(goldResult, passFailStep, instanceID, c.workDir, c.httpClient)
+ c.resultState, err = newResultState(goldResult, config, c.workDir, c.httpClient)
if err != nil {
return err
}
@@ -274,18 +300,24 @@
}
// newResultState creates a new instance resultState and downloads the relevant files from Gold.
-func newResultState(goldResult *jsonio.GoldResults, passFailStep bool, instanceID, workDir string, httpClient *http.Client) (*resultState, error) {
+func newResultState(goldResult *jsonio.GoldResults, config *GoldClientConfig, workDir string, httpClient *http.Client) (*resultState, error) {
// TODO(stephana): Move deriving the URLs and the bucket to a central place in the backend
// or get rid of the bucket entirely and expose an upload URL (requires authentication)
+ goldURL := config.OverrideGoldURL
+ if goldURL == "" {
+ goldURL = fmt.Sprintf(goldURLTmpl, config.InstanceID)
+ }
+
ret := &resultState{
GoldResults: goldResult,
- PerTestPassFail: passFailStep,
- InstanceID: instanceID,
- GoldURL: fmt.Sprintf("https://%s-gold.skia.org", instanceID),
- Bucket: fmt.Sprintf("skia-gold-%s", instanceID),
+ PerTestPassFail: config.PassFailStep,
+ InstanceID: config.InstanceID,
+ GoldURL: goldURL,
+ Bucket: fmt.Sprintf(bucketNameTmpl, config.InstanceID),
workDir: workDir,
+ httpClient: httpClient,
}
if err := ret.loadKnownHashes(); err != nil {
@@ -375,16 +407,17 @@
var urlPath string
if r.GoldResults.Issue > 0 {
issueID := strconv.FormatInt(r.GoldResults.Issue, 10)
- urlPath = strings.Replace(web.EXPECATIONS_ISSUE_ROUTE, "{issue_id}", issueID, 1)
+ urlPath = strings.Replace(shared.EXPECATIONS_ISSUE_ROUTE, "{issue_id}", issueID, 1)
} else {
- urlPath = strings.Replace(web.EXPECATIONS_ROUTE, "{commit_hash}", r.GoldResults.GitHash, 1)
+ urlPath = strings.Replace(shared.EXPECATIONS_ROUTE, "{commit_hash}", r.GoldResults.GitHash, 1)
}
- url := fmt.Sprintf("%s/%s", r.GoldURL, urlPath)
+ url := fmt.Sprintf("%s/%s", r.GoldURL, strings.TrimLeft(urlPath, "/"))
resp, err := r.httpClient.Get(url)
if err != nil {
return err
}
+
jsonBytes, err := ioutil.ReadAll(resp.Body)
if err != nil {
return skerr.Fmt("Error reading body of request to %s: %s", url, err)
@@ -393,7 +426,14 @@
return skerr.Fmt("Error closing response from request to %s: %s", url, err)
}
- return json.Unmarshal(jsonBytes, &r.Expectations)
+ exp := &baseline.CommitableBaseLine{}
+
+ if err := json.Unmarshal(jsonBytes, exp); err != nil {
+ return skerr.Fmt("Error parsing JSON: %s", err)
+ }
+
+ r.Expectations = exp.Baseline
+ return nil
}
// getResultFilePath returns that path in GCS where the result file should be stored.
@@ -409,7 +449,7 @@
// Assemble a path that looks like this:
// <path_prefix>/YYYY/MM/DD/HH/<git_hash>/<build_id>/<time_stamp>/<per_run_file_name>.json
- // The first segements up to 'HH' are required so the Gold ingester can scan these prefixes for
+ // The first segments up to 'HH' are required so the Gold ingester can scan these prefixes for
// new files. The later segments are necessary to make the path unique within the runs of one
// hour and increase readability of the paths for troubleshooting.
// It is vital that the times segments of the path are based on UTC location.
diff --git a/golden/cmd/baseline_server/main.go b/golden/cmd/baseline_server/main.go
index 063967e..5f8e8b5 100644
--- a/golden/cmd/baseline_server/main.go
+++ b/golden/cmd/baseline_server/main.go
@@ -15,6 +15,7 @@
"go.skia.org/infra/go/httputils"
"go.skia.org/infra/go/skiaversion"
"go.skia.org/infra/go/sklog"
+ "go.skia.org/infra/golden/go/shared"
"go.skia.org/infra/golden/go/storage"
"go.skia.org/infra/golden/go/web"
gstorage "google.golang.org/api/storage/v1"
@@ -60,8 +61,8 @@
router := mux.NewRouter()
// Retrieving that baseline for master and an Gerrit issue are handled the same way
- router.HandleFunc(web.EXPECATIONS_ROUTE, handlers.JsonBaselineHandler).Methods("GET")
- router.HandleFunc(web.EXPECATIONS_ISSUE_ROUTE, handlers.JsonBaselineHandler).Methods("GET")
+ router.HandleFunc(shared.EXPECATIONS_ROUTE, handlers.JsonBaselineHandler).Methods("GET")
+ router.HandleFunc(shared.EXPECATIONS_ISSUE_ROUTE, handlers.JsonBaselineHandler).Methods("GET")
// Start the server
sklog.Infof("Serving on http://127.0.0.1" + *port)
diff --git a/golden/cmd/skiacorrectness/main.go b/golden/cmd/skiacorrectness/main.go
index 6640e8d..b7217b7 100644
--- a/golden/cmd/skiacorrectness/main.go
+++ b/golden/cmd/skiacorrectness/main.go
@@ -42,6 +42,7 @@
"go.skia.org/infra/golden/go/ignore"
"go.skia.org/infra/golden/go/indexer"
"go.skia.org/infra/golden/go/search"
+ "go.skia.org/infra/golden/go/shared"
"go.skia.org/infra/golden/go/status"
"go.skia.org/infra/golden/go/storage"
"go.skia.org/infra/golden/go/tryjobs"
@@ -424,45 +425,57 @@
router.PathPrefix("/res/").HandlerFunc(web.MakeResourceHandler(*resourcesDir))
router.HandleFunc(OAUTH2_CALLBACK_PATH, login.OAuth2CallbackHandler)
+ // TODO(stephana): remove "/_/hashes" in favor of "/json/hashes" once all clients have switched.
+
// /_/hashes is used by the bots to find hashes it does not need to upload.
router.HandleFunc("/_/hashes", handlers.TextAllHashesHandler).Methods("GET")
router.HandleFunc("/json/version", skiaversion.JsonHandler)
router.HandleFunc("/loginstatus/", login.StatusHandler)
router.HandleFunc("/logout/", login.LogoutHandler)
- // json handlers only used by the new UI.
- router.HandleFunc("/json/byblame", handlers.JsonByBlameHandler).Methods("GET")
- router.HandleFunc("/json/list", handlers.JsonListTestsHandler).Methods("GET")
- router.HandleFunc("/json/paramset", handlers.JsonParamsHandler).Methods("GET")
- router.HandleFunc("/json/commits", handlers.JsonCommitsHandler).Methods("GET")
- router.HandleFunc("/json/diff", handlers.JsonDiffHandler).Methods("GET")
- router.HandleFunc("/json/details", handlers.JsonDetailsHandler).Methods("GET")
- router.HandleFunc("/json/triage", handlers.JsonTriageHandler).Methods("POST")
- router.HandleFunc("/json/clusterdiff", handlers.JsonClusterDiffHandler).Methods("GET")
- router.HandleFunc("/json/cmp", handlers.JsonCompareTestHandler).Methods("POST")
- router.HandleFunc("/json/triagelog", handlers.JsonTriageLogHandler).Methods("GET")
- router.HandleFunc("/json/triagelog/undo", handlers.JsonTriageUndoHandler).Methods("POST")
- router.HandleFunc("/json/failure", handlers.JsonListFailureHandler).Methods("GET")
- router.HandleFunc("/json/failure/clear", handlers.JsonClearFailureHandler).Methods("POST")
- router.HandleFunc("/json/cleardigests", handlers.JsonClearDigests).Methods("POST")
- router.HandleFunc("/json/search", handlers.JsonSearchHandler).Methods("GET")
- router.HandleFunc("/json/export", handlers.JsonExportHandler).Methods("GET")
- router.HandleFunc("/json/tryjob", handlers.JsonTryjobListHandler).Methods("GET")
- router.HandleFunc("/json/tryjob/{id}", handlers.JsonTryjobSummaryHandler).Methods("GET")
+ // Set up a subrouter for the '/json' routes which make up the Gold API.
+ // This makes routing faster, but also returns a failure when an /json route is
+ // requested that doesn't exit.
+ jsonRouter := router.PathPrefix("/json").Subrouter()
+ trim := func(r string) string { return strings.TrimPrefix(r, "/json") }
+
+ jsonRouter.HandleFunc(trim("/json/hashes"), handlers.TextAllHashesHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/byblame"), handlers.JsonByBlameHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/list"), handlers.JsonListTestsHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/paramset"), handlers.JsonParamsHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/commits"), handlers.JsonCommitsHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/diff"), handlers.JsonDiffHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/details"), handlers.JsonDetailsHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/triage"), handlers.JsonTriageHandler).Methods("POST")
+ jsonRouter.HandleFunc(trim("/json/clusterdiff"), handlers.JsonClusterDiffHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/cmp"), handlers.JsonCompareTestHandler).Methods("POST")
+ jsonRouter.HandleFunc(trim("/json/triagelog"), handlers.JsonTriageLogHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/triagelog/undo"), handlers.JsonTriageUndoHandler).Methods("POST")
+ jsonRouter.HandleFunc(trim("/json/failure"), handlers.JsonListFailureHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/failure/clear"), handlers.JsonClearFailureHandler).Methods("POST")
+ jsonRouter.HandleFunc(trim("/json/cleardigests"), handlers.JsonClearDigests).Methods("POST")
+ jsonRouter.HandleFunc(trim("/json/search"), handlers.JsonSearchHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/export"), handlers.JsonExportHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/tryjob"), handlers.JsonTryjobListHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/tryjob/{id}"), handlers.JsonTryjobSummaryHandler).Methods("GET")
// Retrieving that baseline for master and an Gerrit issue are handled the same way
- router.HandleFunc(web.EXPECATIONS_ROUTE, handlers.JsonBaselineHandler).Methods("GET")
- router.HandleFunc(web.EXPECATIONS_ISSUE_ROUTE, handlers.JsonBaselineHandler).Methods("GET")
- router.HandleFunc("/json/refresh/{id}", handlers.JsonRefreshIssue).Methods("GET")
+ jsonRouter.HandleFunc(trim(shared.EXPECATIONS_ROUTE), handlers.JsonBaselineHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim(shared.EXPECATIONS_ISSUE_ROUTE), handlers.JsonBaselineHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/refresh/{id}"), handlers.JsonRefreshIssue).Methods("GET")
// Only expose these endpoints if login is enforced across the app or this an open site.
if openSite {
- router.HandleFunc("/json/ignores", handlers.JsonIgnoresHandler).Methods("GET")
- router.HandleFunc("/json/ignores/add/", handlers.JsonIgnoresAddHandler).Methods("POST")
- router.HandleFunc("/json/ignores/del/{id}", handlers.JsonIgnoresDeleteHandler).Methods("POST")
- router.HandleFunc("/json/ignores/save/{id}", handlers.JsonIgnoresUpdateHandler).Methods("POST")
+ jsonRouter.HandleFunc(trim("/json/ignores"), handlers.JsonIgnoresHandler).Methods("GET")
+ jsonRouter.HandleFunc(trim("/json/ignores/add/"), handlers.JsonIgnoresAddHandler).Methods("POST")
+ jsonRouter.HandleFunc(trim("/json/ignores/del/{id}"), handlers.JsonIgnoresDeleteHandler).Methods("POST")
+ jsonRouter.HandleFunc(trim("/json/ignores/save/{id}"), handlers.JsonIgnoresUpdateHandler).Methods("POST")
}
+ // Make sure we return a 404 for anything that starts with /json and could not be found.
+ jsonRouter.HandleFunc("/{ignore:.*}", http.NotFound)
+ router.HandleFunc("/json", http.NotFound)
+
// For everything else serve the same markup.
indexFile := *resourcesDir + "/index.html"
indexTemplate := template.Must(template.New("").ParseFiles(indexFile)).Lookup("index.html")
diff --git a/golden/go/baseline/baseline.go b/golden/go/baseline/baseline.go
index baacb22..e3ac105 100644
--- a/golden/go/baseline/baseline.go
+++ b/golden/go/baseline/baseline.go
@@ -34,8 +34,13 @@
// in tile.
func GetBaselineForMaster(exps types.Expectations, tile *tiling.Tile) *CommitableBaseLine {
commits := tile.Commits
- var startCommit *tiling.Commit = nil
- var endCommit *tiling.Commit = nil
+ if len(tile.Commits) == 0 {
+ return nil
+ }
+
+ // Set the start and end commit in case there are no traces
+ var startCommit *tiling.Commit = tile.Commits[0]
+ var endCommit *tiling.Commit = tile.Commits[len(tile.Commits)-1]
masterBaseline := types.TestExp{}
for _, trace := range tile.Traces {
diff --git a/golden/go/diff/diff.go b/golden/go/diff/diff.go
index 3f63ce6..84bb847 100644
--- a/golden/go/diff/diff.go
+++ b/golden/go/diff/diff.go
@@ -138,7 +138,7 @@
// ImageHandler returns a http.Handler for the given path prefix. The caller
// can then serve images of the format:
// <urlPrefix>/images/<digests>.png
- // <irlPrefix>/diffs/<digest1>-<digests2>.png
+ // <urlPrefix>/diffs/<digest1>-<digests2>.png
ImageHandler(urlPrefix string) (http.Handler, error)
// WarmDigest will fetch the given digests. If sync is true the call will
diff --git a/golden/go/search/parse.go b/golden/go/search/parse.go
index c51d001..f7d0b49 100644
--- a/golden/go/search/parse.go
+++ b/golden/go/search/parse.go
@@ -6,11 +6,10 @@
"io"
"net/http"
"net/url"
- "strconv"
- "strings"
"go.skia.org/infra/go/util"
"go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/shared"
"go.skia.org/infra/golden/go/types"
)
@@ -85,7 +84,7 @@
}
ctQuery.ColumnQuery.Limit = util.MinInt32(ctQuery.ColumnQuery.Limit, MAX_LIMIT)
- validate := Validation{}
+ validate := shared.Validation{}
// Parse the patchsets.
ctQuery.ColumnQuery.Patchsets = validate.Int64SliceValue("patchsets", ctQuery.ColumnQuery.PatchsetsStr, nil)
@@ -105,123 +104,6 @@
// TODO(stephana): Validation should be factored out into a separate package.
-// Validation is a container to collect error messages during validation of a
-// input with multiple fields.
-type Validation []string
-
-// StrValue validates a string value against containment in a set of options.
-// Argument:
-// name: name of the field being validated.
-// val: value to be validated.
-// options: list of options, one of which value can contain.
-// defaultVal: default value in case val is empty. Can be equal to "".
-// If there is a problem an error message will be added to the Validation object.
-func (v *Validation) StrValue(name string, val *string, options []string, defaultVal string) {
- if *val == "" && defaultVal != "" {
- *val = defaultVal
- return
- }
- if !util.In(*val, options) {
- *v = append(*v, fmt.Sprintf("Field '%s' needs to be one of '%s'", name, strings.Join(options, ",")))
- }
-}
-
-// StrFormValue does the same as StrValue but extracts the given name from
-// the request via r.FormValue(..).
-func (v *Validation) StrFormValue(r *http.Request, name string, val *string, options []string, defaultVal string) {
- *val = r.FormValue(name)
- v.StrValue(name, val, options, defaultVal)
-}
-
-// Float64Value parses the value given in strVal and returns it. If strVal is empty
-// the default value is returned.
-func (v *Validation) Float64Value(name string, strVal string, defaultVal float64) float64 {
- if strVal == "" {
- return defaultVal
- }
-
- tempVal, err := strconv.ParseFloat(strVal, 64)
- if err != nil {
- *v = append(*v, fmt.Sprintf("Field '%s' is not a valid float: %s", name, err))
- }
- return tempVal
-}
-
-// Int64Value parses the value given in strVal and returns it. If strVal is empty
-// the default value is returned.
-func (v *Validation) Int64Value(name string, strVal string, defaultVal int64) int64 {
- if strVal == "" {
- return defaultVal
- }
-
- tempVal, err := strconv.ParseInt(strVal, 10, 64)
- if err != nil {
- *v = append(*v, fmt.Sprintf("Field '%s' is not a valid int: %s", name, err))
- }
- return tempVal
-}
-
-// Float64FormValue does the same as Float64Value but extracts the value from the request object.
-func (v *Validation) Float64FormValue(r *http.Request, name string, defaultVal float64) float64 {
- return v.Float64Value(name, r.FormValue(name), defaultVal)
-}
-
-// Int64FormValue does the same as Int64Value but extracts the value from the request object.
-func (v *Validation) Int64FormValue(r *http.Request, name string, defaultVal int64) int64 {
- return v.Int64Value(name, r.FormValue(name), defaultVal)
-}
-
-// Int64SliceValue parses a comma-separated list of int values and returns them.
-func (v *Validation) Int64SliceValue(name string, strVal string, defaultVal []int64) []int64 {
- if strVal == "" {
- return defaultVal
- }
-
- splitVals := strings.Split(strVal, ",")
- ret := make([]int64, 0, len(splitVals))
- for _, oneStrVal := range splitVals {
- tempVal, err := strconv.ParseInt(oneStrVal, 10, 64)
- if err != nil {
- *v = append(*v, fmt.Sprintf("Field '%s' is not a valid list of comma separated integers: %s", name, err))
- return nil
- }
- ret = append(ret, tempVal)
- }
- return ret
-}
-
-// Int64SliceFormValue does the same as Int64SliceValue but extracts the given
-// name from the request.
-func (v *Validation) Int64SliceFormValue(r *http.Request, name string, defaultVal []int64) []int64 {
- return v.Int64SliceValue(name, r.FormValue(name), defaultVal)
-}
-
-// QueryFormValue extracts a URL-encoded query from the form values and decodes it.
-// If the named field was not available in the given request an empty set url.Values
-// is returned. If an error occurs it will be added to the error list of the validation
-// object.
-func (v *Validation) QueryFormValue(r *http.Request, name string) map[string][]string {
- if q := r.FormValue(name); q != "" {
- ret, err := url.ParseQuery(q)
- if err != nil {
- *v = append(*v, fmt.Sprintf("Unable to parse query: %s. Error: %s", q, err))
- return nil
- }
- return ret
- }
- return map[string][]string{}
-}
-
-// Errors returns a concatenation of all error values accumulated in validation or nil
-// if there were no errors.
-func (v *Validation) Errors() error {
- if len(*v) == 0 {
- return nil
- }
-
- return fmt.Errorf("%s", strings.Join(*v, "\n"))
-}
-
// ParseQuery parses the request parameters from the URL query string or from the
// form parameters and stores the parsed and validated values in query.
func ParseQuery(r *http.Request, query *Query) error {
@@ -240,7 +122,7 @@
query.Match = []string{types.PRIMARY_KEY_FIELD}
}
- validate := Validation{}
+ validate := shared.Validation{}
// Parse the query strings. Note Query and RQuery have different types, but the
// same underlying type: map[string][]string
diff --git a/golden/go/shared/shared.go b/golden/go/shared/shared.go
new file mode 100644
index 0000000..34c05b6
--- /dev/null
+++ b/golden/go/shared/shared.go
@@ -0,0 +1,139 @@
+// Package shared contains constants and functions shared by various backend servers
+// and frontend clients like goldctl.
+package shared
+
+import (
+ "fmt"
+ "net/http"
+ "net/url"
+ "strconv"
+ "strings"
+
+ "go.skia.org/infra/go/util"
+)
+
+// Define common routes used by multiple servers and goldctl
+const (
+ // BASELINE_ROUTE serves the expectations of the master branch
+ EXPECATIONS_ROUTE = "/json/expecations/commit/{commit_hash}"
+
+ // BASELINE_ISSUE_ROUTE serves the baseline for the Gerrit CL identified by 'id'
+ EXPECATIONS_ISSUE_ROUTE = "/json/expecations/issue/{issue_id}"
+)
+
+// Validation is a container to collect error messages during validation of a
+// input with multiple fields.
+type Validation []string
+
+// StrValue validates a string value against containment in a set of options.
+// Argument:
+// name: name of the field being validated.
+// val: value to be validated.
+// options: list of options, one of which value can contain.
+// defaultVal: default value in case val is empty. Can be equal to "".
+// If there is a problem an error message will be added to the Validation object.
+func (v *Validation) StrValue(name string, val *string, options []string, defaultVal string) {
+ if *val == "" && defaultVal != "" {
+ *val = defaultVal
+ return
+ }
+ if !util.In(*val, options) {
+ *v = append(*v, fmt.Sprintf("Field '%s' needs to be one of '%s'", name, strings.Join(options, ",")))
+ }
+}
+
+// StrFormValue does the same as StrValue but extracts the given name from
+// the request via r.FormValue(..).
+func (v *Validation) StrFormValue(r *http.Request, name string, val *string, options []string, defaultVal string) {
+ *val = r.FormValue(name)
+ v.StrValue(name, val, options, defaultVal)
+}
+
+// Float64Value parses the value given in strVal and returns it. If strVal is empty
+// the default value is returned.
+func (v *Validation) Float64Value(name string, strVal string, defaultVal float64) float64 {
+ if strVal == "" {
+ return defaultVal
+ }
+
+ tempVal, err := strconv.ParseFloat(strVal, 64)
+ if err != nil {
+ *v = append(*v, fmt.Sprintf("Field '%s' is not a valid float: %s", name, err))
+ }
+ return tempVal
+}
+
+// Int64Value parses the value given in strVal and returns it. If strVal is empty
+// the default value is returned.
+func (v *Validation) Int64Value(name string, strVal string, defaultVal int64) int64 {
+ if strVal == "" {
+ return defaultVal
+ }
+
+ tempVal, err := strconv.ParseInt(strVal, 10, 64)
+ if err != nil {
+ *v = append(*v, fmt.Sprintf("Field '%s' is not a valid int: %s", name, err))
+ }
+ return tempVal
+}
+
+// Float64FormValue does the same as Float64Value but extracts the value from the request object.
+func (v *Validation) Float64FormValue(r *http.Request, name string, defaultVal float64) float64 {
+ return v.Float64Value(name, r.FormValue(name), defaultVal)
+}
+
+// Int64FormValue does the same as Int64Value but extracts the value from the request object.
+func (v *Validation) Int64FormValue(r *http.Request, name string, defaultVal int64) int64 {
+ return v.Int64Value(name, r.FormValue(name), defaultVal)
+}
+
+// Int64SliceValue parses a comma-separated list of int values and returns them.
+func (v *Validation) Int64SliceValue(name string, strVal string, defaultVal []int64) []int64 {
+ if strVal == "" {
+ return defaultVal
+ }
+
+ splitVals := strings.Split(strVal, ",")
+ ret := make([]int64, 0, len(splitVals))
+ for _, oneStrVal := range splitVals {
+ tempVal, err := strconv.ParseInt(oneStrVal, 10, 64)
+ if err != nil {
+ *v = append(*v, fmt.Sprintf("Field '%s' is not a valid list of comma separated integers: %s", name, err))
+ return nil
+ }
+ ret = append(ret, tempVal)
+ }
+ return ret
+}
+
+// Int64SliceFormValue does the same as Int64SliceValue but extracts the given
+// name from the request.
+func (v *Validation) Int64SliceFormValue(r *http.Request, name string, defaultVal []int64) []int64 {
+ return v.Int64SliceValue(name, r.FormValue(name), defaultVal)
+}
+
+// QueryFormValue extracts a URL-encoded query from the form values and decodes it.
+// If the named field was not available in the given request an empty set url.Values
+// is returned. If an error occurs it will be added to the error list of the validation
+// object.
+func (v *Validation) QueryFormValue(r *http.Request, name string) map[string][]string {
+ if q := r.FormValue(name); q != "" {
+ ret, err := url.ParseQuery(q)
+ if err != nil {
+ *v = append(*v, fmt.Sprintf("Unable to parse query: %s. Error: %s", q, err))
+ return nil
+ }
+ return ret
+ }
+ return map[string][]string{}
+}
+
+// Errors returns a concatenation of all error values accumulated in validation or nil
+// if there were no errors.
+func (v *Validation) Errors() error {
+ if len(*v) == 0 {
+ return nil
+ }
+
+ return fmt.Errorf("%s", strings.Join(*v, "\n"))
+}
diff --git a/golden/go/storage/baseliner.go b/golden/go/storage/baseliner.go
index e34e77e..72bbd54 100644
--- a/golden/go/storage/baseliner.go
+++ b/golden/go/storage/baseliner.go
@@ -1,6 +1,8 @@
package storage
import (
+ "github.com/davecgh/go-spew/spew"
+ "go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/tiling"
"go.skia.org/infra/golden/go/baseline"
@@ -44,18 +46,23 @@
// PushMasterBaselines writes the baselines for the master branch to GCS.
func (b *Baseliner) PushMasterBaselines(tile *tiling.Tile) error {
if !b.CanWriteBaseline() {
- return sklog.FmtErrorf("Trying to write baseline while GCS path is not configured.")
+ return skerr.Fmt("Trying to write baseline while GCS path is not configured.")
}
_, baseLine, err := b.getMasterBaseline(tile)
if err != nil {
- return sklog.FmtErrorf("Error retrieving master baseline: %s", err)
+ return skerr.Fmt("Error retrieving master baseline: %s", err)
+ }
+
+ if baseLine == nil {
+ sklog.Infof("No baseline available.")
+ return nil
}
// Write the baseline to GCS.
outputPath, err := b.gStorageClient.WriteBaseLine(baseLine)
if err != nil {
- return sklog.FmtErrorf("Error writing baseline to GCS: %s", err)
+ return skerr.Fmt("Error writing baseline to GCS: %s", err)
}
sklog.Infof("Baseline for master written to %s.", outputPath)
return nil
@@ -64,18 +71,18 @@
// PushIssueBaseline writes the baseline for a Gerrit issue to GCS.
func (b *Baseliner) PushIssueBaseline(issueID int64, tile *tiling.Tile, tallies *tally.Tallies) error {
if !b.CanWriteBaseline() {
- return sklog.FmtErrorf("Trying to write baseline while GCS path is not configured.")
+ return skerr.Fmt("Trying to write baseline while GCS path is not configured.")
}
issueExpStore := b.issueExpStoreFactory(issueID)
exp, err := issueExpStore.Get()
if err != nil {
- return sklog.FmtErrorf("Unable to get issue expecations: %s", err)
+ return skerr.Fmt("Unable to get issue expecations: %s", err)
}
tryjobs, tryjobResults, err := b.tryjobStore.GetTryjobs(issueID, nil, true, true)
if err != nil {
- return sklog.FmtErrorf("Unable to get TryjobResults")
+ return skerr.Fmt("Unable to get TryjobResults")
}
talliesByTest := tallies.ByTest()
baseLine := baseline.GetBaselineForIssue(issueID, tryjobs, tryjobResults, exp, tile.Commits, talliesByTest)
@@ -83,7 +90,7 @@
// Write the baseline to GCS.
outputPath, err := b.gStorageClient.WriteBaseLine(baseLine)
if err != nil {
- return sklog.FmtErrorf("Error writing baseline to GCS: %s", err)
+ return skerr.Fmt("Error writing baseline to GCS: %s", err)
}
sklog.Infof("Baseline for issue %d written to %s.", issueID, outputPath)
return nil
@@ -100,6 +107,7 @@
egroup.Go(func() error {
var err error
masterBaseline, err = b.gStorageClient.ReadBaseline(commitHash, 0)
+ sklog.Infof("Master: %s %s", commitHash, spew.Sdump(masterBaseline))
return err
})
@@ -107,6 +115,7 @@
egroup.Go(func() error {
var err error
issueBaseline, err = b.gStorageClient.ReadBaseline(commitHash, issueID)
+ sklog.Infof("issue %s %d: %s", commitHash, issueID, spew.Sdump(issueBaseline))
return err
})
}
@@ -125,7 +134,7 @@
func (b *Baseliner) getMasterBaseline(tile *tiling.Tile) (types.Expectations, *baseline.CommitableBaseLine, error) {
exps, err := b.expectationsStore.Get()
if err != nil {
- return nil, nil, sklog.FmtErrorf("Unable to retrieve expectations: %s", err)
+ return nil, nil, skerr.Fmt("Unable to retrieve expectations: %s", err)
}
return exps, baseline.GetBaselineForMaster(exps, tile), nil
diff --git a/golden/go/storage/gsclient.go b/golden/go/storage/gsclient.go
index 7b25493..52ca50b 100644
--- a/golden/go/storage/gsclient.go
+++ b/golden/go/storage/gsclient.go
@@ -96,7 +96,10 @@
if err != nil {
// If the item doesn't exist we return an empty baseline
if err == gstorage.ErrObjectNotExist {
- return &baseline.CommitableBaseLine{Baseline: types.TestExp{}}, nil
+ return &baseline.CommitableBaseLine{
+ Baseline: types.TestExp{},
+ Issue: issueID,
+ }, nil
}
return nil, sklog.FmtErrorf("Error fetching attributes of baseline file: %s", err)
}
diff --git a/golden/go/storage/storage_test.go b/golden/go/storage/storage_test.go
index 35e2b2a..dbe1559 100644
--- a/golden/go/storage/storage_test.go
+++ b/golden/go/storage/storage_test.go
@@ -139,8 +139,8 @@
}()
// Read the master baseline that has not been written
- emptyBaseline := &baseline.CommitableBaseLine{Baseline: types.TestExp{}}
- foundBaseline, err := gsClient.ReadBaseline("", 0)
+ emptyBaseline := &baseline.CommitableBaseLine{Baseline: types.TestExp{}, Issue: 5344}
+ foundBaseline, err := gsClient.ReadBaseline("", 5344)
assert.NoError(t, err)
assert.Equal(t, emptyBaseline, foundBaseline)
diff --git a/golden/go/web/web.go b/golden/go/web/web.go
index 62d3599..9280506 100644
--- a/golden/go/web/web.go
+++ b/golden/go/web/web.go
@@ -26,6 +26,7 @@
"go.skia.org/infra/golden/go/ignore"
"go.skia.org/infra/golden/go/indexer"
"go.skia.org/infra/golden/go/search"
+ "go.skia.org/infra/golden/go/shared"
"go.skia.org/infra/golden/go/status"
"go.skia.org/infra/golden/go/storage"
"go.skia.org/infra/golden/go/summary"
@@ -34,15 +35,6 @@
"go.skia.org/infra/golden/go/validation"
)
-// Define common routes used by multiple servers
-const (
- // BASELINE_ROUTE serves the expectations of the master branch
- EXPECATIONS_ROUTE = "/json/expecations/commit/{commit_hash}"
-
- // BASELINE_ISSUE_ROUTE serves the baseline for the Gerrit CL identified by 'id'
- EXPECATIONS_ISSUE_ROUTE = "/json/expecations/issue/{issue_id}"
-)
-
const (
// DEFAULT_PAGE_SIZE is the default page size used for pagination.
DEFAULT_PAGE_SIZE = 20
@@ -871,7 +863,7 @@
q := r.URL.Query()
offset, size, err := httputils.PaginationParams(q, 0, DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE)
if err == nil {
- validate := search.Validation{}
+ validate := shared.Validation{}
issue := validate.Int64Value("issue", q.Get("issue"), 0)
if err := validate.Errors(); err != nil {
httputils.ReportError(w, r, err, "Unable to retrieve triage log.")
@@ -1027,6 +1019,15 @@
httputils.ReportError(w, r, err, "Issue ID must be valid integer.")
return
}
+
+ // Retrieve the commit at HEAD
+ tmpHashes := wh.Storages.Git.LastN(r.Context(), 1)
+ if len(tmpHashes) == 0 {
+ msg := "No commit information available"
+ httputils.ReportError(w, r, skerr.Fmt(msg), msg)
+ return
+ }
+ commitHash = tmpHashes[0]
} else {
// Since this was not called for an issue, we need to extract a Git hash.
var ok bool
