From b1b59a844fa71fb869e6ce280fc25e0fcfe9e24f Mon Sep 17 00:00:00 2001 From: "alessandro.pinna" Date: Fri, 22 Dec 2023 09:39:41 +0100 Subject: [PATCH 1/7] gateway: add missing sortdirection to get orgs cursor the SortDirection field in the returned cursor wasn't set. Set it to the right value. Convert tests to matrix tests to reduce code duplication and add tests with both sort directions. --- internal/services/gateway/action/org.go | 5 +- tests/setup_test.go | 222 ++++++++++++++---------- 2 files changed, 134 insertions(+), 93 deletions(-) diff --git a/internal/services/gateway/action/org.go b/internal/services/gateway/action/org.go index e7b8963ea..8a0445c7f 100644 --- a/internal/services/gateway/action/org.go +++ b/internal/services/gateway/action/org.go @@ -89,7 +89,10 @@ func (h *ActionHandler) GetOrgs(ctx context.Context, req *GetOrgsRequest) (*GetO var outCursor string if resp.HasMore && len(orgs) > 0 { lastRemoteSourceName := orgs[len(orgs)-1].Name - outCursor, err = MarshalCursor(&StartCursor{Start: lastRemoteSourceName}) + outCursor, err = MarshalCursor(&StartCursor{ + Start: lastRemoteSourceName, + SortDirection: sortDirection, + }) if err != nil { return nil, errors.WithStack(err) } diff --git a/tests/setup_test.go b/tests/setup_test.go index 1b3674229..86b091b9c 100644 --- a/tests/setup_test.go +++ b/tests/setup_test.go @@ -4043,14 +4043,13 @@ func TestGetOrgs(t *testing.T) { gwAdminClient := gwclient.NewClient(sc.config.Gateway.APIExposedURL, sc.config.Gateway.AdminToken) - // create users _, _, err := gwAdminClient.CreateUser(ctx, &gwapitypes.CreateUserRequest{UserName: agolaUser01}) testutil.NilError(t, err) tokenUser01, _, err := gwAdminClient.CreateUserToken(ctx, agolaUser01, &gwapitypes.CreateUserTokenRequest{TokenName: "test"}) testutil.NilError(t, err) - gwClientUser01 := gwclient.NewClient(sc.config.Gateway.APIExposedURL, tokenUser01.Token) + gwUser01Client := gwclient.NewClient(sc.config.Gateway.APIExposedURL, tokenUser01.Token) allOrgs := []*gwapitypes.OrgResponse{} publicOrgs := []*gwapitypes.OrgResponse{} @@ -4069,109 +4068,148 @@ func TestGetOrgs(t *testing.T) { } } - t.Run("test get public orgs with default limit greater than orgs", func(t *testing.T) { - orgs, resp, err := gwClientUser01.GetOrgs(ctx, &gwclient.ListOptions{SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedOrgs := 9 - assert.Assert(t, cmp.Len(orgs, expectedOrgs)) - assert.Assert(t, resp.Cursor == "") - - assert.DeepEqual(t, publicOrgs[:expectedOrgs], orgs) - }) - - t.Run("test get public/private orgs with default limit greater than orgs", func(t *testing.T) { - orgs, resp, err := gwAdminClient.GetOrgs(ctx, &gwclient.ListOptions{SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedOrgs := 18 - assert.Assert(t, cmp.Len(orgs, expectedOrgs)) - assert.Assert(t, resp.Cursor == "") - }) - - t.Run("test get public orgs with limit less than orgs", func(t *testing.T) { - orgs, resp, err := gwClientUser01.GetOrgs(ctx, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedOrgs := 5 - assert.Assert(t, cmp.Len(orgs, expectedOrgs)) - assert.Assert(t, resp.Cursor != "") - }) - - t.Run("test get public/private orgs with limit less than orgs", func(t *testing.T) { - orgs, resp, err := gwAdminClient.GetOrgs(ctx, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedOrgs := 5 - assert.Assert(t, cmp.Len(orgs, expectedOrgs)) - assert.Assert(t, resp.Cursor != "") - }) - - t.Run("test get public orgs with limit less than orgs continuation", func(t *testing.T) { - respAllOrgs := []*gwapitypes.OrgResponse{} + tests := []struct { + name string + getPublicOrgsOnly bool + limit int + sortDirection gwapitypes.SortDirection + expectedCallsNumber int + }{ + { + name: "test get public orgs with limit = 0", + getPublicOrgsOnly: true, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get public/private orgs with limit = 0", + getPublicOrgsOnly: false, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get public orgs with limit less than orgs", + getPublicOrgsOnly: true, + limit: 2, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 5, + }, + { + name: "test get public orgs with limit greater than orgs", + getPublicOrgsOnly: true, + limit: 10, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get public orgs with limit less than orgs continuation", + getPublicOrgsOnly: true, + limit: 3, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 3, + }, + { + name: "test get public/private orgs with limit less than orgs continuation", + getPublicOrgsOnly: false, + limit: 3, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 6, + }, + { + name: "test get public orgs with limit = 0 and sortDirection desc", + getPublicOrgsOnly: true, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + { + name: "test get public/private orgs with limit = 0 and sortDirection desc", + getPublicOrgsOnly: false, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + { + name: "test get public orgs with limit less than orgs and sortDirection desc", + getPublicOrgsOnly: true, + limit: 2, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 5, + }, + { + name: "test get public orgs with limit greater than orgs and sortDirection desc", + getPublicOrgsOnly: true, + limit: 10, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + { + name: "test get public orgs with limit less than orgs continuation and sortDirection desc", + getPublicOrgsOnly: true, + limit: 3, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 3, + }, + { + name: "test get public/private orgs with limit less than orgs continuation and sortDirection desc", + getPublicOrgsOnly: false, + limit: 3, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 6, + }, + } - respOrgs, resp, err := gwClientUser01.GetOrgs(ctx, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + // populate the expected orgs and client + var expectedOrgs []*gwapitypes.OrgResponse + var client *gwclient.Client + if tt.getPublicOrgsOnly { + expectedOrgs = append(expectedOrgs, publicOrgs...) + client = gwUser01Client + } else { + expectedOrgs = append(expectedOrgs, allOrgs...) + client = gwAdminClient + } - expectedOrgs := 5 - assert.Assert(t, cmp.Len(respOrgs, expectedOrgs)) - assert.Assert(t, resp.Cursor != "") + // reverse if sortDirection is desc + // TODO(sgotti) use go 1.21 generics slices.Reverse when removing support for go < 1.21 + if tt.sortDirection == gwapitypes.SortDirectionDesc { + for i, j := 0, len(expectedOrgs)-1; i < j; i, j = i+1, j-1 { + expectedOrgs[i], expectedOrgs[j] = expectedOrgs[j], expectedOrgs[i] + } + } - respAllOrgs = append(respAllOrgs, respOrgs...) + var respAllOrgs []*gwapitypes.OrgResponse - // fetch next results - for { - respOrgs, resp, err = gwClientUser01.GetOrgs(ctx, &gwclient.ListOptions{Cursor: resp.Cursor, Limit: 5}) + respOrgs, res, err := client.GetOrgs(ctx, &gwclient.ListOptions{ + Limit: tt.limit, SortDirection: tt.sortDirection, + }) testutil.NilError(t, err) - expectedOrgs := 5 - assert.Assert(t, resp.Cursor == "" || (len(respOrgs) == expectedOrgs)) - respAllOrgs = append(respAllOrgs, respOrgs...) + callsNumber := 1 - if resp.Cursor == "" { - break - } - } - - expectedOrgs = 9 - assert.Assert(t, cmp.Len(respAllOrgs, expectedOrgs)) - - assert.DeepEqual(t, publicOrgs, respAllOrgs) - }) - - t.Run("test get public/private orgs with limit less than orgs continuation", func(t *testing.T) { - respAllOrgs := []*gwapitypes.OrgResponse{} - - respOrgs, resp, err := gwAdminClient.GetOrgs(ctx, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedOrgs := 5 - assert.Assert(t, cmp.Len(respOrgs, expectedOrgs)) - assert.Assert(t, resp.Cursor != "") - - respAllOrgs = append(respAllOrgs, respOrgs...) - - // fetch next results - for { - respOrgs, resp, err = gwAdminClient.GetOrgs(ctx, &gwclient.ListOptions{Cursor: resp.Cursor, Limit: 5}) - testutil.NilError(t, err) + // fetch next results + for { + if res.Cursor == "" { + break + } - expectedOrgs := 5 - assert.Assert(t, resp.Cursor == "" || (len(respOrgs) == expectedOrgs)) + respOrgs, res, err = client.GetOrgs(ctx, &gwclient.ListOptions{ + Cursor: res.Cursor, Limit: tt.limit, + }) + testutil.NilError(t, err) - respAllOrgs = append(respAllOrgs, respOrgs...) + callsNumber++ - if resp.Cursor == "" { - break + respAllOrgs = append(respAllOrgs, respOrgs...) } - } - - expectedOrgs = 18 - assert.Assert(t, cmp.Len(respAllOrgs, expectedOrgs)) - assert.DeepEqual(t, allOrgs, respAllOrgs) - }) + assert.Assert(t, cmp.Len(respAllOrgs, len(expectedOrgs))) + assert.DeepEqual(t, respAllOrgs, expectedOrgs) + assert.Assert(t, cmp.Equal(callsNumber, tt.expectedCallsNumber)) + }) + } } func TestGetOrgMembers(t *testing.T) { From 504a4d0066b2f1b35d99bbcc3937e999efd24875 Mon Sep 17 00:00:00 2001 From: "alessandro.pinna" Date: Fri, 22 Dec 2023 11:54:28 +0100 Subject: [PATCH 2/7] gateway: add missing sortdirection to get org members cursor the SortDirection field in the returned cursor wasn't set. Set it to the right value. Convert tests to matrix tests to reduce code duplication and add tests with both sort directions. --- internal/services/gateway/action/org.go | 5 +- tests/setup_test.go | 131 +++++++++++++++--------- 2 files changed, 85 insertions(+), 51 deletions(-) diff --git a/internal/services/gateway/action/org.go b/internal/services/gateway/action/org.go index 8a0445c7f..b2bcc450c 100644 --- a/internal/services/gateway/action/org.go +++ b/internal/services/gateway/action/org.go @@ -149,7 +149,10 @@ func (h *ActionHandler) GetOrgMembers(ctx context.Context, req *GetOrgMembersReq var outCursor string if resp.HasMore && len(orgMembers) > 0 { lastUserName := orgMembers[len(orgMembers)-1].User.Name - outCursor, err = MarshalCursor(&StartCursor{Start: lastUserName}) + outCursor, err = MarshalCursor(&StartCursor{ + Start: lastUserName, + SortDirection: sortDirection, + }) if err != nil { return nil, errors.WithStack(err) } diff --git a/tests/setup_test.go b/tests/setup_test.go index 86b091b9c..802737424 100644 --- a/tests/setup_test.go +++ b/tests/setup_test.go @@ -4226,76 +4226,107 @@ func TestGetOrgMembers(t *testing.T) { createLinkedAccount(ctx, t, sc.gitea, sc.config) gwClient := gwclient.NewClient(sc.config.Gateway.APIExposedURL, sc.config.Gateway.AdminToken) - users := []*gwapitypes.UserResponse{} for i := 1; i < 10; i++ { - user, _, err := gwClient.CreateUser(ctx, &gwapitypes.CreateUserRequest{UserName: fmt.Sprintf("orguser%d", i)}) + _, _, err := gwClient.CreateUser(ctx, &gwapitypes.CreateUserRequest{UserName: fmt.Sprintf("orguser%d", i)}) testutil.NilError(t, err) - - users = append(users, user) } org, _, err := gwClient.CreateOrg(ctx, &gwapitypes.CreateOrgRequest{Name: agolaOrg01, Visibility: gwapitypes.VisibilityPublic}) testutil.NilError(t, err) - for _, user := range users { - _, _, err := gwClient.AddOrgMember(ctx, agolaOrg01, user.ID, gwapitypes.MemberRoleMember) - testutil.NilError(t, err) - } - - t.Run("test get org members with default limit greater than org members", func(t *testing.T) { - res, resp, err := gwClient.GetOrgMembers(ctx, org.ID, &gwclient.ListOptions{SortDirection: gwapitypes.SortDirectionAsc}) + var allOrgMembers []*gwapitypes.OrgMemberResponse + for i := 1; i < 10; i++ { + orgMember, _, err := gwClient.AddOrgMember(ctx, agolaOrg01, fmt.Sprintf("orguser%d", i), gwapitypes.MemberRoleMember) testutil.NilError(t, err) - expectedOrgMembers := 9 - assert.Assert(t, cmp.Len(res.Members, expectedOrgMembers)) - assert.Assert(t, resp.Cursor == "") - }) - - t.Run("test get org members with limit less than org members", func(t *testing.T) { - res, resp, err := gwClient.GetOrgMembers(ctx, org.ID, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) + allOrgMembers = append(allOrgMembers, &orgMember.OrgMemberResponse) + } - expectedOrgMembers := 5 - assert.Assert(t, cmp.Len(res.Members, expectedOrgMembers)) - assert.Assert(t, resp.Cursor != "") - }) + tests := []struct { + name string + limit int + sortDirection gwapitypes.SortDirection + expectedCallsNumber int + }{ + { + name: "test get org members with limit = 0", + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get org members with limit less than org members continuation", + limit: 2, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 5, + }, + { + name: "test get org members with limit greater than org members", + limit: 10, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get org members with limit = 0 and sortDirection desc", + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + { + name: "test get org members with limit less than org members continuation and sortDirection desc", + limit: 2, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 5, + }, + { + name: "test get org members with limit greater than org members and sortDirection desc", + limit: 10, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + } - t.Run("test get org members with limit less than org members continuation", func(t *testing.T) { - orgMembers := []*gwapitypes.OrgMemberResponse{} + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + expectedOrgMembers := append([]*gwapitypes.OrgMemberResponse{}, allOrgMembers...) + // reverse if sortDirection is desc + // TODO(sgotti) use go 1.21 generics slices.Reverse when removing support for go < 1.21 + if tt.sortDirection == gwapitypes.SortDirectionDesc { + for i, j := 0, len(expectedOrgMembers)-1; i < j; i, j = i+1, j-1 { + expectedOrgMembers[i], expectedOrgMembers[j] = expectedOrgMembers[j], expectedOrgMembers[i] + } + } - res, resp, err := gwClient.GetOrgMembers(ctx, org.ID, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) + var respAllOrgMembers []*gwapitypes.OrgMemberResponse - expectedOrgMembers := 5 - assert.Assert(t, cmp.Len(res.Members, expectedOrgMembers)) - assert.Assert(t, resp.Cursor != "") + respOrgMembers, res, err := gwClient.GetOrgMembers(ctx, org.ID, &gwclient.ListOptions{ + Limit: tt.limit, SortDirection: tt.sortDirection, + }) + testutil.NilError(t, err) - orgMembers = append(orgMembers, res.Members...) + respAllOrgMembers = append(respAllOrgMembers, respOrgMembers.Members...) + callsNumber := 1 - // fetch next results - for { - res, resp, err = gwClient.GetOrgMembers(ctx, org.ID, &gwclient.ListOptions{Cursor: resp.Cursor, Limit: 5}) - testutil.NilError(t, err) + // fetch next results + for { + if res.Cursor == "" { + break + } - expectedOrgMembers := 5 - assert.Assert(t, resp.Cursor == "" || (len(res.Members) == expectedOrgMembers)) + respOrgMembers, res, err = gwClient.GetOrgMembers(ctx, org.ID, &gwclient.ListOptions{ + Cursor: res.Cursor, Limit: tt.limit, + }) + testutil.NilError(t, err) - orgMembers = append(orgMembers, res.Members...) + callsNumber++ - if resp.Cursor == "" { - break + respAllOrgMembers = append(respAllOrgMembers, respOrgMembers.Members...) } - } - - expectedOrgMembers = 9 - assert.Assert(t, cmp.Len(orgMembers, expectedOrgMembers)) - orgMemberUsers := []*gwapitypes.UserResponse{} - for _, orgMember := range orgMembers { - orgMemberUsers = append(orgMemberUsers, orgMember.User) - } - assert.DeepEqual(t, users, orgMemberUsers) - }) + assert.Assert(t, cmp.Len(respAllOrgMembers, len(expectedOrgMembers))) + assert.DeepEqual(t, respAllOrgMembers, expectedOrgMembers) + assert.Assert(t, cmp.Equal(callsNumber, tt.expectedCallsNumber)) + }) + } } func TestGetUserOrgs(t *testing.T) { From ef64e5302bea5a62ca11b4f3029e74bcf9d67fa8 Mon Sep 17 00:00:00 2001 From: "alessandro.pinna" Date: Fri, 22 Dec 2023 14:35:47 +0100 Subject: [PATCH 3/7] gateway: add missing sortdirection to get remote sources cursor the SortDirection field in the returned cursor wasn't set. Set it to the right value. Convert tests to matrix tests to reduce code duplication and add tests with both sort directions. --- .../services/gateway/action/remotesource.go | 5 +- tests/setup_test.go | 115 ++++++++++++------ 2 files changed, 79 insertions(+), 41 deletions(-) diff --git a/internal/services/gateway/action/remotesource.go b/internal/services/gateway/action/remotesource.go index aefebe637..8b2c5330e 100644 --- a/internal/services/gateway/action/remotesource.go +++ b/internal/services/gateway/action/remotesource.go @@ -64,7 +64,10 @@ func (h *ActionHandler) GetRemoteSources(ctx context.Context, req *GetRemoteSour var outCursor string if resp.HasMore && len(remoteSources) > 0 { lastRemoteSourceName := remoteSources[len(remoteSources)-1].Name - outCursor, err = MarshalCursor(&StartCursor{Start: lastRemoteSourceName}) + outCursor, err = MarshalCursor(&StartCursor{ + Start: lastRemoteSourceName, + SortDirection: sortDirection, + }) if err != nil { return nil, errors.WithStack(err) } diff --git a/tests/setup_test.go b/tests/setup_test.go index 802737424..1f0434e8d 100644 --- a/tests/setup_test.go +++ b/tests/setup_test.go @@ -3901,56 +3901,91 @@ func TestGetRemoteSources(t *testing.T) { remoteSources = append(remoteSources, remoteSource) } - t.Run("test get remote sources with default limit greater than remote sources", func(t *testing.T) { - remoteSources, resp, err := gwClient.GetRemoteSources(ctx, &gwclient.ListOptions{SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedRemoteSources := 9 - assert.Assert(t, cmp.Len(remoteSources, expectedRemoteSources)) - assert.Assert(t, resp.Cursor == "") - }) - - t.Run("test get remote sources with limit less than remote sources", func(t *testing.T) { - remoteSources, resp, err := gwClient.GetRemoteSources(ctx, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedRemoteSources := 5 - assert.Assert(t, cmp.Len(remoteSources, expectedRemoteSources)) - assert.Assert(t, resp.Cursor != "") - }) + tests := []struct { + name string + limit int + sortDirection gwapitypes.SortDirection + expectedCallsNumber int + }{ + { + name: "test get remote sources with limit = 0", + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get remote sources with limit less than remote sources continuation", + limit: 2, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 5, + }, + { + name: "test get remote sources with limit greater than remote sources", + limit: 10, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get remote sources with limit = 0 and sortDirection desc", + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + { + name: "test get remote sources with limit less than remote sources continuation and sortDirection desc", + limit: 2, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 5, + }, + { + name: "test get remote sources with limit greater than remote sources and sortDirection desc", + limit: 10, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + } - t.Run("test get remote sources with limit less than remote sources continuation", func(t *testing.T) { - respAllRemoteSources := []*gwapitypes.RemoteSourceResponse{} + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + expectedRemoteSources := append([]*gwapitypes.RemoteSourceResponse{}, remoteSources...) + // reverse if sortDirection is desc + // TODO(sgotti) use go 1.21 generics slices.Reverse when removing support for go < 1.21 + if tt.sortDirection == gwapitypes.SortDirectionDesc { + for i, j := 0, len(expectedRemoteSources)-1; i < j; i, j = i+1, j-1 { + expectedRemoteSources[i], expectedRemoteSources[j] = expectedRemoteSources[j], expectedRemoteSources[i] + } + } - respRemoteSources, resp, err := gwClient.GetRemoteSources(ctx, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) + var respAllRemoteSources []*gwapitypes.RemoteSourceResponse - expectedRemoteSources := 5 - assert.Assert(t, cmp.Len(respRemoteSources, expectedRemoteSources)) - assert.Assert(t, resp.Cursor != "") + respRemoteSources, res, err := gwClient.GetRemoteSources(ctx, &gwclient.ListOptions{ + Limit: tt.limit, SortDirection: tt.sortDirection, + }) + testutil.NilError(t, err) - respAllRemoteSources = append(respAllRemoteSources, respRemoteSources...) + respAllRemoteSources = append(respAllRemoteSources, respRemoteSources...) + callsNumber := 1 - // fetch next results - for { - respRemoteSources, resp, err = gwClient.GetRemoteSources(ctx, &gwclient.ListOptions{Cursor: resp.Cursor, Limit: 5}) - testutil.NilError(t, err) + // fetch next results + for { + if res.Cursor == "" { + break + } - expectedRemoteSources := 5 - assert.Assert(t, resp.Cursor == "" || (len(respRemoteSources) == expectedRemoteSources)) + respRemoteSources, res, err = gwClient.GetRemoteSources(ctx, &gwclient.ListOptions{ + Cursor: res.Cursor, Limit: tt.limit, + }) + testutil.NilError(t, err) - respAllRemoteSources = append(respAllRemoteSources, respRemoteSources...) + callsNumber++ - if resp.Cursor == "" { - break + respAllRemoteSources = append(respAllRemoteSources, respRemoteSources...) } - } - expectedRemoteSources = 9 - assert.Assert(t, cmp.Len(respAllRemoteSources, expectedRemoteSources)) - - assert.DeepEqual(t, remoteSources, respAllRemoteSources) - }) + assert.Assert(t, cmp.Len(respAllRemoteSources, len(expectedRemoteSources))) + assert.DeepEqual(t, respAllRemoteSources, expectedRemoteSources) + assert.Assert(t, cmp.Equal(callsNumber, tt.expectedCallsNumber)) + }) + } } func TestGetUsers(t *testing.T) { From 136908541fb0cdfb788b60b1135fc1561c445eba Mon Sep 17 00:00:00 2001 From: "alessandro.pinna" Date: Fri, 22 Dec 2023 14:46:21 +0100 Subject: [PATCH 4/7] gateway: add missing sortdirection to get users cursor the SortDirection field in the returned cursor wasn't set. Set it to the right value. Convert tests to matrix tests to reduce code duplication and add tests with both sort directions. --- internal/services/gateway/action/user.go | 5 +- tests/setup_test.go | 121 +++++++++++++++-------- 2 files changed, 82 insertions(+), 44 deletions(-) diff --git a/internal/services/gateway/action/user.go b/internal/services/gateway/action/user.go index 1a883beb4..5350ae995 100644 --- a/internal/services/gateway/action/user.go +++ b/internal/services/gateway/action/user.go @@ -174,7 +174,10 @@ func (h *ActionHandler) GetUsers(ctx context.Context, req *GetUsersRequest) (*Ge var outCursor string if resp.HasMore && len(csusers) > 0 { lastUserName := csusers[len(csusers)-1].Name - outCursor, err = MarshalCursor(&StartCursor{Start: lastUserName}) + outCursor, err = MarshalCursor(&StartCursor{ + Start: lastUserName, + SortDirection: sortDirection, + }) if err != nil { return nil, errors.WithStack(err) } diff --git a/tests/setup_test.go b/tests/setup_test.go index 1f0434e8d..908ea00cf 100644 --- a/tests/setup_test.go +++ b/tests/setup_test.go @@ -4009,60 +4009,95 @@ func TestGetUsers(t *testing.T) { users = append(users, user) } - t.Run("test get users with default limit greater than users", func(t *testing.T) { - users, resp, err := gwClient.GetUsers(ctx, &gwclient.ListOptions{SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedUsers := 9 - assert.Assert(t, cmp.Len(users, expectedUsers)) - assert.Assert(t, resp.Cursor == "") - }) - - t.Run("test get users with limit less than users", func(t *testing.T) { - users, resp, err := gwClient.GetUsers(ctx, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedUsers := 5 - assert.Assert(t, cmp.Len(users, expectedUsers)) - assert.Assert(t, resp.Cursor != "") - }) - - t.Run("test get users with limit less than users continuation", func(t *testing.T) { - respAllUsers := []*gwapitypes.UserResponse{} - - respUsers, resp, err := gwClient.GetUsers(ctx, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) + tests := []struct { + name string + limit int + sortDirection gwapitypes.SortDirection + expectedCallsNumber int + }{ + { + name: "test get users with limit = 0", + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get users with limit less than users continuation", + limit: 2, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 5, + }, + { + name: "test get users with limit greater than users", + limit: 10, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get users with limit = 0 and sortDirection desc", + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + { + name: "test get users with limit less than users continuation and sortDirection desc", + limit: 2, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 5, + }, + { + name: "test get users with limit greater than users and sortDirection desc", + limit: 10, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + } - expectedUsers := 5 - assert.Assert(t, cmp.Len(respUsers, expectedUsers)) - assert.Assert(t, resp.Cursor != "") + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + expectedUsers := append([]*gwapitypes.UserResponse{}, users...) + // reverse if sortDirection is desc + // TODO(sgotti) use go 1.21 generics slices.Reverse when removing support for go < 1.21 + if tt.sortDirection == gwapitypes.SortDirectionDesc { + for i, j := 0, len(expectedUsers)-1; i < j; i, j = i+1, j-1 { + expectedUsers[i], expectedUsers[j] = expectedUsers[j], expectedUsers[i] + } + } - for _, respUser := range respUsers { - respAllUsers = append(respAllUsers, &gwapitypes.UserResponse{ID: respUser.ID, UserName: respUser.UserName}) - } + var respAllUsers []*gwapitypes.UserResponse - // fetch next results - for { - respUsers, resp, err = gwClient.GetUsers(ctx, &gwclient.ListOptions{Cursor: resp.Cursor, Limit: 5}) + respUsers, res, err := gwClient.GetUsers(ctx, &gwclient.ListOptions{ + Limit: tt.limit, SortDirection: tt.sortDirection, + }) testutil.NilError(t, err) - expectedUsers := 5 - assert.Assert(t, resp.Cursor == "" || (len(respUsers) == expectedUsers)) - for _, respUser := range respUsers { respAllUsers = append(respAllUsers, &gwapitypes.UserResponse{ID: respUser.ID, UserName: respUser.UserName}) } + callsNumber := 1 - if resp.Cursor == "" { - break - } - } + // fetch next results + for { + if res.Cursor == "" { + break + } - expectedUsers = 9 - assert.Assert(t, cmp.Len(respAllUsers, expectedUsers)) + respUsers, res, err = gwClient.GetUsers(ctx, &gwclient.ListOptions{ + Cursor: res.Cursor, Limit: tt.limit, + }) + testutil.NilError(t, err) - assert.DeepEqual(t, users, respAllUsers) - }) + callsNumber++ + + for _, respUser := range respUsers { + respAllUsers = append(respAllUsers, &gwapitypes.UserResponse{ID: respUser.ID, UserName: respUser.UserName}) + } + } + + assert.Assert(t, cmp.Len(respAllUsers, len(expectedUsers))) + assert.DeepEqual(t, respAllUsers, expectedUsers) + assert.Assert(t, cmp.Equal(callsNumber, tt.expectedCallsNumber)) + }) + } } func TestGetOrgs(t *testing.T) { From d8f8b2497d2b38b16b2337cf921baaf81abdda5f Mon Sep 17 00:00:00 2001 From: "alessandro.pinna" Date: Fri, 22 Dec 2023 14:57:18 +0100 Subject: [PATCH 5/7] gateway: add missing sortdirection to get user orgs cursor the SortDirection field in the returned cursor wasn't set. Set it to the right value. Convert tests to matrix tests to reduce code duplication and add tests with both sort directions. --- internal/services/gateway/action/user.go | 5 +- tests/setup_test.go | 123 +++++++++++++++-------- 2 files changed, 83 insertions(+), 45 deletions(-) diff --git a/internal/services/gateway/action/user.go b/internal/services/gateway/action/user.go index 5350ae995..334882011 100644 --- a/internal/services/gateway/action/user.go +++ b/internal/services/gateway/action/user.go @@ -125,7 +125,10 @@ func (h *ActionHandler) GetUserOrgs(ctx context.Context, req *GetUserOrgsRequest var outCursor string if resp.HasMore && len(orgs) > 0 { lastOrgName := orgs[len(orgs)-1].Organization.Name - outCursor, err = MarshalCursor(&StartCursor{Start: lastOrgName}) + outCursor, err = MarshalCursor(&StartCursor{ + Start: lastOrgName, + SortDirection: sortDirection, + }) if err != nil { return nil, errors.WithStack(err) } diff --git a/tests/setup_test.go b/tests/setup_test.go index 908ea00cf..712a9a7b1 100644 --- a/tests/setup_test.go +++ b/tests/setup_test.go @@ -4434,60 +4434,95 @@ func TestGetUserOrgs(t *testing.T) { gwClient = gwclient.NewClient(sc.config.Gateway.APIExposedURL, tokenUser.Token) - t.Run("test get user orgs with default limit greater than user orgs", func(t *testing.T) { - userOrgs, resp, err := gwClient.GetUserOrgs(ctx, &gwclient.ListOptions{SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedUserOrgs := 9 - assert.Assert(t, cmp.Len(userOrgs, expectedUserOrgs)) - assert.Assert(t, resp.Cursor == "") - }) - - t.Run("test get user orgs with limit less than user orgs", func(t *testing.T) { - res, resp, err := gwClient.GetUserOrgs(ctx, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedUserOrgs := 5 - assert.Assert(t, cmp.Len(res, expectedUserOrgs)) - assert.Assert(t, resp.Cursor != "") - }) + tests := []struct { + name string + limit int + sortDirection gwapitypes.SortDirection + expectedCallsNumber int + }{ + { + name: "test get user orgs with limit = 0", + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get user orgs with limit less than user orgs continuation", + limit: 2, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 5, + }, + { + name: "test get user orgs with limit greater than user orgs", + limit: 10, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get user orgs with limit = 0 and sortDirection desc", + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + { + name: "test get user orgs with limit less than user orgs continuation and sortDirection desc", + limit: 2, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 5, + }, + { + name: "test get user orgs with limit greater than user orgs and sortDirection desc", + limit: 10, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + } - t.Run("test get user orgs with limit less than user orgs continuation", func(t *testing.T) { - respAllUserOrgs := []*gwapitypes.UserOrgResponse{} + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + expectedUserOrgs := append([]*gwapitypes.OrgResponse{}, orgs...) + // reverse if sortDirection is desc + // TODO(sgotti) use go 1.21 generics slices.Reverse when removing support for go < 1.21 + if tt.sortDirection == gwapitypes.SortDirectionDesc { + for i, j := 0, len(expectedUserOrgs)-1; i < j; i, j = i+1, j-1 { + expectedUserOrgs[i], expectedUserOrgs[j] = expectedUserOrgs[j], expectedUserOrgs[i] + } + } - respUserOrgs, resp, err := gwClient.GetUserOrgs(ctx, &gwclient.ListOptions{Limit: 5, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) + var respAllUserOrgs []*gwapitypes.OrgResponse - expectedUserOrgs := 5 - assert.Assert(t, cmp.Len(respUserOrgs, expectedUserOrgs)) - assert.Assert(t, resp.Cursor != "") + respUserOrgs, res, err := gwClient.GetUserOrgs(ctx, &gwclient.ListOptions{ + Limit: tt.limit, SortDirection: tt.sortDirection, + }) + testutil.NilError(t, err) - respAllUserOrgs = append(respAllUserOrgs, respUserOrgs...) + for _, userOrg := range respUserOrgs { + respAllUserOrgs = append(respAllUserOrgs, userOrg.Organization) + } + callsNumber := 1 - // fetch next results - for { - respUserOrgs, resp, err = gwClient.GetUserOrgs(ctx, &gwclient.ListOptions{Cursor: resp.Cursor, Limit: 5}) - testutil.NilError(t, err) + // fetch next results + for { + if res.Cursor == "" { + break + } - expectedUserOrgs := 5 - assert.Assert(t, resp.Cursor == "" || (len(respUserOrgs) == expectedUserOrgs)) + respUserOrgs, res, err = gwClient.GetUserOrgs(ctx, &gwclient.ListOptions{ + Cursor: res.Cursor, Limit: tt.limit, + }) + testutil.NilError(t, err) - respAllUserOrgs = append(respAllUserOrgs, respUserOrgs...) + callsNumber++ - if resp.Cursor == "" { - break + for _, userOrg := range respUserOrgs { + respAllUserOrgs = append(respAllUserOrgs, userOrg.Organization) + } } - } - - expectedUserOrgs = 9 - assert.Assert(t, cmp.Len(respAllUserOrgs, expectedUserOrgs)) - userOrgs := []*gwapitypes.OrgResponse{} - for _, userOrg := range respAllUserOrgs { - userOrgs = append(userOrgs, userOrg.Organization) - } - assert.DeepEqual(t, orgs, userOrgs) - }) + assert.Assert(t, cmp.Len(respAllUserOrgs, len(expectedUserOrgs))) + assert.DeepEqual(t, respAllUserOrgs, expectedUserOrgs) + assert.Assert(t, cmp.Equal(callsNumber, tt.expectedCallsNumber)) + }) + } } func TestMaintenance(t *testing.T) { From ca744b0edd6017c93256066132c4b1d8ed16d41c Mon Sep 17 00:00:00 2001 From: "alessandro.pinna" Date: Wed, 27 Dec 2023 09:25:06 +0100 Subject: [PATCH 6/7] gateway: add missing sortdirection to get project commit status deliveries cursor the SortDirection field in the returned cursor wasn't set. Set it to the right value. Convert tests to matrix tests to reduce code duplication and add tests with both sort directions. --- .../gateway/action/commitstatusdelivery.go | 5 +- tests/setup_test.go | 196 ++++++++++++------ 2 files changed, 142 insertions(+), 59 deletions(-) diff --git a/internal/services/gateway/action/commitstatusdelivery.go b/internal/services/gateway/action/commitstatusdelivery.go index 62d764ef8..8d4105b61 100644 --- a/internal/services/gateway/action/commitstatusdelivery.go +++ b/internal/services/gateway/action/commitstatusdelivery.go @@ -69,7 +69,10 @@ func (h *ActionHandler) GetProjectCommitStatusDeliveries(ctx context.Context, re var outCursor string if resp.HasMore && len(commitStatusDeliveries) > 0 { lastCommitStatusDeliverySequence := commitStatusDeliveries[len(commitStatusDeliveries)-1].Sequence - outCursor, err = MarshalCursor(&StartSequenceCursor{StartSequence: lastCommitStatusDeliverySequence}) + outCursor, err = MarshalCursor(&StartSequenceCursor{ + StartSequence: lastCommitStatusDeliverySequence, + SortDirection: sortDirection, + }) if err != nil { return nil, errors.WithStack(err) } diff --git a/tests/setup_test.go b/tests/setup_test.go index 712a9a7b1..c16712761 100644 --- a/tests/setup_test.go +++ b/tests/setup_test.go @@ -6301,75 +6301,155 @@ func TestGetProjectCommitStatusDeliveries(t *testing.T) { return true, nil }) - t.Run("test get project commit status deliveries", func(t *testing.T) { - commitStatusDeliveries, resp, err := gwUser01Client.GetProjectCommitStatusDeliveries(ctx, project.ID, nil, &gwclient.ListOptions{Limit: 0, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - assert.Assert(t, cmp.Len(commitStatusDeliveries, 2*runCount)) - assert.Assert(t, resp.Cursor == "") - for _, r := range commitStatusDeliveries { - assert.Equal(t, r.DeliveryStatus, gwapitypes.DeliveryStatusDelivered) - } - }) - - t.Run("test get project commit status deliveries with limit less than project commit status deliveries continuation", func(t *testing.T) { - commitStatusDeliveries, _, err := gwUser01Client.GetProjectCommitStatusDeliveries(ctx, project.ID, nil, &gwclient.ListOptions{SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - respAllCommitStatusDeliveries := []*gwapitypes.CommitStatusDeliveryResponse{} - - respCommitStatusDeliveries, resp, err := gwUser01Client.GetProjectCommitStatusDeliveries(ctx, project.ID, nil, &gwclient.ListOptions{Limit: 1, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedCommitStatusDeliveries := 1 - assert.Assert(t, cmp.Len(respCommitStatusDeliveries, expectedCommitStatusDeliveries)) - assert.Assert(t, resp.Cursor != "") - - respAllCommitStatusDeliveries = append(respAllCommitStatusDeliveries, respCommitStatusDeliveries...) + commitStatusDeliveries, resp, err := gwUser01Client.GetProjectCommitStatusDeliveries(ctx, project.ID, nil, &gwclient.ListOptions{SortDirection: gwapitypes.SortDirectionAsc}) + testutil.NilError(t, err) - // fetch next results - for { - respCommitStatusDeliveries, resp, err = gwUser01Client.GetProjectCommitStatusDeliveries(ctx, project.ID, nil, &gwclient.ListOptions{Cursor: resp.Cursor, Limit: 1}) - testutil.NilError(t, err) + assert.Assert(t, cmp.Len(commitStatusDeliveries, 2*runCount)) + assert.Assert(t, resp.Cursor == "") + for _, r := range commitStatusDeliveries { + assert.Assert(t, cmp.Equal(r.DeliveryStatus, gwapitypes.DeliveryStatusDelivered)) + } - assert.Assert(t, resp.Cursor == "" || (len(respCommitStatusDeliveries) == expectedCommitStatusDeliveries)) + tests := []struct { + name string + client *gwclient.Client + projectRef string + limit int + sortDirection gwapitypes.SortDirection + deliveryStatusFilter []string + expectedCallsNumber int + expectedErr string + }{ + { + name: "test get project commit status deliveries with limit = 0", + client: gwUser01Client, + projectRef: project.ID, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get project commit status deliveries with limit less than project commit status deliveries continuation", + client: gwUser01Client, + projectRef: project.ID, + limit: 2, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: runCount, + }, + { + name: "test get project commit status deliveries with limit greater than project commit status deliveries", + client: gwUser01Client, + projectRef: project.ID, + limit: 10, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get project commit status deliveries with limit = 0 and sortDirection desc", + client: gwUser01Client, + projectRef: project.ID, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + { + name: "test get project commit status deliveries with limit less than project commit status deliveries continuation and sortDirection desc", + client: gwUser01Client, + projectRef: project.ID, + limit: 2, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: runCount, + }, + { + name: "test get project commit status deliveries with limit greater than project commit status deliveries and sortDirection desc", + client: gwUser01Client, + projectRef: project.ID, + limit: 10, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + { + name: "test get project commit status deliveries with user unauthorized", + client: gwUser02Client, + projectRef: project.ID, + sortDirection: gwapitypes.SortDirectionAsc, + expectedErr: remoteErrorForbidden, + }, + { + name: "test get project commit status deliveries with not existing project", + client: gwUser01Client, + projectRef: "project02", + sortDirection: gwapitypes.SortDirectionAsc, + expectedErr: remoteErrorNotExist, + }, + { + name: "test get project commit status deliveries with deliverystatus = delivered", + client: gwUser01Client, + projectRef: project.ID, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get project commit status deliveries with deliverystatus = deliveryError", + client: gwUser01Client, + projectRef: project.ID, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + } - respAllCommitStatusDeliveries = append(respAllCommitStatusDeliveries, respCommitStatusDeliveries...) + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + // populate the expected commit status deliveries + expectedProject01CommitStatusDeliveries := []*gwapitypes.CommitStatusDeliveryResponse{} + for _, c := range commitStatusDeliveries { + if len(tt.deliveryStatusFilter) > 0 && !util.StringInSlice(tt.deliveryStatusFilter, string(c.DeliveryStatus)) { + continue + } + expectedProject01CommitStatusDeliveries = append(expectedProject01CommitStatusDeliveries, c) + } - if resp.Cursor == "" { - break + // reverse if sortDirection is desc + // TODO(sgotti) use go 1.21 generics slices.Reverse when removing support for go < 1.21 + if tt.sortDirection == gwapitypes.SortDirectionDesc { + for i, j := 0, len(expectedProject01CommitStatusDeliveries)-1; i < j; i, j = i+1, j-1 { + expectedProject01CommitStatusDeliveries[i], expectedProject01CommitStatusDeliveries[j] = expectedProject01CommitStatusDeliveries[j], expectedProject01CommitStatusDeliveries[i] + } } - } - expectedCommitStatusDeliveries = 2 * runCount - assert.Assert(t, cmp.Len(respAllCommitStatusDeliveries, expectedCommitStatusDeliveries)) + var respAllCommitStatusDeliveries []*gwapitypes.CommitStatusDeliveryResponse - assert.DeepEqual(t, commitStatusDeliveries, respAllCommitStatusDeliveries) - }) + respCommitStatusDeliveries, res, err := tt.client.GetProjectCommitStatusDeliveries(ctx, tt.projectRef, tt.deliveryStatusFilter, &gwclient.ListOptions{ + Limit: tt.limit, SortDirection: tt.sortDirection, + }) + if tt.expectedErr == "" { + testutil.NilError(t, err) + } else { + assert.Error(t, err, tt.expectedErr) + return + } - t.Run("test get project commit status deliveries with user unauthorized", func(t *testing.T) { - _, _, err = gwUser02Client.GetProjectCommitStatusDeliveries(ctx, project.ID, nil, &gwclient.ListOptions{Limit: 0, SortDirection: gwapitypes.SortDirectionAsc}) - assert.Error(t, err, remoteErrorForbidden) - }) + respAllCommitStatusDeliveries = append(respAllCommitStatusDeliveries, respCommitStatusDeliveries...) + callsNumber := 1 - t.Run("test get project commit status deliveries with not existing project", func(t *testing.T) { - _, _, err = gwUser01Client.GetProjectCommitStatusDeliveries(ctx, "project02", nil, &gwclient.ListOptions{Limit: 0, SortDirection: gwapitypes.SortDirectionAsc}) - assert.Error(t, err, remoteErrorNotExist) - }) + // fetch next results + for { + if res.Cursor == "" { + break + } - t.Run("test get project commit status deliveries with deliverystatus = delivered", func(t *testing.T) { - commitStatusDeliveries, resp, err := gwUser01Client.GetProjectCommitStatusDeliveries(ctx, project.ID, []string{string(nstypes.DeliveryStatusDelivered)}, &gwclient.ListOptions{Limit: 0, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) + respCommitStatusDeliveries, res, err = tt.client.GetProjectCommitStatusDeliveries(ctx, tt.projectRef, tt.deliveryStatusFilter, &gwclient.ListOptions{ + Cursor: res.Cursor, Limit: tt.limit, + }) + testutil.NilError(t, err) - assert.Assert(t, cmp.Len(commitStatusDeliveries, 2*runCount)) - assert.Assert(t, resp.Cursor == "") - }) + callsNumber++ - t.Run("test get project commit status deliveries with deliverystatus = deliveryError", func(t *testing.T) { - commitStatusDeliveries, resp, err := gwUser01Client.GetProjectCommitStatusDeliveries(ctx, project.ID, []string{string(nstypes.DeliveryStatusDeliveryError)}, &gwclient.ListOptions{Limit: 0, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) + respAllCommitStatusDeliveries = append(respAllCommitStatusDeliveries, respCommitStatusDeliveries...) + } - assert.Assert(t, cmp.Len(commitStatusDeliveries, 0)) - assert.Assert(t, resp.Cursor == "") - }) + assert.Assert(t, cmp.Len(respAllCommitStatusDeliveries, len(expectedProject01CommitStatusDeliveries))) + assert.DeepEqual(t, respAllCommitStatusDeliveries, expectedProject01CommitStatusDeliveries) + assert.Assert(t, cmp.Equal(callsNumber, tt.expectedCallsNumber)) + }) + } } From 49bb1fcdd11fe2944df6b12b5ef0bc8e459a7c1b Mon Sep 17 00:00:00 2001 From: "alessandro.pinna" Date: Wed, 27 Dec 2023 11:48:30 +0100 Subject: [PATCH 7/7] gateway: add missing sortdirection to get project run webhook deliveries cursor the SortDirection field in the returned cursor wasn't set. Set it to the right value. Convert tests to matrix tests to reduce code duplication and add tests with both sort directions. --- .../gateway/action/runwebhookdelivery.go | 5 +- tests/setup_test.go | 193 ++++++++++++------ 2 files changed, 137 insertions(+), 61 deletions(-) diff --git a/internal/services/gateway/action/runwebhookdelivery.go b/internal/services/gateway/action/runwebhookdelivery.go index fc7bcd365..bba86b830 100644 --- a/internal/services/gateway/action/runwebhookdelivery.go +++ b/internal/services/gateway/action/runwebhookdelivery.go @@ -69,7 +69,10 @@ func (h *ActionHandler) GetProjectRunWebhookDeliveries(ctx context.Context, req var outCursor string if resp.HasMore && len(runWebhookDeliveries) > 0 { lastRunWebhookDeliverySequence := runWebhookDeliveries[len(runWebhookDeliveries)-1].Sequence - outCursor, err = MarshalCursor(&StartSequenceCursor{StartSequence: lastRunWebhookDeliverySequence}) + outCursor, err = MarshalCursor(&StartSequenceCursor{ + StartSequence: lastRunWebhookDeliverySequence, + SortDirection: sortDirection, + }) if err != nil { return nil, errors.WithStack(err) } diff --git a/tests/setup_test.go b/tests/setup_test.go index c16712761..6347ece90 100644 --- a/tests/setup_test.go +++ b/tests/setup_test.go @@ -65,7 +65,6 @@ import ( cstypes "agola.io/agola/services/configstore/types" gwapitypes "agola.io/agola/services/gateway/api/types" gwclient "agola.io/agola/services/gateway/client" - nstypes "agola.io/agola/services/notification/types" rstypes "agola.io/agola/services/runservice/types" ) @@ -5762,77 +5761,151 @@ func TestGetProjectRunWebhookDeliveries(t *testing.T) { return true, nil }) - t.Run("test get project run webhook deliveries", func(t *testing.T) { - runWebhookDeliveries, resp, err := gwUser01Client.GetProjectRunWebhookDeliveries(ctx, project.ID, nil, &gwclient.ListOptions{Limit: 0, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - assert.Assert(t, cmp.Len(runWebhookDeliveries, 4)) - assert.Assert(t, resp.Cursor == "") - for _, r := range runWebhookDeliveries { - assert.Equal(t, r.DeliveryStatus, gwapitypes.DeliveryStatusDelivered) - } - }) - - t.Run("test get project run webhook deliveries with limit less than project run webhook deliveries continuation", func(t *testing.T) { - runWebhookDeliveries, _, err := gwUser01Client.GetProjectRunWebhookDeliveries(ctx, project.ID, nil, &gwclient.ListOptions{SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - respAllRunWebhookDeliveries := []*gwapitypes.RunWebhookDeliveryResponse{} - - respRunWebhookDeliveries, resp, err := gwUser01Client.GetProjectRunWebhookDeliveries(ctx, project.ID, nil, &gwclient.ListOptions{Limit: 1, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) - - expectedRunWebhookDeliveries := 1 - assert.Assert(t, cmp.Len(respRunWebhookDeliveries, expectedRunWebhookDeliveries)) - assert.Assert(t, resp.Cursor != "") - - respAllRunWebhookDeliveries = append(respAllRunWebhookDeliveries, respRunWebhookDeliveries...) - - // fetch next results - for { - respRunWebhookDeliveries, resp, err = gwUser01Client.GetProjectRunWebhookDeliveries(ctx, project.ID, nil, &gwclient.ListOptions{Cursor: resp.Cursor, Limit: 1}) - testutil.NilError(t, err) + runWebhookDeliveries, _, err := gwUser01Client.GetProjectRunWebhookDeliveries(ctx, project.ID, nil, &gwclient.ListOptions{Limit: 0, SortDirection: gwapitypes.SortDirectionAsc}) + testutil.NilError(t, err) - assert.Assert(t, resp.Cursor == "" || (len(respRunWebhookDeliveries) == expectedRunWebhookDeliveries)) + tests := []struct { + name string + client *gwclient.Client + projectRef string + limit int + sortDirection gwapitypes.SortDirection + deliveryStatusFilter []string + expectedCallsNumber int + expectedErr string + }{ + { + name: "test get project run webhook deliveries with limit = 0", + client: gwUser01Client, + projectRef: project.ID, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get project run webhook deliveries with limit less than project run webhook deliveries continuation", + client: gwUser01Client, + projectRef: project.ID, + limit: 2, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 2, + }, + { + name: "test get project run webhook deliveries with limit greater than project run webhook deliveries", + client: gwUser01Client, + projectRef: project.ID, + limit: 5, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get project run webhook deliveries with limit = 0 and sortDirection desc", + client: gwUser01Client, + projectRef: project.ID, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + { + name: "test get project run webhook deliveries with limit less than project run webhook deliveries continuation and sortDirection desc", + client: gwUser01Client, + projectRef: project.ID, + limit: 2, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 2, + }, + { + name: "test get project run webhook deliveries with limit greater than project run webhook deliveries and sortDirection desc", + client: gwUser01Client, + projectRef: project.ID, + limit: 5, + sortDirection: gwapitypes.SortDirectionDesc, + expectedCallsNumber: 1, + }, + { + name: "test get project run webhook deliveries with user unauthorized", + client: gwUser02Client, + projectRef: project.ID, + sortDirection: gwapitypes.SortDirectionAsc, + expectedErr: remoteErrorForbidden, + }, + { + name: "test get project run webhook deliveries with not existing project", + client: gwUser01Client, + projectRef: "project02", + sortDirection: gwapitypes.SortDirectionAsc, + expectedErr: remoteErrorNotExist, + }, + { + name: "test get project run webhook deliveries with deliverystatus = delivered", + client: gwUser01Client, + projectRef: project.ID, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + { + name: "test get project run webhook deliveries with deliverystatus = deliveryError", + client: gwUser01Client, + projectRef: project.ID, + sortDirection: gwapitypes.SortDirectionAsc, + expectedCallsNumber: 1, + }, + } - respAllRunWebhookDeliveries = append(respAllRunWebhookDeliveries, respRunWebhookDeliveries...) + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + // populate the expected commit status deliveries + expectedProject01RunWebhookDeliveries := []*gwapitypes.RunWebhookDeliveryResponse{} + for _, r := range runWebhookDeliveries { + if len(tt.deliveryStatusFilter) > 0 && !util.StringInSlice(tt.deliveryStatusFilter, string(r.DeliveryStatus)) { + continue + } + expectedProject01RunWebhookDeliveries = append(expectedProject01RunWebhookDeliveries, r) + } - if resp.Cursor == "" { - break + // reverse if sortDirection is desc + // TODO(sgotti) use go 1.21 generics slices.Reverse when removing support for go < 1.21 + if tt.sortDirection == gwapitypes.SortDirectionDesc { + for i, j := 0, len(expectedProject01RunWebhookDeliveries)-1; i < j; i, j = i+1, j-1 { + expectedProject01RunWebhookDeliveries[i], expectedProject01RunWebhookDeliveries[j] = expectedProject01RunWebhookDeliveries[j], expectedProject01RunWebhookDeliveries[i] + } } - } - expectedRunWebhookDeliveries = 4 - assert.Assert(t, cmp.Len(respAllRunWebhookDeliveries, expectedRunWebhookDeliveries)) + var respAllRunWebhookDeliveries []*gwapitypes.RunWebhookDeliveryResponse - assert.DeepEqual(t, runWebhookDeliveries, respAllRunWebhookDeliveries) - }) + respRunWebhookDeliveries, res, err := tt.client.GetProjectRunWebhookDeliveries(ctx, tt.projectRef, tt.deliveryStatusFilter, &gwclient.ListOptions{ + Limit: tt.limit, SortDirection: tt.sortDirection, + }) + if tt.expectedErr == "" { + testutil.NilError(t, err) + } else { + assert.Error(t, err, tt.expectedErr) + return + } - t.Run("test get project run webhook deliveries with user unauthorized", func(t *testing.T) { - _, _, err = gwUser02Client.GetProjectRunWebhookDeliveries(ctx, project.ID, nil, &gwclient.ListOptions{Limit: 0, SortDirection: gwapitypes.SortDirectionAsc}) - assert.Error(t, err, remoteErrorForbidden) - }) + respAllRunWebhookDeliveries = append(respAllRunWebhookDeliveries, respRunWebhookDeliveries...) + callsNumber := 1 - t.Run("test get project run webhook deliveries with not existing project", func(t *testing.T) { - _, _, err = gwUser01Client.GetProjectRunWebhookDeliveries(ctx, "project02", nil, &gwclient.ListOptions{Limit: 0, SortDirection: gwapitypes.SortDirectionAsc}) - assert.Error(t, err, remoteErrorNotExist) - }) + // fetch next results + for { + if res.Cursor == "" { + break + } - t.Run("test get project run webhook deliveries with deliverystatus = delivered", func(t *testing.T) { - runWebhookDeliveries, resp, err := gwUser01Client.GetProjectRunWebhookDeliveries(ctx, project.ID, []string{string(nstypes.DeliveryStatusDelivered)}, &gwclient.ListOptions{Limit: 0, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) + respRunWebhookDeliveries, res, err = tt.client.GetProjectRunWebhookDeliveries(ctx, tt.projectRef, tt.deliveryStatusFilter, &gwclient.ListOptions{ + Cursor: res.Cursor, Limit: tt.limit, + }) + testutil.NilError(t, err) - assert.Assert(t, cmp.Len(runWebhookDeliveries, 4)) - assert.Assert(t, resp.Cursor == "") - }) + callsNumber++ - t.Run("test get project run webhook deliveries with deliverystatus = deliveryError", func(t *testing.T) { - runWebhookDeliveries, resp, err := gwUser01Client.GetProjectRunWebhookDeliveries(ctx, project.ID, []string{string(nstypes.DeliveryStatusDeliveryError)}, &gwclient.ListOptions{Limit: 0, SortDirection: gwapitypes.SortDirectionAsc}) - testutil.NilError(t, err) + respAllRunWebhookDeliveries = append(respAllRunWebhookDeliveries, respRunWebhookDeliveries...) + } - assert.Assert(t, cmp.Len(runWebhookDeliveries, 0)) - assert.Assert(t, resp.Cursor == "") - }) + assert.Assert(t, cmp.Len(respAllRunWebhookDeliveries, len(expectedProject01RunWebhookDeliveries))) + assert.DeepEqual(t, respAllRunWebhookDeliveries, expectedProject01RunWebhookDeliveries) + assert.Assert(t, cmp.Equal(callsNumber, tt.expectedCallsNumber)) + }) + } } func TestProjectRunWebhookRedelivery(t *testing.T) {