GitLab reviews may not have the updated_at field set (#18450)
* GitLab reviews may not have the updated_at field set Fallback to created_at if that the case and to time.Now() if it is also missing. Fixes: 18434 * use assert.WithinDuration Co-authored-by: Loïc Dachary <loic@dachary.org>
This commit is contained in:
parent
2ad74a503d
commit
e19b9653ea
3 changed files with 159 additions and 10 deletions
|
@ -640,13 +640,22 @@ func (g *GitlabDownloader) GetReviews(context base.IssueContext) ([]*base.Review
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
var createdAt time.Time
|
||||||
|
if approvals.CreatedAt != nil {
|
||||||
|
createdAt = *approvals.CreatedAt
|
||||||
|
} else if approvals.UpdatedAt != nil {
|
||||||
|
createdAt = *approvals.UpdatedAt
|
||||||
|
} else {
|
||||||
|
createdAt = time.Now()
|
||||||
|
}
|
||||||
|
|
||||||
reviews := make([]*base.Review, 0, len(approvals.ApprovedBy))
|
reviews := make([]*base.Review, 0, len(approvals.ApprovedBy))
|
||||||
for _, user := range approvals.ApprovedBy {
|
for _, user := range approvals.ApprovedBy {
|
||||||
reviews = append(reviews, &base.Review{
|
reviews = append(reviews, &base.Review{
|
||||||
IssueIndex: context.LocalID(),
|
IssueIndex: context.LocalID(),
|
||||||
ReviewerID: int64(user.User.ID),
|
ReviewerID: int64(user.User.ID),
|
||||||
ReviewerName: user.User.Username,
|
ReviewerName: user.User.Username,
|
||||||
CreatedAt: *approvals.UpdatedAt,
|
CreatedAt: createdAt,
|
||||||
// All we get are approvals
|
// All we get are approvals
|
||||||
State: base.ReviewStateApproved,
|
State: base.ReviewStateApproved,
|
||||||
})
|
})
|
||||||
|
|
|
@ -8,13 +8,16 @@ import (
|
||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
"os"
|
"os"
|
||||||
|
"strconv"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
base "code.gitea.io/gitea/modules/migration"
|
base "code.gitea.io/gitea/modules/migration"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/xanzy/go-gitlab"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestGitlabDownloadRepo(t *testing.T) {
|
func TestGitlabDownloadRepo(t *testing.T) {
|
||||||
|
@ -310,12 +313,14 @@ func TestGitlabDownloadRepo(t *testing.T) {
|
||||||
assert.NoError(t, err)
|
assert.NoError(t, err)
|
||||||
assertReviewsEqual(t, []*base.Review{
|
assertReviewsEqual(t, []*base.Review{
|
||||||
{
|
{
|
||||||
|
IssueIndex: 1,
|
||||||
ReviewerID: 4102996,
|
ReviewerID: 4102996,
|
||||||
ReviewerName: "zeripath",
|
ReviewerName: "zeripath",
|
||||||
CreatedAt: time.Date(2019, 11, 28, 16, 2, 8, 377000000, time.UTC),
|
CreatedAt: time.Date(2019, 11, 28, 16, 2, 8, 377000000, time.UTC),
|
||||||
State: "APPROVED",
|
State: "APPROVED",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
|
IssueIndex: 1,
|
||||||
ReviewerID: 527793,
|
ReviewerID: 527793,
|
||||||
ReviewerName: "axifive",
|
ReviewerName: "axifive",
|
||||||
CreatedAt: time.Date(2019, 11, 28, 16, 2, 8, 377000000, time.UTC),
|
CreatedAt: time.Date(2019, 11, 28, 16, 2, 8, 377000000, time.UTC),
|
||||||
|
@ -327,6 +332,7 @@ func TestGitlabDownloadRepo(t *testing.T) {
|
||||||
assert.NoError(t, err)
|
assert.NoError(t, err)
|
||||||
assertReviewsEqual(t, []*base.Review{
|
assertReviewsEqual(t, []*base.Review{
|
||||||
{
|
{
|
||||||
|
IssueIndex: 2,
|
||||||
ReviewerID: 4575606,
|
ReviewerID: 4575606,
|
||||||
ReviewerName: "real6543",
|
ReviewerName: "real6543",
|
||||||
CreatedAt: time.Date(2020, 4, 19, 19, 24, 21, 108000000, time.UTC),
|
CreatedAt: time.Date(2020, 4, 19, 19, 24, 21, 108000000, time.UTC),
|
||||||
|
@ -334,3 +340,137 @@ func TestGitlabDownloadRepo(t *testing.T) {
|
||||||
},
|
},
|
||||||
}, rvs)
|
}, rvs)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func gitlabClientMockSetup(t *testing.T) (*http.ServeMux, *httptest.Server, *gitlab.Client) {
|
||||||
|
// mux is the HTTP request multiplexer used with the test server.
|
||||||
|
mux := http.NewServeMux()
|
||||||
|
|
||||||
|
// server is a test HTTP server used to provide mock API responses.
|
||||||
|
server := httptest.NewServer(mux)
|
||||||
|
|
||||||
|
// client is the Gitlab client being tested.
|
||||||
|
client, err := gitlab.NewClient("", gitlab.WithBaseURL(server.URL))
|
||||||
|
if err != nil {
|
||||||
|
server.Close()
|
||||||
|
t.Fatalf("Failed to create client: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return mux, server, client
|
||||||
|
}
|
||||||
|
|
||||||
|
func gitlabClientMockTeardown(server *httptest.Server) {
|
||||||
|
server.Close()
|
||||||
|
}
|
||||||
|
|
||||||
|
type reviewTestCase struct {
|
||||||
|
repoID, prID, reviewerID int
|
||||||
|
reviewerName string
|
||||||
|
createdAt, updatedAt *time.Time
|
||||||
|
expectedCreatedAt time.Time
|
||||||
|
}
|
||||||
|
|
||||||
|
func convertTestCase(t reviewTestCase) (func(w http.ResponseWriter, r *http.Request), base.Review) {
|
||||||
|
var updatedAtField string
|
||||||
|
if t.updatedAt == nil {
|
||||||
|
updatedAtField = ""
|
||||||
|
} else {
|
||||||
|
updatedAtField = `"updated_at": "` + t.updatedAt.Format(time.RFC3339) + `",`
|
||||||
|
}
|
||||||
|
|
||||||
|
var createdAtField string
|
||||||
|
if t.createdAt == nil {
|
||||||
|
createdAtField = ""
|
||||||
|
} else {
|
||||||
|
createdAtField = `"created_at": "` + t.createdAt.Format(time.RFC3339) + `",`
|
||||||
|
}
|
||||||
|
|
||||||
|
handler := func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
fmt.Fprint(w, `
|
||||||
|
{
|
||||||
|
"id": 5,
|
||||||
|
"iid": `+strconv.Itoa(t.prID)+`,
|
||||||
|
"project_id": `+strconv.Itoa(t.repoID)+`,
|
||||||
|
"title": "Approvals API",
|
||||||
|
"description": "Test",
|
||||||
|
"state": "opened",
|
||||||
|
`+createdAtField+`
|
||||||
|
`+updatedAtField+`
|
||||||
|
"merge_status": "cannot_be_merged",
|
||||||
|
"approvals_required": 2,
|
||||||
|
"approvals_left": 1,
|
||||||
|
"approved_by": [
|
||||||
|
{
|
||||||
|
"user": {
|
||||||
|
"name": "Administrator",
|
||||||
|
"username": "`+t.reviewerName+`",
|
||||||
|
"id": `+strconv.Itoa(t.reviewerID)+`,
|
||||||
|
"state": "active",
|
||||||
|
"avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon",
|
||||||
|
"web_url": "http://localhost:3000/root"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}`)
|
||||||
|
}
|
||||||
|
review := base.Review{
|
||||||
|
IssueIndex: int64(t.prID),
|
||||||
|
ReviewerID: int64(t.reviewerID),
|
||||||
|
ReviewerName: t.reviewerName,
|
||||||
|
CreatedAt: t.expectedCreatedAt,
|
||||||
|
State: "APPROVED",
|
||||||
|
}
|
||||||
|
|
||||||
|
return handler, review
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestGitlabGetReviews(t *testing.T) {
|
||||||
|
mux, server, client := gitlabClientMockSetup(t)
|
||||||
|
defer gitlabClientMockTeardown(server)
|
||||||
|
|
||||||
|
repoID := 1324
|
||||||
|
|
||||||
|
downloader := &GitlabDownloader{
|
||||||
|
ctx: context.Background(),
|
||||||
|
client: client,
|
||||||
|
repoID: repoID,
|
||||||
|
}
|
||||||
|
|
||||||
|
createdAt := time.Date(2020, 4, 19, 19, 24, 21, 0, time.UTC)
|
||||||
|
|
||||||
|
for _, testCase := range []reviewTestCase{
|
||||||
|
{
|
||||||
|
repoID: repoID,
|
||||||
|
prID: 1,
|
||||||
|
reviewerID: 801,
|
||||||
|
reviewerName: "someone1",
|
||||||
|
createdAt: nil,
|
||||||
|
updatedAt: &createdAt,
|
||||||
|
expectedCreatedAt: createdAt,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
repoID: repoID,
|
||||||
|
prID: 2,
|
||||||
|
reviewerID: 802,
|
||||||
|
reviewerName: "someone2",
|
||||||
|
createdAt: &createdAt,
|
||||||
|
updatedAt: nil,
|
||||||
|
expectedCreatedAt: createdAt,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
repoID: repoID,
|
||||||
|
prID: 3,
|
||||||
|
reviewerID: 803,
|
||||||
|
reviewerName: "someone3",
|
||||||
|
createdAt: nil,
|
||||||
|
updatedAt: nil,
|
||||||
|
expectedCreatedAt: time.Now(),
|
||||||
|
},
|
||||||
|
} {
|
||||||
|
mock, review := convertTestCase(testCase)
|
||||||
|
mux.HandleFunc(fmt.Sprintf("/api/v4/projects/%d/merge_requests/%d/approvals", testCase.repoID, testCase.prID), mock)
|
||||||
|
|
||||||
|
rvs, err := downloader.GetReviews(base.BasicIssueContext(testCase.prID))
|
||||||
|
assert.NoError(t, err)
|
||||||
|
assertReviewsEqual(t, []*base.Review{&review}, rvs)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
@ -223,15 +223,15 @@ func assertRepositoryEqual(t *testing.T, expected, actual *base.Repository) {
|
||||||
}
|
}
|
||||||
|
|
||||||
func assertReviewEqual(t *testing.T, expected, actual *base.Review) {
|
func assertReviewEqual(t *testing.T, expected, actual *base.Review) {
|
||||||
assert.Equal(t, expected.ID, actual.ID)
|
assert.Equal(t, expected.ID, actual.ID, "ID")
|
||||||
assert.Equal(t, expected.IssueIndex, actual.IssueIndex)
|
assert.Equal(t, expected.IssueIndex, actual.IssueIndex, "IsssueIndex")
|
||||||
assert.Equal(t, expected.ReviewerID, actual.ReviewerID)
|
assert.Equal(t, expected.ReviewerID, actual.ReviewerID, "ReviewerID")
|
||||||
assert.Equal(t, expected.ReviewerName, actual.ReviewerName)
|
assert.Equal(t, expected.ReviewerName, actual.ReviewerName, "ReviewerName")
|
||||||
assert.Equal(t, expected.Official, actual.Official)
|
assert.Equal(t, expected.Official, actual.Official, "Official")
|
||||||
assert.Equal(t, expected.CommitID, actual.CommitID)
|
assert.Equal(t, expected.CommitID, actual.CommitID, "CommitID")
|
||||||
assert.Equal(t, expected.Content, actual.Content)
|
assert.Equal(t, expected.Content, actual.Content, "Content")
|
||||||
assertTimeEqual(t, expected.CreatedAt, actual.CreatedAt)
|
assert.WithinDuration(t, expected.CreatedAt, actual.CreatedAt, 10*time.Second)
|
||||||
assert.Equal(t, expected.State, actual.State)
|
assert.Equal(t, expected.State, actual.State, "State")
|
||||||
assertReviewCommentsEqual(t, expected.Comments, actual.Comments)
|
assertReviewCommentsEqual(t, expected.Comments, actual.Comments)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue