dashboard/app: fix testing for go1.11 runtime

0. Remove aetest build tag. We don't need it anymore, go test should work.
1. IsDevAppServer does not return true in tests anymore, so don't use it
2. Use a different mechanism to register test/prod config.
We don't have aetest tag anymore, so we need something even more dynamic.
3. Fix new golangci-lint warnings: all test files are checked now.

Update #1461

22 files changed, 50 insertions, 86 deletions

diff --git a/.gitignore b/.gitignore
index 34ddd10f6..97e0be570 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,6 +4,7 @@
 workdir*
 
 bin/
+dashboard/app/config_prod.go
 
 # jetbrains goland
 .idea
diff --git a/dashboard/app/README.md b/dashboard/app/README.md
index 30a3c1e73..eaffe720d 100644
--- a/dashboard/app/README.md
+++ b/dashboard/app/README.md
@@ -10,9 +10,9 @@ and in particular [support package docs](https://cloud.google.com/appengine/docs
 
 **Note**: The app is not stable and is not officially supported. It's here only to power the main deployment.
 
-To test the app one needs to install the SDK and add the `goapp` binary to `$PATH`, then run:
+To test the app one needs to install the SDK and add the `dev_appserver.py` binary to `$PATH`, then run:
 ```
-goapp test -tags=aetest github.com/google/syzkaller/dashboard/app
+go test github.com/google/syzkaller/dashboard/app
 ```
 During development it's handy to use `-short` flag to not run the longest tests.
 
diff --git a/dashboard/app/access.go b/dashboard/app/access.go
index cd53094a2..0ebcf1ce7 100644
--- a/dashboard/app/access.go
+++ b/dashboard/app/access.go
@@ -10,7 +10,6 @@ import (
 	"strings"
 
 	"golang.org/x/net/context"
-	"google.golang.org/appengine"
 	db "google.golang.org/appengine/datastore"
 	"google.golang.org/appengine/log"
 	"google.golang.org/appengine/user"
@@ -46,6 +45,9 @@ func checkAccessLevel(c context.Context, r *http.Request, level AccessLevel) err
 	return ErrAccess
 }
 
+// AuthDomain is broken in AppEngine tests.
+var isBrokenAuthDomainInTest = false
+
 func accessLevel(c context.Context, r *http.Request) AccessLevel {
 	if user.IsAdmin(c) {
 		switch r.FormValue("access") {
@@ -59,7 +61,7 @@ func accessLevel(c context.Context, r *http.Request) AccessLevel {
 	u := user.Current(c)
 	if u == nil ||
 		// devappserver is broken
-		u.AuthDomain != "gmail.com" && !appengine.IsDevAppServer() ||
+		u.AuthDomain != "gmail.com" && !isBrokenAuthDomainInTest ||
 		!strings.HasSuffix(u.Email, config.AuthDomain) {
 		return AccessPublic
 	}
diff --git a/dashboard/app/access_test.go b/dashboard/app/access_test.go
index 442034a95..1de53f75f 100644
--- a/dashboard/app/access_test.go
+++ b/dashboard/app/access_test.go
@@ -1,8 +1,6 @@
 // Copyright 2018 syzkaller project authors. All rights reserved.
 // Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
 
-// +build aetest
-
 package main
 
 import (
@@ -327,7 +325,7 @@ func TestAccess(t *testing.T) {
 				t.Fatal(err1)
 			}
 			c.expectNE(err, nil)
-			httpErr, ok := err.(HttpError)
+			httpErr, ok := err.(HTTPError)
 			c.expectTrue(ok)
 			c.expectEQ(httpErr.Code, http.StatusTemporaryRedirect)
 			c.expectEQ(httpErr.Headers["Location"], []string{loginURL})
diff --git a/dashboard/app/aetest.go b/dashboard/app/aetest.go
deleted file mode 100644
index 3166d72c2..000000000
--- a/dashboard/app/aetest.go
+++ /dev/null
@@ -1,10 +0,0 @@
-// Copyright 2019 syzkaller project authors. All rights reserved.
-// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
-
-// +build aetest
-
-package main
-
-// isAppEngineTest is meant to be used in prod config to either
-// load the config or just check its correctness.
-const isAppEngineTest = true
diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go
index 9030c0e64..6ece0cee5 100644
--- a/dashboard/app/app_test.go
+++ b/dashboard/app/app_test.go
@@ -1,8 +1,6 @@
 // Copyright 2017 syzkaller project authors. All rights reserved.
 // Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
 
-// +build aetest
-
 package main
 
 import (
@@ -18,6 +16,9 @@ import (
 )
 
 func init() {
+	isBrokenAuthDomainInTest = true
+	obsoleteWhatWontBeFixBisected = true
+	notifyAboutUnsuccessfulBisections = true
 	initMocks()
 	installConfig(testConfig)
 }
@@ -388,7 +389,7 @@ func checkLoginRedirect(c *Ctx, accessLevel AccessLevel, url string) {
 func checkRedirect(c *Ctx, accessLevel AccessLevel, from, to string, status int) {
 	_, err := c.httpRequest("GET", from, "", accessLevel)
 	c.expectNE(err, nil)
-	httpErr, ok := err.(HttpError)
+	httpErr, ok := err.(HTTPError)
 	c.expectTrue(ok)
 	c.expectEQ(httpErr.Code, status)
 	c.expectEQ(httpErr.Headers["Location"], []string{to})
diff --git a/dashboard/app/bisect_test.go b/dashboard/app/bisect_test.go
index ed37d2ac2..cebb0b933 100644
--- a/dashboard/app/bisect_test.go
+++ b/dashboard/app/bisect_test.go
@@ -1,8 +1,6 @@
 // Copyright 2019 syzkaller project authors. All rights reserved.
 // Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
 
-// +build aetest
-
 package main
 
 import (
@@ -372,12 +370,11 @@ https://goo.gl/tpsmEJ#testing-patches`,
 	dbBug, dbCrash, _ = c.loadBug(extBugID)
 	reproSyzLink = externalLink(c.ctx, textReproSyz, dbCrash.ReproSyz)
 	reproCLink = externalLink(c.ctx, textReproC, dbCrash.ReproC)
-	dbJob, dbBuild, dbJobCrash = c.loadJob(jobID)
+	dbJob, dbBuild, _ = c.loadJob(jobID)
 	kernelConfigLink = externalLink(c.ctx, textKernelConfig, dbBuild.KernelConfig)
 	bisectCrashReportLink = externalLink(c.ctx, textCrashReport, dbJob.CrashReport)
 	bisectCrashLogLink = externalLink(c.ctx, textCrashLog, dbJob.CrashLog)
 	bisectLogLink = externalLink(c.ctx, textLog, dbJob.Log)
-	crashLogLink = externalLink(c.ctx, textCrashLog, dbJobCrash.Log)
 
 	{
 		msg := c.pollEmailBug()
diff --git a/dashboard/app/commit_poll_test.go b/dashboard/app/commit_poll_test.go
index 207ea4fc6..5f80db826 100644
--- a/dashboard/app/commit_poll_test.go
+++ b/dashboard/app/commit_poll_test.go
@@ -1,8 +1,6 @@
 // Copyright 2019 syzkaller project authors. All rights reserved.
 // Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
 
-// +build aetest
-
 package main
 
 import (
diff --git a/dashboard/app/config.go b/dashboard/app/config.go
index 4b57667ab..cac0e9035 100644
--- a/dashboard/app/config.go
+++ b/dashboard/app/config.go
@@ -193,22 +193,18 @@ func (cfg *Config) ReportingByName(name string) *Reporting {
 	return nil
 }
 
-// config is populated by installConfig which should be called either from tests
-// or from a separate file that provides actual production config.
-var config *GlobalConfig
-
-func init() {
-	// Prevents gometalinter from considering everything as dead code.
-	if false && isAppEngineTest {
-		installConfig(nil)
-	}
-}
+// config is installed either by tests or from mainConfig in main function
+// (a separate file should install mainConfig in an init function).
+var (
+	config     *GlobalConfig
+	mainConfig *GlobalConfig
+)
 
 func installConfig(cfg *GlobalConfig) {
+	checkConfig(cfg)
 	if config != nil {
 		panic("another config is already installed")
 	}
-	checkConfig(cfg)
 	config = cfg
 	initEmailReporting()
 	initHTTPHandlers()
@@ -216,6 +212,9 @@ func installConfig(cfg *GlobalConfig) {
 }
 
 func checkConfig(cfg *GlobalConfig) {
+	if cfg == nil {
+		panic("installing nil config")
+	}
 	if len(cfg.Namespaces) == 0 {
 		panic("no namespaces found")
 	}
diff --git a/dashboard/app/dashboard.go b/dashboard/app/dashboard.go
index 971d336f6..50de4f6fa 100644
--- a/dashboard/app/dashboard.go
+++ b/dashboard/app/dashboard.go
@@ -8,5 +8,6 @@ import (
 )
 
 func main() {
+	installConfig(mainConfig)
 	appengine.Main()
 }
diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go
index 0d77a4fe1..9c8dfd711 100644
--- a/dashboard/app/email_test.go
+++ b/dashboard/app/email_test.go
@@ -1,8 +1,6 @@
 // Copyright 2017 syzkaller project authors. All rights reserved.
 // Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
 
-// +build aetest
-
 package main
 
 import (
diff --git a/dashboard/app/fix_test.go b/dashboard/app/fix_test.go
index 33a3c5aad..9505177e3 100644
--- a/dashboard/app/fix_test.go
+++ b/dashboard/app/fix_test.go
@@ -1,8 +1,6 @@
 // Copyright 2017 syzkaller project authors. All rights reserved.
 // Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
 
-// +build aetest
-
 package main
 
 import (
diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go
index ce40210f7..0933f1436 100644
--- a/dashboard/app/jobs.go
+++ b/dashboard/app/jobs.go
@@ -14,7 +14,6 @@ import (
 	"github.com/google/syzkaller/pkg/email"
 	"github.com/google/syzkaller/pkg/vcs"
 	"golang.org/x/net/context"
-	"google.golang.org/appengine"
 	db "google.golang.org/appengine/datastore"
 	"google.golang.org/appengine/log"
 )
@@ -558,6 +557,11 @@ func updateBugBisection(c context.Context, job *Job, jobKey *db.Key, req *dashap
 	return nil
 }
 
+// TODO: this is temporal for gradual bisection rollout.
+// Notify only about successful cause bisection for now.
+// For now we only enable this in tests.
+var notifyAboutUnsuccessfulBisections = false
+
 func pollCompletedJobs(c context.Context, typ string) ([]*dashapi.BugReport, error) {
 	var jobs []*Job
 	keys, err := db.NewQuery("Job").
@@ -577,9 +581,7 @@ func pollCompletedJobs(c context.Context, typ string) ([]*dashapi.BugReport, err
 		if reporting.Config.Type() != typ {
 			continue
 		}
-		// TODO: this is temporal for gradual bisection rollout.
-		// Notify only about successful cause bisection for now.
-		if !appengine.IsDevAppServer() && job.Type == JobBisectCause && len(job.Commits) != 1 {
+		if !notifyAboutUnsuccessfulBisections && job.Type == JobBisectCause && len(job.Commits) != 1 {
 			continue
 		}
 		// If BisectFix results in a crash on HEAD, no notification is sent out.
diff --git a/dashboard/app/jobs_test.go b/dashboard/app/jobs_test.go
index afa5cd106..473ca0dc5 100644
--- a/dashboard/app/jobs_test.go
+++ b/dashboard/app/jobs_test.go
@@ -1,8 +1,6 @@
 // Copyright 2017 syzkaller project authors. All rights reserved.
 // Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
 
-// +build aetest
-
 package main
 
 import (
diff --git a/dashboard/app/noaetest.go b/dashboard/app/noaetest.go
deleted file mode 100644
index b642ed66c..000000000
--- a/dashboard/app/noaetest.go
+++ /dev/null
@@ -1,8 +0,0 @@
-// Copyright 2019 syzkaller project authors. All rights reserved.
-// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
-
-// +build !aetest
-
-package main
-
-const isAppEngineTest = false
diff --git a/dashboard/app/notifications_test.go b/dashboard/app/notifications_test.go
index c7771a627..593f99c01 100644
--- a/dashboard/app/notifications_test.go
+++ b/dashboard/app/notifications_test.go
@@ -1,8 +1,6 @@
 // Copyright 2019 syzkaller project authors. All rights reserved.
 // Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
 
-// +build aetest
-
 package main
 
 import (
@@ -219,6 +217,7 @@ func TestEmailNotifObsoleted(t *testing.T) {
 	report = c.pollEmailBug()
 	c.incomingEmail(report.Sender, "#syz upstream")
 	report = c.pollEmailBug()
+	_ = report
 
 	c.advanceTime(101 * 24 * time.Hour)
 	notif = c.pollEmailBug()
@@ -242,6 +241,7 @@ func TestEmailNotifNotObsoleted(t *testing.T) {
 	report1 := c.pollEmailBug()
 	c.incomingEmail(report1.Sender, "#syz upstream")
 	report1 = c.pollEmailBug()
+	_ = report1
 
 	// This crash will get another crash later.
 	crash2 := testCrash(build, 2)
@@ -249,6 +249,7 @@ func TestEmailNotifNotObsoleted(t *testing.T) {
 	report2 := c.pollEmailBug()
 	c.incomingEmail(report2.Sender, "#syz upstream")
 	report2 = c.pollEmailBug()
+	_ = report2
 
 	// This crash will get some activity later.
 	crash3 := testCrash(build, 3)
@@ -296,6 +297,7 @@ func TestEmailNotifObsoletedManager(t *testing.T) {
 	report := c.pollEmailBug()
 	c.incomingEmail(report.Sender, "#syz upstream")
 	report = c.pollEmailBug()
+	_ = report
 	c.advanceTime(200 * 24 * time.Hour)
 	notif := c.pollEmailBug()
 	c.expectTrue(strings.Contains(notif.Body, "Auto-closing this bug as obsolete"))
diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go
index d508857f1..4022bfb23 100644
--- a/dashboard/app/reporting.go
+++ b/dashboard/app/reporting.go
@@ -16,7 +16,6 @@ import (
 	"github.com/google/syzkaller/pkg/email"
 	"github.com/google/syzkaller/pkg/html"
 	"golang.org/x/net/context"
-	"google.golang.org/appengine"
 	db "google.golang.org/appengine/datastore"
 	"google.golang.org/appengine/log"
 )
@@ -223,14 +222,17 @@ func handleReportNotif(c context.Context, typ string, bug *Bug) (*dashapi.BugNot
 	return nil, nil
 }
 
+// TODO: this is what we would like to do, but we need to figure out
+// KMSAN story: we don't do fix bisection on it (rebased),
+// do we want to close all old KMSAN bugs with repros?
+// For now we only enable this in tests.
+var obsoleteWhatWontBeFixBisected = false
+
 func (bug *Bug) wontBeFixBisected() bool {
 	if bug.ReproLevel == ReproLevelNone {
 		return true
 	}
-	// TODO: this is what we would like to do, but we need to figure out
-	// KMSAN story: we don't do fix bisection on it (rebased),
-	// do we want to close all old KMSAN bugs with repros?
-	if appengine.IsDevAppServer() {
+	if obsoleteWhatWontBeFixBisected {
 		cfg := config.Namespaces[bug.Namespace]
 		for _, mgr := range bug.HappenedOn {
 			if !cfg.Managers[mgr].FixBisectionDisabled {
diff --git a/dashboard/app/reporting_external.go b/dashboard/app/reporting_external.go
index f487ac9d3..2120660fe 100644
--- a/dashboard/app/reporting_external.go
+++ b/dashboard/app/reporting_external.go
@@ -17,14 +17,6 @@ import (
 // The external system is meant to poll for new bugs with apiReportingPoll,
 // and report back bug status updates with apiReportingUpdate.
 
-type ExternalConfig struct {
-	ID string
-}
-
-func (cfg *ExternalConfig) Type() string {
-	return cfg.ID
-}
-
 func apiReportingPollBugs(c context.Context, r *http.Request, payload []byte) (interface{}, error) {
 	req := new(dashapi.PollBugsRequest)
 	if err := json.Unmarshal(payload, req); err != nil {
diff --git a/dashboard/app/reporting_test.go b/dashboard/app/reporting_test.go
index aad68751c..4471fe930 100644
--- a/dashboard/app/reporting_test.go
+++ b/dashboard/app/reporting_test.go
@@ -1,8 +1,6 @@
 // Copyright 2017 syzkaller project authors. All rights reserved.
 // Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
 
-// +build aetest
-
 package main
 
 import (
diff --git a/dashboard/app/repro_test.go b/dashboard/app/repro_test.go
index c704b9226..45599925c 100644
--- a/dashboard/app/repro_test.go
+++ b/dashboard/app/repro_test.go
@@ -1,8 +1,6 @@
 // Copyright 2018 syzkaller project authors. All rights reserved.
 // Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
 
-// +build aetest
-
 package main
 
 import (
diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go
index 50cfefdd5..46dcc1f45 100644
--- a/dashboard/app/util_test.go
+++ b/dashboard/app/util_test.go
@@ -3,10 +3,6 @@
 
 // The test uses aetest package that starts local dev_appserver and handles all requests locally:
 // https://cloud.google.com/appengine/docs/standard/go/tools/localunittesting/reference
-// The test requires installed appengine SDK (dev_appserver), so we guard it by aetest tag.
-// Run the test with: goapp test -tags=aetest
-
-// +build aetest
 
 package main
 
@@ -90,7 +86,7 @@ func (c *Ctx) expectForbidden(err error) {
 	if err == nil {
 		c.t.Fatalf("\n%v: expected to fail as 403, but it does not", caller(0))
 	}
-	httpErr, ok := err.(HttpError)
+	httpErr, ok := err.(HTTPError)
 	if !ok || httpErr.Code != http.StatusForbidden {
 		c.t.Fatalf("\n%v: expected to fail as 403, but it failed as %v", caller(0), err)
 	}
@@ -209,18 +205,18 @@ func (c *Ctx) httpRequest(method, url, body string, access AccessLevel) ([]byte,
 	http.DefaultServeMux.ServeHTTP(w, r)
 	c.t.Logf("REPLY: %v", w.Code)
 	if w.Code != http.StatusOK {
-		return nil, HttpError{w.Code, w.Body.String(), w.HeaderMap}
+		return nil, HTTPError{w.Code, w.Body.String(), w.Result().Header}
 	}
 	return w.Body.Bytes(), nil
 }
 
-type HttpError struct {
+type HTTPError struct {
 	Code    int
 	Body    string
 	Headers http.Header
 }
 
-func (err HttpError) Error() string {
+func (err HTTPError) Error() string {
 	return fmt.Sprintf("%v: %v", err.Code, err.Body)
 }
 
@@ -378,10 +374,6 @@ func (client *apiClient) pollNotifs(expect int) []*dashapi.BugNotification {
 	return resp.Notifications
 }
 
-func (client *apiClient) pollNotif() *dashapi.BugNotification {
-	return client.pollNotifs(1)[0]
-}
-
 func (client *apiClient) updateBug(extID string, status dashapi.BugStatus, dup string) {
 	reply, _ := client.ReportingUpdate(&dashapi.BugUpdate{
 		ID:     extID,
diff --git a/pkg/gce/gce.go b/pkg/gce/gce.go
index 59731a858..c940bdeaf 100644
--- a/pkg/gce/gce.go
+++ b/pkg/gce/gce.go
@@ -53,6 +53,11 @@ func NewContext() (*Context, error) {
 		return nil, fmt.Errorf("failed to get a token source: %v", err)
 	}
 	httpClient := oauth2.NewClient(background, tokenSource)
+	// nolint
+	// compute.New is deprecated: please use NewService instead.
+	// To provide a custom HTTP client, use option.WithHTTPClient.
+	// If you are using google.golang.org/api/googleapis/transport.APIKey,
+	// use option.WithAPIKey with NewService instead.
 	ctx.computeService, _ = compute.New(httpClient)
 	// Obtain project name, zone and current instance IP address.
 	ctx.ProjectID, err = ctx.getMeta("project/project-id")