Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start of the work to have a flexible scheme #3037

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/checks/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (s checksAPIImpl) CreateWPTCheckSuite(appID, installationID int64, sha stri

func (s checksAPIImpl) GetWPTRepoAppInstallationIDs() (appID, installationID int64) {
// Production
if s.GetHostname() == "wpt.fyi" {
if s.GetOrigin().Hostname() == "wpt.fyi" {
return wptfyiCheckAppID, wptRepoInstallationID
}
// Default to staging
Expand Down
28 changes: 14 additions & 14 deletions api/checks/mock_checks/api_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions api/checks/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,9 @@ func getDiffSummary(aeAPI shared.AppEngineAPI, diffAPI shared.DiffAPI, suite sha
}

diffURL := diffAPI.GetDiffURL(baseRun, headRun, &diffFilter)
host := aeAPI.GetHostname()
origin := aeAPI.GetOrigin()
checkState := summaries.CheckState{
HostName: host,
HostName: origin.Host,
TestRun: &headRun,
Product: checkProduct,
HeadSHA: headRun.FullRevisionHash,
Expand Down Expand Up @@ -231,7 +231,7 @@ func getDiffSummary(aeAPI shared.AppEngineAPI, diffAPI shared.DiffAPI, suite sha
resultsComparison := summaries.ResultsComparison{
BaseRun: baseRun,
HeadRun: headRun,
HostURL: fmt.Sprintf("https://%s/", host),
HostURL: origin.Host,
DiffURL: diffURL.String(),
}
if headRun.LabelsSet().Contains(shared.PRHeadLabel) {
Expand Down
10 changes: 8 additions & 2 deletions api/checks/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ func TestGetDiffSummary_Regressed(t *testing.T) {
aeAPI.EXPECT().Context().AnyTimes().Return(context.Background())
aeAPI.EXPECT().IsFeatureEnabled(onlyChangesAsRegressionsFeature).Return(enabled)
aeAPI.EXPECT().IsFeatureEnabled(failChecksOnRegressionFeature).Return(false)
aeAPI.EXPECT().GetHostname()
origin, _ := url.Parse("https://some-origin.com")
aeAPI.EXPECT().GetOrigin().Return(origin)
diffAPI := sharedtest.NewMockDiffAPI(mockCtrl)
diffAPI.EXPECT().GetRunsDiff(before, after, sharedtest.SameDiffFilter("ADC"), gomock.Any()).Return(runDiff, nil)
if enabled {
Expand All @@ -49,6 +50,7 @@ func TestGetDiffSummary_Regressed(t *testing.T) {
_, ok := summary.(summaries.Regressed)
assert.True(t, ok)
assert.Equal(t, suite.PRNumbers, summary.GetCheckState().PRNumbers)
assert.Equal(t, "some-origin.com", summary.GetCheckState().HostName)
})
}
testSummary(false)
Expand All @@ -68,7 +70,10 @@ func TestGetDiffSummary_Completed(t *testing.T) {
aeAPI.EXPECT().Context().AnyTimes().Return(context.Background())
aeAPI.EXPECT().IsFeatureEnabled(onlyChangesAsRegressionsFeature).Return(false)
aeAPI.EXPECT().IsFeatureEnabled(failChecksOnRegressionFeature).Return(false)
aeAPI.EXPECT().GetHostname()
aeAPI.EXPECT().GetOrigin().Return(&url.URL{
Scheme: "https",
Host: "example.com",
})
diffAPI := sharedtest.NewMockDiffAPI(mockCtrl)
diffAPI.EXPECT().GetRunsDiff(before, after, gomock.Any(), gomock.Any()).Return(runDiff, nil)
diffURL, _ := url.Parse("https://wpt.fyi/results?diff")
Expand All @@ -83,6 +88,7 @@ func TestGetDiffSummary_Completed(t *testing.T) {
_, ok := summary.(summaries.Completed)
assert.True(t, ok)
assert.Equal(t, suite.PRNumbers, summary.GetCheckState().PRNumbers)
assert.Equal(t, "example.com", summary.GetCheckState().HostName)
}

func getBeforeAndAfterRuns() (before, after shared.TestRun) {
Expand Down
7 changes: 5 additions & 2 deletions api/receiver/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"io/ioutil"
"net/http"
"net/url"
"path"
"strings"
"time"

Expand Down Expand Up @@ -67,8 +68,10 @@ func (c client) CreateRun(
payload.Add("labels", strings.Join(labels, ","))
}
// Ensure we call back to this appengine version instance.
host := c.aeAPI.GetVersionedHostname()
payload.Add("callback_url", fmt.Sprintf("https://%s/api/results/create", host))
u := c.aeAPI.GetVersionedOrigin()
u.Path = path.Join(u.Path, "/api/results/create")

payload.Add("callback_url", u.String())

uploadURL := c.aeAPI.GetResultsUploadURL()
req, err := http.NewRequest("POST", uploadURL.String(), strings.NewReader(payload.Encode()))
Expand Down
5 changes: 4 additions & 1 deletion api/receiver/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func TestCreateRun(t *testing.T) {
assert.Nil(t, r.ParseForm())
assert.Equal(t, []string{"https://wpt.fyi/results.json.gz"}, r.PostForm["result_url"])
assert.Equal(t, []string{"https://wpt.fyi/screenshots.db.gz"}, r.PostForm["screenshot_url"])
assert.Equal(t, []string{"http://localhost:8080/api/results/create"}, r.PostForm["callback_url"])
assert.Equal(t, "foo,bar", r.PostForm.Get("labels"))
w.Write([]byte("OK"))
visited = true
Expand All @@ -41,7 +42,9 @@ func TestCreateRun(t *testing.T) {
defer mockC.Finish()
aeAPI := sharedtest.NewMockAppEngineAPI(mockC)
gomock.InOrder(
aeAPI.EXPECT().GetVersionedHostname().Return("localhost:8080"),
aeAPI.EXPECT().GetVersionedOrigin().Return(&url.URL{
Scheme: "http",
Host: "localhost:8080"}),
aeAPI.EXPECT().GetResultsUploadURL().Return(serverURL),
aeAPI.EXPECT().GetHTTPClientWithTimeout(UploadTimeout).Return(server.Client()),
)
Expand Down
28 changes: 14 additions & 14 deletions api/receiver/mock_receiver/api_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion api/screenshot/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func uploadScreenshotHandler(w http.ResponseWriter, r *http.Request) {
return
}
bucketName := "wptd-screenshots-staging"
if aeAPI.GetHostname() == "wpt.fyi" {
if aeAPI.GetOrigin().Hostname() == "wpt.fyi" {
bucketName = "wptd-screenshots"
}
bucket := gcs.Bucket(bucketName)
Expand Down
2 changes: 1 addition & 1 deletion api/screenshot_redirect_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func apiScreenshotRedirectHandler(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
aeAPI := shared.NewAppEngineAPI(ctx)
bucket := "wptd-screenshots-staging"
if aeAPI.GetHostname() == "wpt.fyi" {
if aeAPI.GetOrigin().Hostname() == "wpt.fyi" {
bucket = "wptd-screenshots"
}
url := fmt.Sprintf("https://storage.googleapis.com/%s/%s.png", bucket, shot)
Expand Down
27 changes: 23 additions & 4 deletions api/taskcluster/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@ func strPtr(s string) *string {
return &s
}

func testOrigin() *url.URL {
// Because these tests call the function multiple times, we need to use
// DoAndReturn and pass this function for the mock assertion.
// Without it, it will append the path to the same pointer
u := new(url.URL)
u.Scheme = "http"
u.Host = "localhost:8080"
return u
}

func TestShouldProcessStatus_states(t *testing.T) {
status := tc.StatusEventPayload{}
status.State = strPtr("success")
Expand Down Expand Up @@ -250,6 +260,8 @@ func TestCreateAllRuns_success(t *testing.T) {
var requested uint32
requested = 0
handler := func(w http.ResponseWriter, r *http.Request) {
assert.Nil(t, r.ParseForm())
assert.Equal(t, []string{"http://localhost:8080/api/results/create"}, r.PostForm["callback_url"])
atomic.AddUint32(&requested, 1)
w.Write([]byte("OK"))
}
Expand All @@ -261,7 +273,8 @@ func TestCreateAllRuns_success(t *testing.T) {
mockC := gomock.NewController(t)
defer mockC.Finish()
aeAPI := sharedtest.NewMockAppEngineAPI(mockC)
aeAPI.EXPECT().GetVersionedHostname().AnyTimes().Return("localhost:8080")

aeAPI.EXPECT().GetVersionedOrigin().MinTimes(1).DoAndReturn(testOrigin)
aeAPI.EXPECT().GetHTTPClientWithTimeout(uc.UploadTimeout).AnyTimes().Return(server.Client())
aeAPI.EXPECT().GetResultsUploadURL().AnyTimes().Return(serverURL)

Expand Down Expand Up @@ -308,6 +321,8 @@ func TestCreateAllRuns_one_error(t *testing.T) {
var requested uint32
requested = 0
handler := func(w http.ResponseWriter, r *http.Request) {
assert.Nil(t, r.ParseForm())
assert.Equal(t, []string{"http://localhost:8080/api/results/create"}, r.PostForm["callback_url"])
if atomic.CompareAndSwapUint32(&requested, 0, 1) {
w.Write([]byte("OK"))
} else if atomic.CompareAndSwapUint32(&requested, 1, 2) {
Expand All @@ -324,7 +339,7 @@ func TestCreateAllRuns_one_error(t *testing.T) {
sha := "abcdef1234abcdef1234abcdef1234abcdef1234"

aeAPI := sharedtest.NewMockAppEngineAPI(mockC)
aeAPI.EXPECT().GetVersionedHostname().MinTimes(1).Return("localhost:8080")
aeAPI.EXPECT().GetVersionedOrigin().MinTimes(1).DoAndReturn(testOrigin)
aeAPI.EXPECT().GetHTTPClientWithTimeout(uc.UploadTimeout).Times(2).Return(server.Client())
serverURL, _ := url.Parse(server.URL)
aeAPI.EXPECT().GetResultsUploadURL().AnyTimes().Return(serverURL)
Expand All @@ -349,6 +364,8 @@ func TestCreateAllRuns_one_error(t *testing.T) {

func TestCreateAllRuns_all_errors(t *testing.T) {
handler := func(w http.ResponseWriter, r *http.Request) {
assert.Nil(t, r.ParseForm())
assert.Equal(t, []string{"http://localhost:8080/api/results/create"}, r.PostForm["callback_url"])
time.Sleep(time.Second * 2)
}
server := httptest.NewServer(http.HandlerFunc(handler))
Expand All @@ -359,7 +376,7 @@ func TestCreateAllRuns_all_errors(t *testing.T) {
sha := "abcdef1234abcdef1234abcdef1234abcdef1234"

aeAPI := sharedtest.NewMockAppEngineAPI(mockC)
aeAPI.EXPECT().GetVersionedHostname().MinTimes(1).Return("localhost:8080")
aeAPI.EXPECT().GetVersionedOrigin().MinTimes(1).DoAndReturn(testOrigin)
// Give a very short timeout (instead of the asked 1min) to make tests faster.
aeAPI.EXPECT().GetHTTPClientWithTimeout(uc.UploadTimeout).MinTimes(1).Return(&http.Client{Timeout: time.Microsecond})
serverURL, _ := url.Parse(server.URL)
Expand All @@ -383,6 +400,8 @@ func TestCreateAllRuns_all_errors(t *testing.T) {

func TestCreateAllRuns_pr_labels_exclude_master(t *testing.T) {
handler := func(w http.ResponseWriter, r *http.Request) {
assert.Nil(t, r.ParseForm())
assert.Equal(t, []string{"http://localhost:8080/api/results/create"}, r.PostForm["callback_url"])
// We should not see a master label here, even though we
// specify one in the call to tc.CreateAllRuns.
defer r.Body.Close()
Expand All @@ -398,7 +417,7 @@ func TestCreateAllRuns_pr_labels_exclude_master(t *testing.T) {
mockC := gomock.NewController(t)
defer mockC.Finish()
aeAPI := sharedtest.NewMockAppEngineAPI(mockC)
aeAPI.EXPECT().GetVersionedHostname().AnyTimes().Return("localhost:8080")
aeAPI.EXPECT().GetVersionedOrigin().AnyTimes().DoAndReturn(testOrigin)
aeAPI.EXPECT().GetHTTPClientWithTimeout(uc.UploadTimeout).AnyTimes().Return(server.Client())
aeAPI.EXPECT().GetResultsUploadURL().AnyTimes().Return(serverURL)

Expand Down
Loading