Add pixel_diff support to poller
BUG=skia:6778
Change-Id: Ib3d50caa91867944e329bebdc57f8cf3a93d2b0c
Reviewed-on: https://skia-review.googlesource.com/20511
Commit-Queue: Ravi Mistry <rmistry@google.com>
Reviewed-by: Ben Wagner <benjaminwagner@google.com>
diff --git a/ct/go/poller/main.go b/ct/go/poller/main.go
index 994b9f2..44cfdc2 100644
--- a/ct/go/poller/main.go
+++ b/ct/go/poller/main.go
@@ -26,6 +26,7 @@
"go.skia.org/infra/ct/go/ctfe/chromium_builds"
"go.skia.org/infra/ct/go/ctfe/chromium_perf"
"go.skia.org/infra/ct/go/ctfe/lua_scripts"
+ "go.skia.org/infra/ct/go/ctfe/pixel_diff"
"go.skia.org/infra/ct/go/ctfe/task_common"
"go.skia.org/infra/ct/go/frontend"
"go.skia.org/infra/ct/go/master_scripts/master_common"
@@ -49,49 +50,6 @@
tasksMtx = sync.Mutex{}
)
-// Enum of states that the poller could be in. Satisfies the fmt.Stringer interface.
-type TaskType uint32
-
-const (
- IDLE TaskType = iota
- UPDATE_AND_BUILD
- CHROMIUM_PERF
- CAPTURE_SKPS
- LUA_SCRIPT
- CHROMIUM_BUILD
- RECREATE_PAGE_SETS
- RECREATE_WEBPAGE_ARCHIVES
- CHROMIUM_ANALYSIS
- POLL
-)
-
-func (t TaskType) String() string {
- switch t {
- case IDLE:
- return "IDLE"
- case UPDATE_AND_BUILD:
- return "UPDATE_AND_BUILD"
- case CHROMIUM_PERF:
- return "CHROMIUM_PERF"
- case CAPTURE_SKPS:
- return "CAPTURE_SKPS"
- case LUA_SCRIPT:
- return "LUA_SCRIPT"
- case CHROMIUM_BUILD:
- return "CHROMIUM_BUILD"
- case RECREATE_PAGE_SETS:
- return "RECREATE_PAGE_SETS"
- case RECREATE_WEBPAGE_ARCHIVES:
- return "RECREATE_WEBPAGE_ARCHIVES"
- case CHROMIUM_ANALYSIS:
- return "CHROMIUM_ANALYSIS"
- case POLL:
- return "POLL"
- default:
- return "(unknown)"
- }
-}
-
// Runs "git pull; make all".
func updateAndBuild() error {
repoMtx.Lock()
@@ -231,6 +189,45 @@
})
}
+// Define frontend.PixelDiffDBTask here so we can add methods.
+type PixelDiffTask struct {
+ pixel_diff.DBTask
+}
+
+func (task *PixelDiffTask) Execute() error {
+ runId := runId(task)
+ for fileSuffix, patch := range map[string]string{
+ ".chromium.patch": task.ChromiumPatch,
+ ".skia.patch": task.SkiaPatch,
+ ".custom_webpages.csv": task.CustomWebpages,
+ } {
+ // Add an extra newline at the end because git sometimes rejects patches due to
+ // missing newlines.
+ patch = patch + "\n"
+ patchPath := filepath.Join(os.TempDir(), runId+fileSuffix)
+ if err := ioutil.WriteFile(patchPath, []byte(patch), 0666); err != nil {
+ return err
+ }
+ defer skutil.Remove(patchPath)
+ }
+ return exec.Run(&exec.Command{
+ Name: "pixel_diff_on_workers",
+ Args: []string{
+ "--emails=" + task.Username,
+ "--description=" + task.Description,
+ "--gae_task_id=" + strconv.FormatInt(task.Id, 10),
+ "--pageset_type=" + task.PageSets,
+ "--benchmark_extra_args=" + task.BenchmarkArgs,
+ "--browser_extra_args_nopatch=" + task.BrowserArgsNoPatch,
+ "--browser_extra_args_withpatch=" + task.BrowserArgsWithPatch,
+ "--run_id=" + runId,
+ "--logtostderr",
+ "--log_id=" + runId,
+ fmt.Sprintf("--local=%t", *master_common.Local),
+ },
+ })
+}
+
// Define frontend.CaptureSkpsDBTask here so we can add methods.
type CaptureSkpsTask struct {
capture_skps.DBTask
@@ -370,6 +367,8 @@
switch t := otherTask.(type) {
case *chromium_perf.DBTask:
return &ChromiumPerfTask{DBTask: *t}
+ case *pixel_diff.DBTask:
+ return &PixelDiffTask{DBTask: *t}
case *capture_skps.DBTask:
return &CaptureSkpsTask{DBTask: *t}
case *lua_scripts.DBTask:
diff --git a/ct/go/poller/poller_test.go b/ct/go/poller/poller_test.go
index 6bf8b59..f3e3276 100644
--- a/ct/go/poller/poller_test.go
+++ b/ct/go/poller/poller_test.go
@@ -16,6 +16,7 @@
"go.skia.org/infra/ct/go/ctfe/chromium_builds"
"go.skia.org/infra/ct/go/ctfe/chromium_perf"
"go.skia.org/infra/ct/go/ctfe/lua_scripts"
+ "go.skia.org/infra/ct/go/ctfe/pixel_diff"
"go.skia.org/infra/ct/go/ctfe/task_common"
ctfeutil "go.skia.org/infra/ct/go/ctfe/util"
"go.skia.org/infra/ct/go/frontend"
@@ -111,6 +112,48 @@
expect.NotNil(t, cmd.Timeout)
}
+func pendingPixelDiffTask() PixelDiffTask {
+ return PixelDiffTask{
+ DBTask: pixel_diff.DBTask{
+ CommonCols: pendingCommonCols(),
+ PageSets: "All",
+ BenchmarkArgs: "benchmarkargs",
+ BrowserArgsNoPatch: "banp",
+ BrowserArgsWithPatch: "bawp",
+ Description: "description",
+ ChromiumPatch: "chromiumpatch",
+ SkiaPatch: "skiapatch",
+ },
+ }
+}
+
+func TestPixelDiffExecute(t *testing.T) {
+ testutils.SmallTest(t)
+ task := pendingPixelDiffTask()
+ mockRun := exec.CommandCollector{}
+ exec.SetRunForTesting(mockRun.Run)
+ defer exec.SetRunForTesting(exec.DefaultRun)
+ err := task.Execute()
+ assert.NoError(t, err)
+ assert.Len(t, mockRun.Commands(), 1)
+ cmd := mockRun.Commands()[0]
+ expect.Equal(t, "pixel_diff_on_workers", cmd.Name)
+ expect.Equal(t, len(cmd.Args), 11)
+ expect.Contains(t, cmd.Args, "--gae_task_id=42")
+ expect.Contains(t, cmd.Args, "--description=description")
+ expect.Contains(t, cmd.Args, "--emails=nobody@chromium.org")
+ expect.Contains(t, cmd.Args, "--pageset_type=All")
+ expect.Contains(t, cmd.Args, "--benchmark_extra_args=benchmarkargs")
+ expect.Contains(t, cmd.Args, "--browser_extra_args_nopatch=banp")
+ expect.Contains(t, cmd.Args, "--browser_extra_args_withpatch=bawp")
+ expect.Contains(t, cmd.Args, "--logtostderr")
+ expect.Contains(t, cmd.Args, "--local=false")
+ runId := getRunId(t, cmd)
+ expect.Contains(t, cmd.Args, "--run_id="+runId)
+ expect.Contains(t, cmd.Args, "--log_id="+runId)
+ expect.NotNil(t, cmd.Timeout)
+}
+
func pendingCaptureSkpsTask() CaptureSkpsTask {
return CaptureSkpsTask{
DBTask: capture_skps.DBTask{
