diff --git a/Makefile b/Makefile index 852c85ccd6..e6416d5a6f 100644 --- a/Makefile +++ b/Makefile @@ -47,6 +47,7 @@ GO_LICENSES_PACKAGE ?= github.com/google/go-licenses@v1.6.0 # renovate: datasour GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1 # renovate: datasource=go DEADCODE_PACKAGE ?= golang.org/x/tools/cmd/deadcode@v0.34.0 # renovate: datasource=go GOMOCK_PACKAGE ?= go.uber.org/mock/mockgen@v0.5.2 # renovate: datasource=go +GOPLS_PACKAGE ?= golang.org/x/tools/gopls@v0.18.1 # renovate: datasource=go RENOVATE_NPM_PACKAGE ?= renovate@40.57.1 # renovate: datasource=docker packageName=data.forgejo.org/renovate/renovate # https://github.com/disposable-email-domains/disposable-email-domains/commits/main/ @@ -221,6 +222,7 @@ help: @echo " - lint-go lint go files" @echo " - lint-go-fix lint go files and fix issues" @echo " - lint-go-vet lint go files with vet" + @echo " - lint-go-gopls lint go files with gopls" @echo " - lint-js lint js files" @echo " - lint-js-fix lint js files and fix issues" @echo " - lint-css lint css files" @@ -485,6 +487,11 @@ lint-go-vet: @echo "Running go vet..." @$(GO) vet ./... +.PHONY: lint-go-gopls +lint-go-gopls: + @echo "Running gopls check..." + @GO=$(GO) GOPLS_PACKAGE=$(GOPLS_PACKAGE) tools/lint-go-gopls.sh $(GO_SOURCES_NO_BINDATA) + .PHONY: lint-editorconfig lint-editorconfig: $(GO) run $(EDITORCONFIG_CHECKER_PACKAGE) templates .forgejo/workflows @@ -925,6 +932,7 @@ deps-tools: $(GO) install $(GO_LICENSES_PACKAGE) $(GO) install $(GOVULNCHECK_PACKAGE) $(GO) install $(GOMOCK_PACKAGE) + $(GO) install $(GOPLS_PACKAGE) node_modules: package-lock.json npm install --no-save diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml index 2c47196c05..f4121284a6 100644 --- a/models/fixtures/comment.yml +++ b/models/fixtures/comment.yml @@ -113,43 +113,3 @@ review_id: 22 assignee_id: 5 created_unix: 946684817 - -- - id: 13 - type: 29 # push - poster_id: 2 - issue_id: 19 # in repo_id 58 - content: '{"is_force_push":false,"commit_ids":["4ca8bcaf27e28504df7bf996819665986b01c847","96cef4a7b72b3c208340ae6f0cf55a93e9077c93","c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2"]}' - created_unix: 1688672373 - -- - id: 14 - type: 29 # push - poster_id: 2 - issue_id: 19 # in repo_id 58 - content: '{"is_force_push":false,"commit_ids":["23576dd018294e476c06e569b6b0f170d0558705"]}' - created_unix: 1688672374 - -- - id: 15 - type: 29 # push - poster_id: 2 - issue_id: 19 # in repo_id 58 - content: '{"is_force_push":false,"commit_ids":["3e64625bd6eb5bcba69ac97de6c8f507402df861", "c704db5794097441aa2d9dd834d5b7e2f8f08108"]}' - created_unix: 1688672375 - -- - id: 16 - type: 29 # push - poster_id: 2 - issue_id: 19 # in repo_id 58 - content: '{"is_force_push":false,"commit_ids":["811d46c7e518f4f180afb862c0db5cb8c80529ce", "747ddb3506a4fa04a7747808eb56ae16f9e933dc", "837d5c8125633d7d258f93b998e867eab0145520", "1978192d98bb1b65e11c2cf37da854fbf94bffd6"]}' - created_unix: 1688672376 - -- - id: 17 - type: 29 # push - poster_id: 2 - issue_id: 19 # in repo_id 58 - content: '{"is_force_push":true,"commit_ids":["1978192d98bb1b65e11c2cf37da854fbf94bffd6", "9b93963cf6de4dc33f915bb67f192d099c301f43"]}' - created_unix: 1749734240 diff --git a/models/fixtures/pull_auto_merge.yml b/models/fixtures/pull_auto_merge.yml deleted file mode 100644 index ca780a73aa..0000000000 --- a/models/fixtures/pull_auto_merge.yml +++ /dev/null @@ -1 +0,0 @@ -[] # empty diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index 91a69c26a7..bf4b89ee0b 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -66,8 +66,6 @@ func applySorts(sess *xorm.Session, sortType string, priorityRepoID int64) { sess.Asc("issue.created_unix").Asc("issue.id") case "recentupdate": sess.Desc("issue.updated_unix").Desc("issue.created_unix").Desc("issue.id") - case "recentclose": - sess.Desc("issue.closed_unix").Desc("issue.created_unix").Desc("issue.id") case "leastupdate": sess.Asc("issue.updated_unix").Asc("issue.created_unix").Asc("issue.id") case "mostcomment": diff --git a/models/issues/pull_list.go b/models/issues/pull_list.go index 8fc0491026..a448673454 100644 --- a/models/issues/pull_list.go +++ b/models/issues/pull_list.go @@ -152,8 +152,7 @@ func PullRequests(ctx context.Context, baseRepoID int64, opts *PullRequestsOptio applySorts(findSession, opts.SortType, 0) findSession = db.SetSessionPagination(findSession, opts) prs := make([]*PullRequest, 0, opts.PageSize) - found := findSession.Find(&prs) - return prs, maxResults, found + return prs, maxResults, findSession.Find(&prs) } // PullRequestList defines a list of pull requests diff --git a/models/issues/pull_test.go b/models/issues/pull_test.go index 3682f6fd25..5a8e8d8aaf 100644 --- a/models/issues/pull_test.go +++ b/models/issues/pull_test.go @@ -79,47 +79,6 @@ func TestPullRequestsNewest(t *testing.T) { } } -func TestPullRequests_Closed_RecentSortType(t *testing.T) { - // Issue ID | Closed At. | Updated At - // 2 | 1707270001 | 1707270001 - // 3 | 1707271000 | 1707279999 - // 11 | 1707279999 | 1707275555 - tests := []struct { - sortType string - expectedIssueIDOrder []int64 - }{ - {"recentupdate", []int64{3, 11, 2}}, - {"recentclose", []int64{11, 3, 2}}, - } - - require.NoError(t, unittest.PrepareTestDatabase()) - _, err := db.Exec(db.DefaultContext, "UPDATE issue SET closed_unix = 1707270001, updated_unix = 1707270001, is_closed = true WHERE id = 2") - require.NoError(t, err) - _, err = db.Exec(db.DefaultContext, "UPDATE issue SET closed_unix = 1707271000, updated_unix = 1707279999, is_closed = true WHERE id = 3") - require.NoError(t, err) - _, err = db.Exec(db.DefaultContext, "UPDATE issue SET closed_unix = 1707279999, updated_unix = 1707275555, is_closed = true WHERE id = 11") - require.NoError(t, err) - - for _, test := range tests { - t.Run(test.sortType, func(t *testing.T) { - prs, _, err := issues_model.PullRequests(db.DefaultContext, 1, &issues_model.PullRequestsOptions{ - ListOptions: db.ListOptions{ - Page: 1, - }, - State: "closed", - SortType: test.sortType, - }) - require.NoError(t, err) - - if assert.Len(t, prs, len(test.expectedIssueIDOrder)) { - for i := range test.expectedIssueIDOrder { - assert.Equal(t, test.expectedIssueIDOrder[i], prs[i].IssueID) - } - } - }) - } -} - func TestLoadRequestedReviewers(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) diff --git a/models/pull/automerge.go b/models/pull/automerge.go index dcc1f39271..63f572309b 100644 --- a/models/pull/automerge.go +++ b/models/pull/automerge.go @@ -10,7 +10,6 @@ import ( "forgejo.org/models/db" repo_model "forgejo.org/models/repo" user_model "forgejo.org/models/user" - "forgejo.org/modules/log" "forgejo.org/modules/timeutil" ) @@ -59,15 +58,13 @@ func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, return ErrAlreadyScheduledToAutoMerge{PullID: pullID} } - scheduledPRM, err := db.GetEngine(ctx).Insert(&AutoMerge{ + _, err := db.GetEngine(ctx).Insert(&AutoMerge{ DoerID: doer.ID, PullID: pullID, MergeStyle: style, Message: message, DeleteBranchAfterMerge: deleteBranch, }) - log.Trace("ScheduleAutoMerge %+v for PR %d", scheduledPRM, pullID) - return err } @@ -84,8 +81,6 @@ func GetScheduledMergeByPullID(ctx context.Context, pullID int64) (bool, *AutoMe return false, nil, err } - log.Trace("GetScheduledMergeByPullID found %+v for PR %d", scheduledPRM, pullID) - scheduledPRM.Doer = doer return true, scheduledPRM, nil } @@ -99,8 +94,6 @@ func DeleteScheduledAutoMerge(ctx context.Context, pullID int64) error { return db.ErrNotExist{Resource: "auto_merge", ID: pullID} } - log.Trace("DeleteScheduledAutoMerge %+v for PR %d", scheduledPRM, pullID) - _, err = db.GetEngine(ctx).ID(scheduledPRM.ID).Delete(&AutoMerge{}) return err } diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index 368152095e..356aaaead0 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -89,8 +89,6 @@ "mail.actions.run_info_previous_status": "Previous Run's Status: %[1]s", "mail.actions.run_info_ref": "Branch: %[1]s (%[2]s)", "mail.actions.run_info_trigger": "Triggered because: %[1]s by: %[2]s", - "repo.diff.commit.next-short": "Next", - "repo.diff.commit.previous-short": "Prev", "discussion.locked": "This discussion has been locked. Commenting is limited to contributors.", "editor.textarea.tab_hint": "Line already indented. Press Tab again or Escape to leave the editor.", "editor.textarea.shift_tab_hint": "No indentation on this line. Press Shift + Tab again or Escape to leave the editor.", diff --git a/package-lock.json b/package-lock.json index 40601d8536..4a91eeecc2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,7 +16,7 @@ "@primer/octicons": "19.14.0", "ansi_up": "6.0.5", "asciinema-player": "3.8.2", - "chart.js": "4.5.0", + "chart.js": "4.4.9", "chartjs-adapter-dayjs-4": "1.0.4", "chartjs-plugin-zoom": "2.2.0", "clippie": "4.1.7", @@ -5373,9 +5373,9 @@ } }, "node_modules/chart.js": { - "version": "4.5.0", - "resolved": "https://registry.npmjs.org/chart.js/-/chart.js-4.5.0.tgz", - "integrity": "sha512-aYeC/jDgSEx8SHWZvANYMioYMZ2KX02W6f6uVfyteuCGcadDLcYVHdfdygsTQkQ4TKn5lghoojAsPj5pu0SnvQ==", + "version": "4.4.9", + "resolved": "https://registry.npmjs.org/chart.js/-/chart.js-4.4.9.tgz", + "integrity": "sha512-EyZ9wWKgpAU0fLJ43YAEIF8sr5F2W3LqbS40ZJyHIner2lY14ufqv2VMp69MAiZ2rpwxEUxEhIH/0U3xyRynxg==", "license": "MIT", "dependencies": { "@kurkle/color": "^0.3.0" diff --git a/package.json b/package.json index b4d170fdb9..f7f04b1c0b 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "@primer/octicons": "19.14.0", "ansi_up": "6.0.5", "asciinema-player": "3.8.2", - "chart.js": "4.5.0", + "chart.js": "4.4.9", "chartjs-adapter-dayjs-4": "1.0.4", "chartjs-plugin-zoom": "2.2.0", "clippie": "4.1.7", diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index c9dda124de..75b4870e51 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -71,7 +71,7 @@ func ListPullRequests(ctx *context.APIContext) { // in: query // description: Type of sort // type: string - // enum: [oldest, recentupdate, recentclose, leastupdate, mostcomment, leastcomment, priority] + // enum: [oldest, recentupdate, leastupdate, mostcomment, leastcomment, priority] // - name: milestone // in: query // description: ID of the milestone diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index fd18646211..2db982d3b6 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -10,16 +10,13 @@ import ( "errors" "fmt" "html" - "html/template" "net/http" "net/url" - "path" "strconv" "strings" "forgejo.org/models" activities_model "forgejo.org/models/activities" - asymkey_model "forgejo.org/models/asymkey" "forgejo.org/models/db" git_model "forgejo.org/models/git" issues_model "forgejo.org/models/issues" @@ -31,13 +28,11 @@ import ( "forgejo.org/models/unit" user_model "forgejo.org/models/user" "forgejo.org/modules/base" - "forgejo.org/modules/charset" "forgejo.org/modules/emoji" "forgejo.org/modules/git" "forgejo.org/modules/gitrepo" issue_template "forgejo.org/modules/issue/template" "forgejo.org/modules/log" - "forgejo.org/modules/markup" "forgejo.org/modules/optional" "forgejo.org/modules/setting" "forgejo.org/modules/structs" @@ -503,7 +498,6 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) ctx.Data["IsPullRequestBroken"] = true ctx.Data["BaseTarget"] = pull.BaseBranch ctx.Data["NumCommits"] = 0 - ctx.Data["CommitIDs"] = map[string]bool{} ctx.Data["NumFiles"] = 0 return nil } @@ -514,12 +508,6 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) ctx.Data["NumCommits"] = len(compareInfo.Commits) ctx.Data["NumFiles"] = compareInfo.NumFiles - commitIDs := map[string]bool{} - for _, commit := range compareInfo.Commits { - commitIDs[commit.ID.String()] = true - } - ctx.Data["CommitIDs"] = commitIDs - if len(compareInfo.Commits) != 0 { sha := compareInfo.Commits[0].ID.String() commitStatuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, sha, db.ListOptionsAll) @@ -603,7 +591,6 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C ctx.Data["IsPullRequestBroken"] = true ctx.Data["BaseTarget"] = pull.BaseBranch ctx.Data["NumCommits"] = 0 - ctx.Data["CommitIDs"] = map[string]bool{} ctx.Data["NumFiles"] = 0 return nil } @@ -614,13 +601,6 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C ctx.Data["NumCommits"] = len(compareInfo.Commits) ctx.Data["NumFiles"] = compareInfo.NumFiles - - commitIDs := map[string]bool{} - for _, commit := range compareInfo.Commits { - commitIDs[commit.ID.String()] = true - } - ctx.Data["CommitIDs"] = commitIDs - return compareInfo } @@ -679,7 +659,6 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C } ctx.Data["BaseTarget"] = pull.BaseBranch ctx.Data["NumCommits"] = 0 - ctx.Data["CommitIDs"] = map[string]bool{} ctx.Data["NumFiles"] = 0 return nil } @@ -757,7 +736,6 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C ctx.Data["IsPullRequestBroken"] = true ctx.Data["BaseTarget"] = pull.BaseBranch ctx.Data["NumCommits"] = 0 - ctx.Data["CommitIDs"] = map[string]bool{} ctx.Data["NumFiles"] = 0 return nil } @@ -782,13 +760,6 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C ctx.Data["NumCommits"] = len(compareInfo.Commits) ctx.Data["NumFiles"] = compareInfo.NumFiles - - commitIDs := map[string]bool{} - for _, commit := range compareInfo.Commits { - commitIDs[commit.ID.String()] = true - } - ctx.Data["CommitIDs"] = commitIDs - return compareInfo } @@ -948,78 +919,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi ctx.Data["IsShowingOnlySingleCommit"] = willShowSpecifiedCommit - if willShowSpecifiedCommit { - commitID := specifiedEndCommit - - ctx.Data["CommitID"] = commitID - - var prevCommit, curCommit, nextCommit *git.Commit - - // Iterate in reverse to properly map "previous" and "next" buttons - for i := len(prInfo.Commits) - 1; i >= 0; i-- { - commit := prInfo.Commits[i] - - if curCommit != nil { - nextCommit = commit - break - } - - if commit.ID.String() == commitID { - curCommit = commit - } else { - prevCommit = commit - } - } - - if curCommit == nil { - ctx.ServerError("Repo.GitRepo.viewPullFiles", git.ErrNotExist{ID: commitID}) - return - } - - ctx.Data["Commit"] = curCommit - if prevCommit != nil { - ctx.Data["PrevCommitLink"] = path.Join(ctx.Repo.RepoLink, "pulls", strconv.FormatInt(issue.Index, 10), "commits", prevCommit.ID.String()) - } - if nextCommit != nil { - ctx.Data["NextCommitLink"] = path.Join(ctx.Repo.RepoLink, "pulls", strconv.FormatInt(issue.Index, 10), "commits", nextCommit.ID.String()) - } - - statuses, _, err := git_model.GetLatestCommitStatus(ctx, ctx.Repo.Repository.ID, commitID, db.ListOptionsAll) - if err != nil { - log.Error("GetLatestCommitStatus: %v", err) - } - - ctx.Data["CommitStatus"] = git_model.CalcCommitStatus(statuses) - ctx.Data["CommitStatuses"] = statuses - - verification := asymkey_model.ParseCommitWithSignature(ctx, curCommit) - ctx.Data["Verification"] = verification - ctx.Data["Author"] = user_model.ValidateCommitWithEmail(ctx, curCommit) - - note := &git.Note{} - err = git.GetNote(ctx, ctx.Repo.GitRepo, specifiedEndCommit, note) - if err == nil { - ctx.Data["NoteCommit"] = note.Commit - ctx.Data["NoteAuthor"] = user_model.ValidateCommitWithEmail(ctx, note.Commit) - ctx.Data["NoteRendered"], err = markup.RenderCommitMessage(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - BranchPath: path.Join("commit", util.PathEscapeSegments(commitID)), - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Ctx: ctx, - }, template.HTMLEscapeString(string(charset.ToUTF8WithFallback(note.Message, charset.ConvertOpts{})))) - if err != nil { - ctx.ServerError("RenderCommitMessage", err) - return - } - } - - endCommitID = commitID - startCommitID = prInfo.MergeBase - ctx.Data["IsShowingAllCommits"] = false - } else if willShowSpecifiedCommitRange { + if willShowSpecifiedCommit || willShowSpecifiedCommitRange { if len(specifiedEndCommit) > 0 { endCommitID = specifiedEndCommit } else { @@ -1030,7 +930,6 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi } else { startCommitID = prInfo.MergeBase } - ctx.Data["IsShowingAllCommits"] = false } else { endCommitID = headCommitID @@ -1038,10 +937,10 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi ctx.Data["IsShowingAllCommits"] = true } - ctx.Data["AfterCommitID"] = endCommitID - ctx.Data["BeforeCommitID"] = startCommitID ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name + ctx.Data["AfterCommitID"] = endCommitID + ctx.Data["BeforeCommitID"] = startCommitID fileOnly := ctx.FormBool("file-only") diff --git a/routers/web/web.go b/routers/web/web.go index 534c37e918..2568c6a853 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1511,10 +1511,7 @@ func registerRoutes(m *web.Route) { m.Group("/commits", func() { m.Get("", context.RepoRef(), repo.SetWhitespaceBehavior, repo.GetPullDiffStats, repo.ViewPullCommits) m.Get("/list", context.RepoRef(), repo.GetPullCommits) - m.Group("/{sha:[a-f0-9]{4,40}}", func() { - m.Get("", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForSingleCommit) - m.Post("/reviews/submit", context.RepoMustNotBeArchived(), web.Bind(forms.SubmitReviewForm{}), repo.SubmitReview) - }) + m.Get("/{sha:[a-f0-9]{4,40}}", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.SetShowOutdatedComments, repo.ViewPullFilesForSingleCommit) }) m.Post("/merge", context.RepoMustNotBeArchived(), web.Bind(forms.MergePullRequestForm{}), context.EnforceQuotaWeb(quota_model.LimitSubjectSizeGitAll, context.QuotaTargetRepo), repo.MergePullRequest) m.Post("/cancel_auto_merge", context.RepoMustNotBeArchived(), repo.CancelAutoMergePullRequest) diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index cbfe3bd54e..f183136907 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -107,7 +107,6 @@ func handlePullRequestAutoMerge(pullID int64, sha string) { return } if !exists { - log.Trace("GetScheduledMergeByPullID found nothing for PR %d", pullID) return } @@ -205,10 +204,6 @@ func handlePullRequestAutoMerge(pullID int64, sha string) { return } - if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) { - log.Error("DeleteScheduledAutoMerge[%d]: %v", pr.ID, err) - } - if err := pull_service.Merge(ctx, pr, doer, baseGitRepo, scheduledPRM.MergeStyle, "", scheduledPRM.Message, true); err != nil { log.Error("pull_service.Merge: %v", err) // FIXME: if merge failed, we should display some error message to the pull request page. diff --git a/services/pull/check.go b/services/pull/check.go index 6002e2ae26..afeb7e675e 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -28,7 +28,6 @@ import ( "forgejo.org/modules/timeutil" asymkey_service "forgejo.org/services/asymkey" notify_service "forgejo.org/services/notify" - shared_automerge "forgejo.org/services/shared/automerge" ) // prPatchCheckerQueue represents a queue to handle update pull request tests @@ -171,7 +170,7 @@ func isSignedIfRequired(ctx context.Context, pr *issues_model.PullRequest, doer // checkAndUpdateStatus checks if pull request is possible to leaving checking status, // and set to be either conflict or mergeable. -func checkAndUpdateStatus(ctx context.Context, pr *issues_model.PullRequest) bool { +func checkAndUpdateStatus(ctx context.Context, pr *issues_model.PullRequest) { // If status has not been changed to conflict by testPatch then we are mergeable if pr.Status == issues_model.PullRequestStatusChecking { pr.Status = issues_model.PullRequestStatusMergeable @@ -185,15 +184,12 @@ func checkAndUpdateStatus(ctx context.Context, pr *issues_model.PullRequest) boo if has { log.Trace("Not updating status for %-v as it is due to be rechecked", pr) - return false + return } if err := pr.UpdateColsIfNotMerged(ctx, "merge_base", "status", "conflicted_files", "changed_protected_files"); err != nil { log.Error("Update[%-v]: %v", pr, err) - return false } - - return true } // getMergeCommit checks if a pull request has been merged @@ -343,22 +339,15 @@ func handler(items ...string) []string { } func testPR(id int64) { - ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("Test PR[%d] from patch checking queue", id)) - defer finished() - - if pr, updated := testPRProtected(ctx, id); pr != nil && updated { - shared_automerge.AddToQueueIfMergeable(ctx, pr) - } -} - -func testPRProtected(ctx context.Context, id int64) (*issues_model.PullRequest, bool) { pullWorkingPool.CheckIn(fmt.Sprint(id)) defer pullWorkingPool.CheckOut(fmt.Sprint(id)) + ctx, _, finished := process.GetManager().AddContext(graceful.GetManager().HammerContext(), fmt.Sprintf("Test PR[%d] from patch checking queue", id)) + defer finished() pr, err := issues_model.GetPullRequestByID(ctx, id) if err != nil { log.Error("Unable to GetPullRequestByID[%d] for testPR: %v", id, err) - return nil, false + return } log.Trace("Testing %-v", pr) @@ -368,12 +357,12 @@ func testPRProtected(ctx context.Context, id int64) (*issues_model.PullRequest, if pr.HasMerged { log.Trace("%-v is already merged (status: %s, merge commit: %s)", pr, pr.Status, pr.MergedCommitID) - return nil, false + return } if manuallyMerged(ctx, pr) { log.Trace("%-v is manually merged (status: %s, merge commit: %s)", pr, pr.Status, pr.MergedCommitID) - return nil, false + return } if err := TestPatch(pr); err != nil { @@ -382,10 +371,9 @@ func testPRProtected(ctx context.Context, id int64) (*issues_model.PullRequest, if err := pr.UpdateCols(ctx, "status"); err != nil { log.Error("update pr [%-v] status to PullRequestStatusError failed: %v", pr, err) } - return nil, false + return } - - return pr, checkAndUpdateStatus(ctx, pr) + checkAndUpdateStatus(ctx, pr) } // CheckPRsForBaseBranch check all pulls with baseBrannch diff --git a/services/shared/automerge/automerge.go b/services/shared/automerge/automerge.go index be7b2f6eb4..1dc309f4b3 100644 --- a/services/shared/automerge/automerge.go +++ b/services/shared/automerge/automerge.go @@ -21,9 +21,9 @@ import ( var PRAutoMergeQueue *queue.WorkerPoolQueue[string] func addToQueue(pr *issues_model.PullRequest, sha string) { - log.Trace("Adding pullID: %d to the automerge queue with sha %s", pr.ID, sha) + log.Trace("Adding pullID: %d to the pull requests patch checking queue with sha %s", pr.ID, sha) if err := PRAutoMergeQueue.Push(fmt.Sprintf("%d_%s", pr.ID, sha)); err != nil { - log.Error("Error adding pullID: %d to the automerge queue %v", pr.ID, err) + log.Error("Error adding pullID: %d to the pull requests patch checking queue %v", pr.ID, err) } } @@ -43,29 +43,32 @@ func StartPRCheckAndAutoMergeBySHA(ctx context.Context, sha string, repo *repo_m return nil } +// StartPRCheckAndAutoMerge start an automerge check and auto merge task for a pull request func StartPRCheckAndAutoMerge(ctx context.Context, pull *issues_model.PullRequest) { if pull == nil || pull.HasMerged || !pull.CanAutoMerge() { return } - commitID := pull.HeadCommitID - if commitID == "" { - commitID = getCommitIDFromRefName(ctx, pull) + if err := pull.LoadBaseRepo(ctx); err != nil { + log.Error("LoadBaseRepo: %v", err) + return } - if commitID == "" { + gitRepo, err := gitrepo.OpenRepository(ctx, pull.BaseRepo) + if err != nil { + log.Error("OpenRepository: %v", err) + return + } + defer gitRepo.Close() + commitID, err := gitRepo.GetRefCommitID(pull.GetGitRefName()) + if err != nil { + log.Error("GetRefCommitID: %v", err) return } addToQueue(pull, commitID) } -var AddToQueueIfMergeable = func(ctx context.Context, pull *issues_model.PullRequest) { - if pull.Status == issues_model.PullRequestStatusMergeable { - StartPRCheckAndAutoMerge(ctx, pull) - } -} - func getPullRequestsByHeadSHA(ctx context.Context, sha string, repo *repo_model.Repository, filter func(*issues_model.PullRequest) bool) (map[int64]*issues_model.PullRequest, error) { gitRepo, err := gitrepo.OpenRepository(ctx, repo) if err != nil { @@ -115,24 +118,3 @@ func getPullRequestsByHeadSHA(ctx context.Context, sha string, repo *repo_model. return pulls, nil } - -func getCommitIDFromRefName(ctx context.Context, pull *issues_model.PullRequest) string { - if err := pull.LoadBaseRepo(ctx); err != nil { - log.Error("LoadBaseRepo: %v", err) - return "" - } - - gitRepo, err := gitrepo.OpenRepository(ctx, pull.BaseRepo) - if err != nil { - log.Error("OpenRepository: %v", err) - return "" - } - defer gitRepo.Close() - commitID, err := gitRepo.GetRefCommitID(pull.GetGitRefName()) - if err != nil { - log.Error("GetRefCommitID: %v", err) - return "" - } - - return commitID -} diff --git a/templates/repo/commit_header.tmpl b/templates/repo/commit_header.tmpl index 9604daf2b0..8a074e9545 100644 --- a/templates/repo/commit_header.tmpl +++ b/templates/repo/commit_header.tmpl @@ -14,19 +14,10 @@ {{end}} {{end}}
-
+

