diff --git a/controller/BUILD.bazel b/controller/BUILD.bazel index dfab44b5..390e1e00 100644 --- a/controller/BUILD.bazel +++ b/controller/BUILD.bazel @@ -24,7 +24,6 @@ go_library( "//internal/targetdiff", "//internal/tgb", "//internal/tgbdiff", - "//mapper/proto", "//observability/metrics", "//orchestrator", "//tangopb", diff --git a/controller/README.md b/controller/README.md index aa516ebf..13310d4c 100644 --- a/controller/README.md +++ b/controller/README.md @@ -10,10 +10,9 @@ response streaming. The package is intentionally thin. It owns the cross-cutting concerns that sit between the wire protocol and the rest of the system: -- **Request validation and translation.** GetChangedTargets validates its - inputs and normalizes them into the internal call shapes used downstream. - GetTargetGraph receives an entity request and the resolved repository - configuration from the handler, which maps and validates the request. +- **Repository input.** Each call receives an entity request and the + resolved repository configuration from the handler. The controller does + not map the request or look up the repository itself. - **Read-through caching.** Where a request can be satisfied from previously computed artifacts, the controller fetches them from storage and streams them back without invoking the orchestrator. Cache misses fall through to diff --git a/controller/controller.go b/controller/controller.go index e1fb8ed0..ee7f28e2 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -86,7 +86,7 @@ type controller struct { // configured repository that the handler resolved for the request. type Controller interface { GetTargetGraph(request entity.GetTargetGraphRequest, outputConfig *pb.OutputConfig, stream pb.TangoServiceGetTargetGraphYARPCServer, repo config.RepositoryConfig) error - GetChangedTargets(request *pb.GetChangedTargetsRequest, stream pb.TangoServiceGetChangedTargetsYARPCServer) error + GetChangedTargets(request entity.GetChangedTargetsRequest, outputConfig *pb.OutputConfig, stream pb.TangoServiceGetChangedTargetsYARPCServer, repo config.RepositoryConfig) error } // NewController creates a new controller. appCtx is cancelled on process diff --git a/controller/controllermock/controllermock.go b/controller/controllermock/controllermock.go index 74e448e3..b1c1a297 100644 --- a/controller/controllermock/controllermock.go +++ b/controller/controllermock/controllermock.go @@ -43,17 +43,17 @@ func (m *MockController) EXPECT() *MockControllerMockRecorder { } // GetChangedTargets mocks base method. -func (m *MockController) GetChangedTargets(request *tangopb.GetChangedTargetsRequest, stream tangopb.TangoServiceGetChangedTargetsYARPCServer) error { +func (m *MockController) GetChangedTargets(request entity.GetChangedTargetsRequest, outputConfig *tangopb.OutputConfig, stream tangopb.TangoServiceGetChangedTargetsYARPCServer, repo config.RepositoryConfig) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetChangedTargets", request, stream) + ret := m.ctrl.Call(m, "GetChangedTargets", request, outputConfig, stream, repo) ret0, _ := ret[0].(error) return ret0 } // GetChangedTargets indicates an expected call of GetChangedTargets. -func (mr *MockControllerMockRecorder) GetChangedTargets(request, stream any) *gomock.Call { +func (mr *MockControllerMockRecorder) GetChangedTargets(request, outputConfig, stream, repo any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChangedTargets", reflect.TypeOf((*MockController)(nil).GetChangedTargets), request, stream) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChangedTargets", reflect.TypeOf((*MockController)(nil).GetChangedTargets), request, outputConfig, stream, repo) } // GetTargetGraph mocks base method. diff --git a/controller/getchangedtargets.go b/controller/getchangedtargets.go index c86a1d58..a3800a4b 100644 --- a/controller/getchangedtargets.go +++ b/controller/getchangedtargets.go @@ -24,7 +24,6 @@ import ( "github.com/uber/tango/config" "github.com/uber/tango/core/cachekey" - tangoerrors "github.com/uber/tango/core/errors" "github.com/uber/tango/core/storage" "github.com/uber/tango/entity" "github.com/uber/tango/internal/mapper" @@ -33,7 +32,6 @@ import ( "github.com/uber/tango/internal/targetdiff" "github.com/uber/tango/internal/tgb" "github.com/uber/tango/internal/tgbdiff" - "github.com/uber/tango/mapper/proto" "github.com/uber/tango/observability/metrics" pb "github.com/uber/tango/tangopb" "go.uber.org/zap" @@ -82,19 +80,11 @@ type job struct { // GetChangedTargets returns the changed targets between two revisions. If the // client disconnects, the stream's context is cancelled and the function // returns with context.Canceled. -func (c *controller) GetChangedTargets(request *pb.GetChangedTargetsRequest, stream pb.TangoServiceGetChangedTargetsYARPCServer) error { - entityReq, mappingErr := proto.ProtoToGetChangedTargetsRequest(request) - if mappingErr != nil { - mappingErr = tangoerrors.NewUser(fmt.Errorf("convert get changed targets request: %w", mappingErr)) - } - repoCfg, repo, repositoryErr := c.resolveRequestRepository(entityReq.First.Remote, mappingErr) - e := c.emitter.Tagged(map[string]string{metrics.TagRepo: repo}) +func (c *controller) GetChangedTargets(entityReq entity.GetChangedTargetsRequest, outputConfig *pb.OutputConfig, stream pb.TangoServiceGetChangedTargetsYARPCServer, repoCfg config.RepositoryConfig) error { + e := c.emitter.Tagged(map[string]string{metrics.TagRepo: repoCfg.RepositoryID}) logger := c.logger.WithLazy( - zap.String("repository", repo), + zap.String("repository", repoCfg.RepositoryID), ) - if repositoryErr != nil { - return repositoryErr - } ctx, cancelLink := c.linkRequestCtx(stream.Context()) defer cancelLink() start := time.Now() @@ -105,13 +95,13 @@ func (c *controller) GetChangedTargets(request *pb.GetChangedTargetsRequest, str // entirely. When OutputConfig is supplied, take max_distance at face value — // see proto/tango.proto OutputConfig.max_distance for the wire-default caveat. maxDist := int32(-1) - if request.GetOutputConfig() != nil { - maxDist = request.GetOutputConfig().GetMaxDistance() + if outputConfig != nil { + maxDist = outputConfig.GetMaxDistance() } // Fast path: stream a previously computed result straight from cache. - if !request.GetBypassCache() { - served, err := c.serveChangedTargetsFromCache(ctx, e, logger, entityReq, request.GetOutputConfig(), stream, repoCfg.RepositoryID, maxDist, start) + if !entityReq.BypassCache { + served, err := c.serveChangedTargetsFromCache(ctx, e, logger, entityReq, outputConfig, stream, repoCfg.RepositoryID, maxDist, start) if err != nil { return fmt.Errorf("serve from cache: %w", err) } @@ -141,7 +131,7 @@ func (c *controller) GetChangedTargets(request *pb.GetChangedTargetsRequest, str c.cacheComparedTargets(logger, entityReq, repoCfg.RepositoryID, changedTargetsResponses) sendStart := time.Now() - if err := sendTrimmedChangedTargets(stream, changedTargetsResponses, maxDist, request.GetOutputConfig()); err != nil { + if err := sendTrimmedChangedTargets(stream, changedTargetsResponses, maxDist, outputConfig); err != nil { return fmt.Errorf("send response: %w", err) } sendDuration := time.Since(sendStart) diff --git a/controller/getchangedtargets_test.go b/controller/getchangedtargets_test.go index da6d7090..9670a108 100644 --- a/controller/getchangedtargets_test.go +++ b/controller/getchangedtargets_test.go @@ -242,16 +242,6 @@ func TestCompareTargetGraphs_AllTargetsFileNoTrigger(t *testing.T) { assert.Equal(t, 0, totalChanged, "no targets should be changed when AllTargetsFiles hashes match") } -func TestGetChangedTargets_ValidationError(t *testing.T) { - ctrl := gomock.NewController(t) - stream := tangomock.NewMockTangoServiceGetChangedTargetsYARPCServer(ctrl) - - c := NewController(context.Background(), Params{RepoConfig: allowAnyRepositoryConfigProvider{}, Logger: zap.NewNop(), Orchestrator: orchestratormock.NewMockOrchestrator(ctrl)}) - - err := c.GetChangedTargets(nil, stream) - require.Error(t, err) -} - func TestGetChangedTargets_CacheHit(t *testing.T) { ctrl := gomock.NewController(t) stream := tangomock.NewMockTangoServiceGetChangedTargetsYARPCServer(ctrl) @@ -285,13 +275,10 @@ func TestGetChangedTargets_CacheHit(t *testing.T) { Orchestrator: orchestratormock.NewMockOrchestrator(ctrl), }) - request := &pb.GetChangedTargetsRequest{ - FirstRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha1"}, - SecondRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha2"}, - OutputConfig: &pb.OutputConfig{MaxDistance: -1}, - } + request := changedTargetsRequest() + outputConfig := &pb.OutputConfig{MaxDistance: -1} - err := c.GetChangedTargets(request, stream) + err := c.GetChangedTargets(request, outputConfig, stream, config.RepositoryConfig{Remote: "repo:go-code", RepositoryID: testRepositoryID("repo:go-code")}) require.NoError(t, err) } @@ -317,13 +304,10 @@ func TestGetChangedTargets_TreehashReadError(t *testing.T) { Orchestrator: orchestratormock.NewMockOrchestrator(ctrl), }) - request := &pb.GetChangedTargetsRequest{ - FirstRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha1"}, - SecondRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha2"}, - OutputConfig: &pb.OutputConfig{MaxDistance: -1}, - } + request := changedTargetsRequest() + outputConfig := &pb.OutputConfig{MaxDistance: -1} - err := c.GetChangedTargets(request, stream) + err := c.GetChangedTargets(request, outputConfig, stream, config.RepositoryConfig{Remote: "repo:go-code", RepositoryID: testRepositoryID("repo:go-code")}) require.Error(t, err) assert.Contains(t, err.Error(), injected.Error()) } @@ -404,13 +388,10 @@ func TestGetChangedTargets_StreamSendError(t *testing.T) { Orchestrator: orchestratormock.NewMockOrchestrator(ctrl), }) - request := &pb.GetChangedTargetsRequest{ - FirstRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha1"}, - SecondRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha2"}, - OutputConfig: &pb.OutputConfig{MaxDistance: -1}, - } + request := changedTargetsRequest() + outputConfig := &pb.OutputConfig{MaxDistance: -1} - err := c.GetChangedTargets(request, stream) + err := c.GetChangedTargets(request, outputConfig, stream, config.RepositoryConfig{Remote: "repo:go-code", RepositoryID: testRepositoryID("repo:go-code")}) assert.Error(t, err) select { @@ -500,13 +481,10 @@ func TestGetChangedTargets_streamChunks(t *testing.T) { Orchestrator: orchestratormock.NewMockOrchestrator(ctrl), }) - request := &pb.GetChangedTargetsRequest{ - FirstRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha1"}, - SecondRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha2"}, - OutputConfig: &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true, IncludeTags: true, IncludeAttributes: true}, - } + request := changedTargetsRequest() + outputConfig := &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true, IncludeTags: true, IncludeAttributes: true} - err := c.GetChangedTargets(request, stream) + err := c.GetChangedTargets(request, outputConfig, stream, config.RepositoryConfig{Remote: "repo:go-code", RepositoryID: testRepositoryID("repo:go-code")}) require.NoError(t, err) select { @@ -595,14 +573,13 @@ func TestGetChangedTargets_CacheWriteUsesAppCtx(t *testing.T) { Orchestrator: orchestratormock.NewMockOrchestrator(ctrl), }) - request := &pb.GetChangedTargetsRequest{ - FirstRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha1"}, - SecondRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha2"}, - OutputConfig: &pb.OutputConfig{MaxDistance: -1}, - } + request := changedTargetsRequest() + outputConfig := &pb.OutputConfig{MaxDistance: -1} handlerDone := make(chan error, 1) - go func() { handlerDone <- c.GetChangedTargets(request, stream) }() + go func() { + handlerDone <- c.GetChangedTargets(request, outputConfig, stream, config.RepositoryConfig{Remote: "repo:go-code", RepositoryID: testRepositoryID("repo:go-code")}) + }() var cacheCtx context.Context select { @@ -1089,13 +1066,10 @@ func TestGetChangedTargets_CacheHitWithDistanceFilter(t *testing.T) { Orchestrator: orchestratormock.NewMockOrchestrator(ctrl), }) - request := &pb.GetChangedTargetsRequest{ - FirstRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha1"}, - SecondRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha2"}, - OutputConfig: &pb.OutputConfig{MaxDistance: 1}, - } + request := changedTargetsRequest() + outputConfig := &pb.OutputConfig{MaxDistance: 1} - err := c.GetChangedTargets(request, stream) + err := c.GetChangedTargets(request, outputConfig, stream, config.RepositoryConfig{Remote: "repo:go-code", RepositoryID: testRepositoryID("repo:go-code")}) require.NoError(t, err) require.Len(t, sent, 2) @@ -1337,15 +1311,7 @@ func TestSendTrimmedChangedTargets_RetainsDeletedAtMaxDistanceOne(t *testing.T) assert.True(t, gotDeleted, "DELETED entry at distance 0 must survive max_distance=1") } -func changedTargetsRequest() *pb.GetChangedTargetsRequest { - return &pb.GetChangedTargetsRequest{ - FirstRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha1"}, - SecondRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha2"}, - OutputConfig: &pb.OutputConfig{MaxDistance: -1}, - } -} - -func changedTargetsEntityRequest() entity.GetChangedTargetsRequest { +func changedTargetsRequest() entity.GetChangedTargetsRequest { return entity.GetChangedTargetsRequest{ First: entity.BuildDescription{Remote: "repo:go-code", BaseSha: "sha1"}, Second: entity.BuildDescription{Remote: "repo:go-code", BaseSha: "sha2"}, @@ -1364,7 +1330,7 @@ func TestServeChangedTargetsFromCache(t *testing.T) { c.storage = st stream := tangomock.NewMockTangoServiceGetChangedTargetsYARPCServer(ctrl) - served, err := c.serveChangedTargetsFromCache(t.Context(), c.emitter, c.logger, changedTargetsEntityRequest(), &pb.OutputConfig{MaxDistance: -1}, stream, testRepositoryID("repo:go-code"), -1, time.Now()) + served, err := c.serveChangedTargetsFromCache(t.Context(), c.emitter, c.logger, changedTargetsRequest(), &pb.OutputConfig{MaxDistance: -1}, stream, testRepositoryID("repo:go-code"), -1, time.Now()) require.NoError(t, err) assert.False(t, served, "a cache miss must not be served") }) @@ -1403,7 +1369,7 @@ func TestServeChangedTargetsFromCache(t *testing.T) { stream := tangomock.NewMockTangoServiceGetChangedTargetsYARPCServer(ctrl) // No Send expectation: a corrupt blob must not send anything to the client. - served, err := c.serveChangedTargetsFromCache(t.Context(), c.emitter, c.logger, changedTargetsEntityRequest(), &pb.OutputConfig{MaxDistance: -1}, stream, testRepositoryID("repo:go-code"), -1, time.Now()) + served, err := c.serveChangedTargetsFromCache(t.Context(), c.emitter, c.logger, changedTargetsRequest(), &pb.OutputConfig{MaxDistance: -1}, stream, testRepositoryID("repo:go-code"), -1, time.Now()) require.NoError(t, err) assert.False(t, served, "a corrupt blob must trigger recompute, not a partial send") }) @@ -1437,7 +1403,7 @@ func TestServeChangedTargetsFromCache(t *testing.T) { stream := tangomock.NewMockTangoServiceGetChangedTargetsYARPCServer(ctrl) stream.EXPECT().Send(gomock.Any()).Return(nil).Times(2) - served, err := c.serveChangedTargetsFromCache(t.Context(), c.emitter, c.logger, changedTargetsEntityRequest(), &pb.OutputConfig{MaxDistance: -1}, stream, testRepositoryID("repo:go-code"), -1, time.Now()) + served, err := c.serveChangedTargetsFromCache(t.Context(), c.emitter, c.logger, changedTargetsRequest(), &pb.OutputConfig{MaxDistance: -1}, stream, testRepositoryID("repo:go-code"), -1, time.Now()) require.NoError(t, err) assert.True(t, served, "a clean cache hit must be served") }) @@ -1447,7 +1413,7 @@ func TestFetchTargetGraphs(t *testing.T) { // BypassCache=true keeps getGraph on the orchestrator path only, so these // tests need no storage mock. bypassRequest := func() entity.GetChangedTargetsRequest { - r := changedTargetsEntityRequest() + r := changedTargetsRequest() r.BypassCache = true return r } diff --git a/controller/getchangedtargets_tgb_test.go b/controller/getchangedtargets_tgb_test.go index d2d1ded3..919c11b8 100644 --- a/controller/getchangedtargets_tgb_test.go +++ b/controller/getchangedtargets_tgb_test.go @@ -133,8 +133,8 @@ func TestGetChangedTargets_TGBNativePath(t *testing.T) { }) request := changedTargetsRequest() - request.OutputConfig = &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true, IncludeTags: true, IncludeAttributes: true} - require.NoError(t, c.GetChangedTargets(request, stream)) + outputConfig := &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true, IncludeTags: true, IncludeAttributes: true} + require.NoError(t, c.GetChangedTargets(request, outputConfig, stream, config.RepositoryConfig{Remote: "repo:go-code", RepositoryID: testRepositoryID("repo:go-code")})) changed, idToName := changedTargetsSent(t, sent) require.Len(t, changed, 1, "should detect exactly the hash-flipped target") @@ -243,8 +243,8 @@ func TestGetChangedTargets_TGBAllTargetsTrigger(t *testing.T) { }) request := changedTargetsRequest() - request.OutputConfig = &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true} - require.NoError(t, c.GetChangedTargets(request, stream)) + outputConfig := &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true} + require.NoError(t, c.GetChangedTargets(request, outputConfig, stream, config.RepositoryConfig{Remote: "repo:go-code", RepositoryID: testRepositoryID("repo:go-code")})) changed, _ := changedTargetsSent(t, sent) require.Len(t, changed, 4, "all targets from second graph should be reported as changed") @@ -288,8 +288,8 @@ func TestGetChangedTargets_TGBAllTargetsTriggerPreservesMembershipChanges(t *tes }) request := changedTargetsRequest() - request.OutputConfig = &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true} - require.NoError(t, c.GetChangedTargets(request, stream)) + outputConfig := &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true} + require.NoError(t, c.GetChangedTargets(request, outputConfig, stream, config.RepositoryConfig{Remote: "repo:go-code", RepositoryID: testRepositoryID("repo:go-code")})) changed, idToName := changedTargetsSent(t, sent) require.Len(t, changed, 4) @@ -366,8 +366,8 @@ func TestGetChangedTargets_TGBAllTargetsNoTrigger(t *testing.T) { }) request := changedTargetsRequest() - request.OutputConfig = &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true} - require.NoError(t, c.GetChangedTargets(request, stream)) + outputConfig := &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true} + require.NoError(t, c.GetChangedTargets(request, outputConfig, stream, config.RepositoryConfig{Remote: "repo:go-code", RepositoryID: testRepositoryID("repo:go-code")})) changed, _ := changedTargetsSent(t, sent) require.Len(t, changed, 1, "only the hash-flipped target should be changed") @@ -412,8 +412,8 @@ func TestGetChangedTargets_TGBMixedFormatFallsBack(t *testing.T) { }) request := changedTargetsRequest() - request.OutputConfig = &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true, IncludeTags: true, IncludeAttributes: true} - require.NoError(t, c.GetChangedTargets(request, stream)) + outputConfig := &pb.OutputConfig{MaxDistance: -1, IncludeHashes: true, IncludeTags: true, IncludeAttributes: true} + require.NoError(t, c.GetChangedTargets(request, outputConfig, stream, config.RepositoryConfig{Remote: "repo:go-code", RepositoryID: testRepositoryID("repo:go-code")})) changed, idToName := changedTargetsSent(t, sent) require.Len(t, changed, 1) diff --git a/handler/getchangedtargets.go b/handler/getchangedtargets.go index dfb227fc..97809853 100644 --- a/handler/getchangedtargets.go +++ b/handler/getchangedtargets.go @@ -25,13 +25,11 @@ import ( ) func (h *handler) GetChangedTargets(request *pb.GetChangedTargetsRequest, stream pb.TangoServiceGetChangedTargetsYARPCServer) (retErr error) { - // Temporary: the handler maps the request only to tag the metrics. The - // controller maps it again until it takes the entity request. entityReq, mappingErr := proto.ProtoToGetChangedTargetsRequest(request) if mappingErr != nil { mappingErr = tangoerrors.NewUser(fmt.Errorf("convert get changed targets request: %w", mappingErr)) } - _, repo, repositoryErr := h.resolveRequestRepository(entityReq.First.Remote, mappingErr) + repoCfg, repo, repositoryErr := h.resolveRequestRepository(entityReq.First.Remote, mappingErr) e := h.emitter.Tagged(map[string]string{metrics.TagRepo: repo}) op := metrics.Begin(e, opGetChangedTargets, metrics.SlowDurationBuckets) logger := h.logger.WithLazy(zap.String("repository", repo)) @@ -45,5 +43,5 @@ func (h *handler) GetChangedTargets(request *pb.GetChangedTargetsRequest, stream if repositoryErr != nil { return repositoryErr } - return h.controller.GetChangedTargets(request, stream) + return h.controller.GetChangedTargets(entityReq, request.GetOutputConfig(), stream, repoCfg) } diff --git a/handler/getchangedtargets_test.go b/handler/getchangedtargets_test.go index 0a6bdd2b..ee109afd 100644 --- a/handler/getchangedtargets_test.go +++ b/handler/getchangedtargets_test.go @@ -19,7 +19,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/uber/tango/config" mock_controller "github.com/uber/tango/controller/controllermock" + "github.com/uber/tango/entity" pb "github.com/uber/tango/tangopb" "go.uber.org/mock/gomock" "go.uber.org/zap/zaptest" @@ -36,12 +38,18 @@ func TestGetChangedTargetsForwardsToController(t *testing.T) { req := &pb.GetChangedTargetsRequest{ FirstRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha1"}, SecondRevision: &pb.BuildDescription{Strategy: pb.COMPUTATION_STRATEGY_UNSET, Remote: "repo:go-code", BaseSha: "sha2"}, + OutputConfig: &pb.OutputConfig{MaxDistance: 2}, + BypassCache: true, } stream := &fakeGetChangedTargetsStream{} - ctrl.EXPECT().GetChangedTargets(gomock.Any(), gomock.Any()).DoAndReturn( - func(gotReq *pb.GetChangedTargetsRequest, gotStream pb.TangoServiceGetChangedTargetsYARPCServer) error { - assert.Same(t, req, gotReq) + ctrl.EXPECT().GetChangedTargets(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(gotReq entity.GetChangedTargetsRequest, gotOutput *pb.OutputConfig, gotStream pb.TangoServiceGetChangedTargetsYARPCServer, gotRepo config.RepositoryConfig) error { + assert.Equal(t, "sha1", gotReq.First.BaseSha) + assert.Equal(t, "sha2", gotReq.Second.BaseSha) + assert.True(t, gotReq.BypassCache) + assert.Same(t, req.OutputConfig, gotOutput) assert.Same(t, stream, gotStream) + assert.Equal(t, "test-repository", gotRepo.RepositoryID) return nil }) diff --git a/handler/metrics_test.go b/handler/metrics_test.go index 921638ff..284d4268 100644 --- a/handler/metrics_test.go +++ b/handler/metrics_test.go @@ -43,7 +43,7 @@ func TestGetTargetGraphMetrics(t *testing.T) { func TestGetChangedTargetsMetrics(t *testing.T) { ts := tally.NewTestScope("", nil) ctrl := mock_controller.NewMockController(gomock.NewController(t)) - ctrl.EXPECT().GetChangedTargets(gomock.Any(), gomock.Any()).Return(nil) + ctrl.EXPECT().GetChangedTargets(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) h := New(Params{Logger: zap.NewNop(), Controller: ctrl, RepoConfig: allowAnyRepositoryConfigProvider{}, Scope: ts}) build := &pb.BuildDescription{Remote: "repo:go-code", BaseSha: "sha", Strategy: pb.COMPUTATION_STRATEGY_UNSET}