{{RenderCommitMessage $.Context .Commit.Message ($.Repository.ComposeMetas ctx)}}{{template "repo/commit_statuses" dict "Status" .CommitStatus "Statuses" .CommitStatuses}}

-
- {{if .PageIsPullFiles}} -
- - {{svg "octicon-chevron-left"}} {{ctx.Locale.Tr "repo.diff.commit.previous-short"}} - - - {{ctx.Locale.Tr "repo.diff.commit.next-short"}} {{svg "octicon-chevron-right"}} - -
- {{else if not $.PageIsWiki}} + {{if not $.PageIsWiki}} +
{{ctx.Locale.Tr "repo.diff.browse_source"}} @@ -141,8 +132,8 @@
{{end}} - {{end}} -
+
+ {{end}}
{{if IsMultilineCommitMessage .Commit.Message}} 
{{RenderCommitBody $.Context .Commit.Message ($.Repository.ComposeMetas ctx)}}
@@ -194,16 +185,9 @@ {{end}}
{{ctx.Locale.Tr "repo.diff.commit"}} - {{if .PageIsPullFiles}} - {{$commitShaLink := (printf "%s/commit/%s" $.RepoLink (PathEscape .CommitID))}} - - {{ShortSha .CommitID}} - - {{else}} - - {{ShortSha .CommitID}} - - {{end}} + + {{ShortSha .CommitID}} +
diff --git a/templates/repo/commit_load_branches_and_tags.tmpl b/templates/repo/commit_load_branches_and_tags.tmpl index 25402ca2f4..ffa0e530e8 100644 --- a/templates/repo/commit_load_branches_and_tags.tmpl +++ b/templates/repo/commit_load_branches_and_tags.tmpl @@ -1,4 +1,4 @@ -{{if not (or .PageIsWiki .PageIsPullFiles)}} +{{if not .PageIsWiki}}
{{if and (not $.Repository.IsArchived) (not .DiffNotAvailable)}} diff --git a/templates/repo/diff/new_review.tmpl b/templates/repo/diff/new_review.tmpl index ded7a6c5fc..13d09babe1 100644 --- a/templates/repo/diff/new_review.tmpl +++ b/templates/repo/diff/new_review.tmpl @@ -1,14 +1,17 @@
-
+ {{if $.IsShowingAllCommits}}
@@ -52,4 +55,5 @@
+ {{end}}
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index bba4ac4949..124d5bd1bd 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -12994,7 +12994,6 @@ "enum": [ "oldest", "recentupdate", - "recentclose", "leastupdate", "mostcomment", "leastcomment", diff --git a/tests/e2e/declare_repos_test.go b/tests/e2e/declare_repos_test.go index 4c68566a48..0ccb776ff7 100644 --- a/tests/e2e/declare_repos_test.go +++ b/tests/e2e/declare_repos_test.go @@ -82,7 +82,7 @@ func newRepo(t *testing.T, userID int64, repoName string, fileChanges []FileChan for _, file := range fileChanges { for i, version := range file.Versions { operation := "update" - if i == 0 { + if i == 0 && file.Filename != "README.md" { operation = "create" } diff --git a/tests/e2e/pr-review.test.e2e.ts b/tests/e2e/pr-review.test.e2e.ts index 577c8bf20a..b619cdbcd1 100644 --- a/tests/e2e/pr-review.test.e2e.ts +++ b/tests/e2e/pr-review.test.e2e.ts @@ -11,7 +11,7 @@ import {save_visual, test} from './utils_e2e.ts'; test.use({user: 'user2'}); -test('PR: Create review from files', async ({page}) => { +test('PR: Finish review', async ({page}) => { const response = await page.goto('/user2/repo1/pulls/5/files'); expect(response?.status()).toBe(200); @@ -22,93 +22,4 @@ test('PR: Create review from files', async ({page}) => { await page.locator('#review-box .js-btn-review').click(); await expect(page.locator('.tippy-box .review-box-panel')).toBeVisible(); await save_visual(page); - - await page.locator('.review-box-panel textarea#_combo_markdown_editor_0') - .fill('This is a review'); - await page.locator('.review-box-panel button.btn-submit[value="approve"]').click(); - await page.waitForURL(/.*\/user2\/repo1\/pulls\/5#issuecomment-\d+/); - await save_visual(page); -}); - -test('PR: Create review from commit', async ({page}) => { - const response = await page.goto('/user2/repo1/pulls/3/commits/4a357436d925b5c974181ff12a994538ddc5a269'); - expect(response?.status()).toBe(200); - - await page.locator('button.add-code-comment').click(); - const code_comment = page.locator('.comment-code-cloud form textarea.markdown-text-editor'); - await expect(code_comment).toBeVisible(); - - await code_comment.fill('This is a code comment'); - await save_visual(page); - - const start_button = page.locator('.comment-code-cloud form button.btn-start-review'); - // Workaround for #7152, where there might already be a pending review state from previous - // test runs (most likely to happen when debugging tests). - if (await start_button.isVisible({timeout: 100})) { - await start_button.click(); - } else { - await page.locator('.comment-code-cloud form button.btn-add-comment').click(); - } - - await expect(page.locator('.comment-list .comment-container')).toBeVisible(); - - // We need to wait for the review to be processed. Checking the comment counter - // conveniently does that. - await expect(page.locator('#review-box .js-btn-review > span.review-comments-counter')).toHaveText('1'); - - await page.locator('#review-box .js-btn-review').click(); - await expect(page.locator('.tippy-box .review-box-panel')).toBeVisible(); - await save_visual(page); - - await page.locator('.review-box-panel textarea.markdown-text-editor') - .fill('This is a review'); - await page.locator('.review-box-panel button.btn-submit[value="approve"]').click(); - await page.waitForURL(/.*\/user2\/repo1\/pulls\/3#issuecomment-\d+/); - await save_visual(page); - - // In addition to testing the ability to delete comments, this also - // performs clean up. If tests are run for multiple platforms, the data isn't reset - // in-between, and subsequent runs of this test would fail, because when there already is - // a comment, the on-hover button to start a conversation doesn't appear anymore. - await page.goto('/user2/repo1/pulls/3/commits/4a357436d925b5c974181ff12a994538ddc5a269'); - await page.locator('.comment-header-right.actions a.context-menu').click(); - - await expect(page.locator('.comment-header-right.actions div.menu').getByText(/Copy link.*/)).toBeVisible(); - // The button to delete a comment will prompt for confirmation using a browser alert. - page.on('dialog', (dialog) => dialog.accept()); - await page.locator('.comment-header-right.actions div.menu .delete-comment').click(); - - await expect(page.locator('.comment-list .comment-container')).toBeHidden(); - await save_visual(page); -}); - -test('PR: Navigate by single commit', async ({page}) => { - const response = await page.goto('/user2/repo1/pulls/3/commits'); - expect(response?.status()).toBe(200); - - await page.locator('tbody.commit-list td.message a').nth(1).click(); - await page.waitForURL(/.*\/user2\/repo1\/pulls\/3\/commits\/4a357436d925b5c974181ff12a994538ddc5a269/); - await save_visual(page); - - let prevButton = page.locator('.commit-header-buttons').getByText(/Prev/); - let nextButton = page.locator('.commit-header-buttons').getByText(/Next/); - await prevButton.waitFor(); - await nextButton.waitFor(); - - await expect(prevButton).toHaveClass(/disabled/); - await expect(nextButton).not.toHaveClass(/disabled/); - await expect(nextButton).toHaveAttribute('href', '/user2/repo1/pulls/3/commits/5f22f7d0d95d614d25a5b68592adb345a4b5c7fd'); - await nextButton.click(); - - await page.waitForURL(/.*\/user2\/repo1\/pulls\/3\/commits\/5f22f7d0d95d614d25a5b68592adb345a4b5c7fd/); - await save_visual(page); - - prevButton = page.locator('.commit-header-buttons').getByText(/Prev/); - nextButton = page.locator('.commit-header-buttons').getByText(/Next/); - await prevButton.waitFor(); - await nextButton.waitFor(); - - await expect(prevButton).not.toHaveClass(/disabled/); - await expect(nextButton).toHaveClass(/disabled/); - await expect(prevButton).toHaveAttribute('href', '/user2/repo1/pulls/3/commits/4a357436d925b5c974181ff12a994538ddc5a269'); }); diff --git a/tests/gitea-repositories-meta/user2/commitsonpr.git/logs/refs/heads/branch1 b/tests/gitea-repositories-meta/user2/commitsonpr.git/logs/refs/heads/branch1 index 34c9ccecdd..cf96195665 100644 --- a/tests/gitea-repositories-meta/user2/commitsonpr.git/logs/refs/heads/branch1 +++ b/tests/gitea-repositories-meta/user2/commitsonpr.git/logs/refs/heads/branch1 @@ -1,2 +1 @@ 0000000000000000000000000000000000000000 1978192d98bb1b65e11c2cf37da854fbf94bffd6 Gitea 1688672383 +0200 push -1978192d98bb1b65e11c2cf37da854fbf94bffd6 9b93963cf6de4dc33f915bb67f192d099c301f43 Forgejo 1749737639 +0200 push diff --git a/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/06/c5865eaeaaa2f92e8fce75a281f6272ee68e90 b/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/06/c5865eaeaaa2f92e8fce75a281f6272ee68e90 deleted file mode 100644 index 009c9849d9..0000000000 Binary files a/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/06/c5865eaeaaa2f92e8fce75a281f6272ee68e90 and /dev/null differ diff --git a/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/2d/65d92dc800c6f448541240c18e82bf36b954bb b/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/2d/65d92dc800c6f448541240c18e82bf36b954bb deleted file mode 100644 index e50c7ad5eb..0000000000 Binary files a/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/2d/65d92dc800c6f448541240c18e82bf36b954bb and /dev/null differ diff --git a/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/9b/93963cf6de4dc33f915bb67f192d099c301f43 b/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/9b/93963cf6de4dc33f915bb67f192d099c301f43 deleted file mode 100644 index 41814996f1..0000000000 Binary files a/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/9b/93963cf6de4dc33f915bb67f192d099c301f43 and /dev/null differ diff --git a/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/ab/a2b386a1eb0b0273ada0fed4f7d075d6e343c1 b/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/ab/a2b386a1eb0b0273ada0fed4f7d075d6e343c1 deleted file mode 100644 index 2d936251cd..0000000000 Binary files a/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/ab/a2b386a1eb0b0273ada0fed4f7d075d6e343c1 and /dev/null differ diff --git a/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/b3/b178fe7c45986f6325aeac1b036c74825ae8f4 b/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/b3/b178fe7c45986f6325aeac1b036c74825ae8f4 deleted file mode 100644 index 7852ab9100..0000000000 Binary files a/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/b3/b178fe7c45986f6325aeac1b036c74825ae8f4 and /dev/null differ diff --git a/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/c9/cf8a3095808af2425255056e01746fef420801 b/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/c9/cf8a3095808af2425255056e01746fef420801 deleted file mode 100644 index 32a51a2837..0000000000 Binary files a/tests/gitea-repositories-meta/user2/commitsonpr.git/objects/c9/cf8a3095808af2425255056e01746fef420801 and /dev/null differ diff --git a/tests/gitea-repositories-meta/user2/commitsonpr.git/refs/heads/branch1 b/tests/gitea-repositories-meta/user2/commitsonpr.git/refs/heads/branch1 index 6fc9179bdd..357fc9d6ed 100644 --- a/tests/gitea-repositories-meta/user2/commitsonpr.git/refs/heads/branch1 +++ b/tests/gitea-repositories-meta/user2/commitsonpr.git/refs/heads/branch1 @@ -1 +1 @@ -9b93963cf6de4dc33f915bb67f192d099c301f43 +1978192d98bb1b65e11c2cf37da854fbf94bffd6 diff --git a/tests/gitea-repositories-meta/user2/commitsonpr.git/refs/pull/1/head b/tests/gitea-repositories-meta/user2/commitsonpr.git/refs/pull/1/head index 6fc9179bdd..357fc9d6ed 100644 --- a/tests/gitea-repositories-meta/user2/commitsonpr.git/refs/pull/1/head +++ b/tests/gitea-repositories-meta/user2/commitsonpr.git/refs/pull/1/head @@ -1 +1 @@ -9b93963cf6de4dc33f915bb67f192d099c301f43 +1978192d98bb1b65e11c2cf37da854fbf94bffd6 diff --git a/tests/integration/README.md b/tests/integration/README.md index a6be7fe72c..d83685388e 100644 --- a/tests/integration/README.md +++ b/tests/integration/README.md @@ -71,11 +71,11 @@ TEST_MYSQL_HOST=localhost:3306 TEST_MYSQL_DBNAME=test?multiStatements=true TEST_ ### Run pgsql integration tests Setup a pgsql database inside docker ``` -docker run -e "POSTGRES_DB=test" -e POSTGRES_PASSWORD=postgres POSTGRESQL_FSYNC=off POSTGRESQL_EXTRA_FLAGS="-c full_page_writes=off" -p 5432:5432 --rm --name pgsql data.forgejo.org/oci/bitnami/postgresql:16 #(Ctrl-c to stop the database) +docker run -e "POSTGRES_DB=test" -e POSTGRES_PASSWORD=postgres -p 5432:5432 --rm --name pgsql postgres:latest #(Ctrl-c to stop the database) ``` Start tests based on the database container ``` -TEST_STORAGE_TYPE=local TEST_PGSQL_HOST=localhost:5432 TEST_PGSQL_DBNAME=test TEST_PGSQL_USERNAME=postgres TEST_PGSQL_PASSWORD=postgres make 'test-pgsql#Test' +TEST_STORAGE_TYPE=local TEST_PGSQL_HOST=localhost:5432 TEST_PGSQL_DBNAME=test TEST_PGSQL_USERNAME=postgres TEST_PGSQL_PASSWORD=postgres make test-pgsql ``` ### Running individual tests diff --git a/tests/integration/patch_status_test.go b/tests/integration/patch_status_test.go index 3ce1dc4cb9..078051fe63 100644 --- a/tests/integration/patch_status_test.go +++ b/tests/integration/patch_status_test.go @@ -4,7 +4,6 @@ package integration import ( - "context" "fmt" "net/http" "net/url" @@ -21,10 +20,7 @@ import ( user_model "forgejo.org/models/user" "forgejo.org/modules/git" "forgejo.org/modules/optional" - "forgejo.org/modules/test" - pull_service "forgejo.org/services/pull" files_service "forgejo.org/services/repository/files" - shared_automerge "forgejo.org/services/shared/automerge" "forgejo.org/tests" "github.com/stretchr/testify/assert" @@ -55,20 +51,6 @@ func TestPatchStatus(t *testing.T) { }) defer f() - testAutomergeQueued := func(t *testing.T, pr *issues_model.PullRequest, expected issues_model.PullRequestStatus) { - t.Helper() - - var actual issues_model.PullRequestStatus = -1 - defer test.MockVariableValue(&shared_automerge.AddToQueueIfMergeable, func(ctx context.Context, pull *issues_model.PullRequest) { - actual = pull.Status - })() - - pull_service.AddToTaskQueue(t.Context(), pr) - assert.Eventually(t, func() bool { - return expected == actual - }, time.Second*5, time.Millisecond*200) - } - testRepoFork(t, session, "user2", repo.Name, "org3", "forked-repo") forkRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "org3", Name: "forked-repo"}) @@ -109,9 +91,7 @@ func TestPatchStatus(t *testing.T) { require.NoError(t, git.NewCommand(t.Context(), "push", "fork", "HEAD:normal").Run(&git.RunOpts{Dir: dstPath})) testPullCreateDirectly(t, session, repo.OwnerName, repo.Name, repo.DefaultBranch, forkRepo.OwnerName, forkRepo.Name, "normal", "across repo normal") - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadRepoID: forkRepo.ID, HeadBranch: "normal"}, "flow = 0") - test(t, pr) - testAutomergeQueued(t, pr, issues_model.PullRequestStatusMergeable) + test(t, unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadRepoID: forkRepo.ID, HeadBranch: "normal"}, "flow = 0")) }) t.Run("Same repository", func(t *testing.T) { @@ -164,9 +144,7 @@ func TestPatchStatus(t *testing.T) { t.Run("Existing", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadRepoID: forkRepo.ID, HeadBranch: "normal"}, "flow = 0") - test(t, pr) - testAutomergeQueued(t, pr, issues_model.PullRequestStatusConflict) + test(t, unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadRepoID: forkRepo.ID, HeadBranch: "normal"}, "flow = 0")) }) t.Run("New", func(t *testing.T) { diff --git a/tests/integration/pull_commit_test.go b/tests/integration/pull_commit_test.go index 8ca78f8147..90d16d725d 100644 --- a/tests/integration/pull_commit_test.go +++ b/tests/integration/pull_commit_test.go @@ -31,20 +31,3 @@ func TestListPullCommits(t *testing.T) { } assert.Equal(t, "4a357436d925b5c974181ff12a994538ddc5a269", pullCommitList.LastReviewCommitSha) } - -func TestPullCommitLinks(t *testing.T) { - defer tests.PrepareTestEnv(t)() - - req := NewRequest(t, "GET", "/user2/repo1/pulls/3/commits") - resp := MakeRequest(t, req, http.StatusOK) - - htmlDoc := NewHTMLParser(t, resp.Body) - - commitSha := htmlDoc.Find(".commit-list td.sha a.sha.label").First() - commitShaHref, _ := commitSha.Attr("href") - assert.Equal(t, "/user2/repo1/pulls/3/commits/5f22f7d0d95d614d25a5b68592adb345a4b5c7fd", commitShaHref) - - commitLink := htmlDoc.Find(".commit-list td.message a").First() - commitLinkHref, _ := commitLink.Attr("href") - assert.Equal(t, "/user2/repo1/pulls/3/commits/5f22f7d0d95d614d25a5b68592adb345a4b5c7fd", commitLinkHref) -} diff --git a/tests/integration/pull_diff_test.go b/tests/integration/pull_diff_test.go index 6db9fc25d8..70e0d5d33a 100644 --- a/tests/integration/pull_diff_test.go +++ b/tests/integration/pull_diff_test.go @@ -14,22 +14,22 @@ import ( ) func TestPullDiff_CompletePRDiff(t *testing.T) { - doTestPRDiff(t, "/user2/commitsonpr/pulls/1/files", []string{"test1.txt", "test10.txt", "test2.txt", "test3.txt", "test4.txt", "test5.txt", "test6.txt", "test7.txt", "test8.txt", "test9.txt"}) + doTestPRDiff(t, "/user2/commitsonpr/pulls/1/files", false, []string{"test1.txt", "test10.txt", "test2.txt", "test3.txt", "test4.txt", "test5.txt", "test6.txt", "test7.txt", "test8.txt", "test9.txt"}) } func TestPullDiff_SingleCommitPRDiff(t *testing.T) { - doTestPRDiff(t, "/user2/commitsonpr/pulls/1/commits/c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2", []string{"test3.txt"}) + doTestPRDiff(t, "/user2/commitsonpr/pulls/1/commits/c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2", true, []string{"test3.txt"}) } func TestPullDiff_CommitRangePRDiff(t *testing.T) { - doTestPRDiff(t, "/user2/commitsonpr/pulls/1/files/4ca8bcaf27e28504df7bf996819665986b01c847..23576dd018294e476c06e569b6b0f170d0558705", []string{"test2.txt", "test3.txt", "test4.txt"}) + doTestPRDiff(t, "/user2/commitsonpr/pulls/1/files/4ca8bcaf27e28504df7bf996819665986b01c847..23576dd018294e476c06e569b6b0f170d0558705", true, []string{"test2.txt", "test3.txt", "test4.txt"}) } func TestPullDiff_StartingFromBaseToCommitPRDiff(t *testing.T) { - doTestPRDiff(t, "/user2/commitsonpr/pulls/1/files/c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2", []string{"test1.txt", "test2.txt", "test3.txt"}) + doTestPRDiff(t, "/user2/commitsonpr/pulls/1/files/c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2", true, []string{"test1.txt", "test2.txt", "test3.txt"}) } -func doTestPRDiff(t *testing.T, prDiffURL string, expectedFilenames []string) { +func doTestPRDiff(t *testing.T, prDiffURL string, reviewBtnDisabled bool, expectedFilenames []string) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2") @@ -52,4 +52,7 @@ func doTestPRDiff(t *testing.T, prDiffURL string, expectedFilenames []string) { filename, _ := s.Attr("data-old-filename") assert.Equal(t, expectedFilenames[i], filename) }) + + // Ensure the review button is enabled for full PR reviews + assert.Equal(t, reviewBtnDisabled, doc.doc.Find(".js-btn-review").HasClass("disabled")) } diff --git a/tests/integration/pull_files_test.go b/tests/integration/pull_files_test.go deleted file mode 100644 index eb7072a4e6..0000000000 --- a/tests/integration/pull_files_test.go +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright 2025 The Forgejo Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package integration - -import ( - "net/http" - "strings" - "testing" - "time" - - repo_model "forgejo.org/models/repo" - "forgejo.org/models/unittest" - "forgejo.org/modules/git" - "forgejo.org/tests" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestPullFilesCommitHeader(t *testing.T) { - defer tests.PrepareTestEnv(t)() - - t.Run("Verify commit info", func(t *testing.T) { - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - gitRepo, err := git.OpenRepository(git.DefaultContext, repo.RepoPath()) - require.NoError(t, err) - defer gitRepo.Close() - - commit, err := gitRepo.GetCommit("62fb502a7172d4453f0322a2cc85bddffa57f07a") - require.NoError(t, err) - - req := NewRequest(t, "GET", "/user2/repo1/pulls/5/commits/62fb502a7172d4453f0322a2cc85bddffa57f07a") - resp := MakeRequest(t, req, http.StatusOK) - - htmlDoc := NewHTMLParser(t, resp.Body) - header := htmlDoc.doc.Find("#diff-commit-header") - - summary := header.Find(".commit-header h3") - assert.Equal(t, commit.Summary(), strings.TrimSpace(summary.Text())) - - author := header.Find(".author strong") - assert.Equal(t, commit.Author.Name, author.Text()) - - date, _ := header.Find("#authored-time relative-time").Attr("datetime") - assert.Equal(t, commit.Author.When.Format(time.RFC3339), date) - - sha := header.Find(".commit-header-row .sha.label") - shaHref, _ := sha.Attr("href") - assert.Equal(t, commit.ID.String()[:10], sha.Find(".shortsha").Text()) - assert.Equal(t, "/user2/repo1/commit/62fb502a7172d4453f0322a2cc85bddffa57f07a", shaHref) - }) - - t.Run("Navigation", func(t *testing.T) { - t.Run("No previous on first commit", func(t *testing.T) { - req := NewRequest(t, "GET", "/user2/commitsonpr/pulls/1/commits/4ca8bcaf27e28504df7bf996819665986b01c847") - resp := MakeRequest(t, req, http.StatusOK) - - htmlDoc := NewHTMLParser(t, resp.Body) - buttons := htmlDoc.doc.Find(".commit-header-buttons a.small.button") - - assert.Equal(t, 2, buttons.Length(), "expected two buttons in commit header") - - assert.True(t, buttons.First().HasClass("disabled"), "'prev' button should be disabled") - assert.False(t, buttons.Last().HasClass("disabled"), "'next' button should not be disabled") - - href, _ := buttons.Last().Attr("href") - assert.Equal(t, "/user2/commitsonpr/pulls/1/commits/96cef4a7b72b3c208340ae6f0cf55a93e9077c93", href) - }) - - t.Run("No next on last commit", func(t *testing.T) { - req := NewRequest(t, "GET", "/user2/commitsonpr/pulls/1/commits/9b93963cf6de4dc33f915bb67f192d099c301f43") - resp := MakeRequest(t, req, http.StatusOK) - - htmlDoc := NewHTMLParser(t, resp.Body) - buttons := htmlDoc.doc.Find(".commit-header-buttons a.small.button") - - assert.Equal(t, 2, buttons.Length(), "expected two buttons in commit header") - - assert.False(t, buttons.First().HasClass("disabled"), "'prev' button should not be disabled") - assert.True(t, buttons.Last().HasClass("disabled"), "'next' button should be disabled") - - href, _ := buttons.First().Attr("href") - assert.Equal(t, "/user2/commitsonpr/pulls/1/commits/2d65d92dc800c6f448541240c18e82bf36b954bb", href) - }) - - t.Run("Both directions on middle commit", func(t *testing.T) { - req := NewRequest(t, "GET", "/user2/commitsonpr/pulls/1/commits/c5626fc9eff57eb1bb7b796b01d4d0f2f3f792a2") - resp := MakeRequest(t, req, http.StatusOK) - - htmlDoc := NewHTMLParser(t, resp.Body) - buttons := htmlDoc.doc.Find(".commit-header-buttons a.small.button") - - assert.Equal(t, 2, buttons.Length(), "expected two buttons in commit header") - - assert.False(t, buttons.First().HasClass("disabled"), "'prev' button should not be disabled") - assert.False(t, buttons.Last().HasClass("disabled"), "'next' button should not be disabled") - - href, _ := buttons.First().Attr("href") - assert.Equal(t, "/user2/commitsonpr/pulls/1/commits/96cef4a7b72b3c208340ae6f0cf55a93e9077c93", href) - - href, _ = buttons.Last().Attr("href") - assert.Equal(t, "/user2/commitsonpr/pulls/1/commits/23576dd018294e476c06e569b6b0f170d0558705", href) - }) - }) -} diff --git a/tests/integration/pull_test.go b/tests/integration/pull_test.go index fd9dbf8888..d5321f6ae5 100644 --- a/tests/integration/pull_test.go +++ b/tests/integration/pull_test.go @@ -30,43 +30,6 @@ func TestViewPulls(t *testing.T) { assert.Equal(t, "Search pulls…", placeholder) } -func TestPullViewConversation(t *testing.T) { - defer tests.PrepareTestEnv(t)() - - req := NewRequest(t, "GET", "/user2/commitsonpr/pulls/1") - resp := MakeRequest(t, req, http.StatusOK) - - htmlDoc := NewHTMLParser(t, resp.Body) - - t.Run("Commits", func(t *testing.T) { - commitLists := htmlDoc.Find(".timeline-item.commits-list") - assert.Equal(t, 4, commitLists.Length()) - - commits := commitLists.Find(".singular-commit") - assert.Equal(t, 10, commits.Length()) - - // First one has not been affected by a force push, therefore it's still part of the - // PR and should link to the PR-scoped review tab - firstCommit := commits.Eq(0) - firstCommitMessageHref, _ := firstCommit.Find("a.default-link").Attr("href") - firstCommitShaHref, _ := firstCommit.Find("a.sha.label").Attr("href") - assert.Equal(t, "/user2/commitsonpr/pulls/1/commits/4ca8bcaf27e28504df7bf996819665986b01c847", firstCommitMessageHref) - assert.Equal(t, "/user2/commitsonpr/pulls/1/commits/4ca8bcaf27e28504df7bf996819665986b01c847", firstCommitShaHref) - - // The fifth commit has been overwritten by a force push. - // Attempting to view the old one in the review tab won't work: - req := NewRequest(t, "GET", "/user2/commitsonpr/pulls/1/commits/3e64625bd6eb5bcba69ac97de6c8f507402df861") - MakeRequest(t, req, http.StatusNotFound) - - // Therefore, this commit should link to the non-PR commit view instead - fifthCommit := commits.Eq(4) - fifthCommitMessageHref, _ := fifthCommit.Find("a.default-link").Attr("href") - fifthCommitShaHref, _ := fifthCommit.Find("a.sha.label").Attr("href") - assert.Equal(t, "/user2/commitsonpr/commit/3e64625bd6eb5bcba69ac97de6c8f507402df861", fifthCommitMessageHref) - assert.Equal(t, "/user2/commitsonpr/commit/3e64625bd6eb5bcba69ac97de6c8f507402df861", fifthCommitShaHref) - }) -} - func TestPullManuallyMergeWarning(t *testing.T) { defer tests.PrepareTestEnv(t)() diff --git a/tests/integration/repo_generate_test.go b/tests/integration/repo_generate_test.go index 44987c14b0..d22e6ecc74 100644 --- a/tests/integration/repo_generate_test.go +++ b/tests/integration/repo_generate_test.go @@ -15,7 +15,6 @@ import ( repo_model "forgejo.org/models/repo" "forgejo.org/models/unittest" user_model "forgejo.org/models/user" - "forgejo.org/modules/git" "forgejo.org/modules/optional" "forgejo.org/modules/setting" "forgejo.org/modules/test" @@ -44,13 +43,9 @@ func assertRepoCreateForm(t *testing.T, htmlDoc *HTMLDoc, owner *user_model.User // the template menu is loaded client-side, so don't assert the option exists assert.Equal(t, templateID, htmlDoc.GetInputValueByName("repo_template"), "Unexpected repo_template selection") - for _, name := range []string{"issue_labels", "gitignores", "license"} { + for _, name := range []string{"issue_labels", "gitignores", "license", "object_format_name"} { htmlDoc.AssertDropdownHasOptions(t, name) } - - if git.SupportHashSha256 { - htmlDoc.AssertDropdownHasOptions(t, "object_format_name") - } } func testRepoGenerate(t *testing.T, session *TestSession, templateID, templateOwnerName, templateRepoName string, user, generateOwner *user_model.User, generateRepoName string) { diff --git a/tools/lint-go-gopls.sh b/tools/lint-go-gopls.sh new file mode 100755 index 0000000000..a222ea14d7 --- /dev/null +++ b/tools/lint-go-gopls.sh @@ -0,0 +1,23 @@ +#!/bin/bash +set -uo pipefail + +cd "$(dirname -- "${BASH_SOURCE[0]}")" && cd .. + +IGNORE_PATTERNS=( + "is deprecated" # TODO: fix these +) + +# lint all go files with 'gopls check' and look for lines starting with the +# current absolute path, indicating a error was found. This is necessary +# because the tool does not set non-zero exit code when errors are found. +# ref: https://github.com/golang/go/issues/67078 +ERROR_LINES=$("$GO" run "$GOPLS_PACKAGE" check "$@" 2>/dev/null | grep -E "^$PWD" | grep -vFf <(printf '%s\n' "${IGNORE_PATTERNS[@]}")); +NUM_ERRORS=$(echo -n "$ERROR_LINES" | wc -l) + +if [ "$NUM_ERRORS" -eq "0" ]; then + exit 0; +else + echo "$ERROR_LINES" + echo "Found $NUM_ERRORS 'gopls check' errors" + exit 1; +fi diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 144424937c..cf916f0361 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -2468,21 +2468,8 @@ tbody.commit-list { display: flex; } -#diff-content-container { - flex: 1; -} - -#diff-commit-header { - /* Counteract the `+2px` for width in `.segment` */ - padding: 0 2px; -} - -#diff-commit-header + h4, -#diff-commit-header + #diff-file-boxes { - margin-top: 8px; -} - #diff-file-boxes { + flex: 1; max-width: 100%; display: flex; flex-direction: column;