From 5bc030efa2cf88ce7f1ec8d8b33c60a7e9408332 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 13 Dec 2024 14:45:32 +0800 Subject: [PATCH 01/19] Fix various UI bugs (#32821) --- modules/markup/markdown/markdown_math_test.go | 40 +++++++++---------- .../markup/markdown/math/block_renderer.go | 13 +++++- .../markup/markdown/math/inline_renderer.go | 9 ++--- templates/repo/view_list.tmpl | 2 +- web_src/css/repo/clone.css | 3 ++ web_src/css/repo/home-file-list.css | 5 +++ web_src/js/markup/math.ts | 5 +-- 7 files changed, 47 insertions(+), 30 deletions(-) diff --git a/modules/markup/markdown/markdown_math_test.go b/modules/markup/markdown/markdown_math_test.go index e371b1c74ab..a2213b2ce76 100644 --- a/modules/markup/markdown/markdown_math_test.go +++ b/modules/markup/markdown/markdown_math_test.go @@ -20,23 +20,23 @@ func TestMathRender(t *testing.T) { }{ { "$a$", - `

a

` + nl, + `

a

` + nl, }, { "$ a $", - `

a

` + nl, + `

a

` + nl, }, { "$a$ $b$", - `

a b

` + nl, + `

a b

` + nl, }, { `\(a\) \(b\)`, - `

a b

` + nl, + `

a b

` + nl, }, { `$a$.`, - `

a.

` + nl, + `

a.

` + nl, }, { `.$a$`, @@ -64,27 +64,27 @@ func TestMathRender(t *testing.T) { }, { "$a$ ($b$) [$c$] {$d$}", - `

a (b) [$c$] {$d$}

` + nl, + `

a (b) [$c$] {$d$}

` + nl, }, { "$$a$$", - `a` + nl, + `a` + nl, }, { "$$a$$ test", - `

a test

` + nl, + `

a test

` + nl, }, { "test $$a$$", - `

test a

` + nl, + `

test a

` + nl, }, { `foo $x=\$$ bar`, - `

foo x=\$ bar

` + nl, + `

foo x=\$ bar

` + nl, }, { `$\text{$b$}$`, - `

\text{$b$}

` + nl, + `

\text{$b$}

` + nl, }, } @@ -110,7 +110,7 @@ func TestMathRenderBlockIndent(t *testing.T) { \alpha \] `, - `

+			`

 \alpha
 
`, @@ -122,7 +122,7 @@ func TestMathRenderBlockIndent(t *testing.T) { \alpha \] `, - `

+			`

 \alpha
 
`, @@ -137,7 +137,7 @@ a d \] `, - `

+			`

 a
 b
 c
@@ -154,7 +154,7 @@ c
   c
   \]
 `,
-			`

+			`

 a
  b
 c
@@ -165,7 +165,7 @@ c
 			"indent-0-oneline",
 			`$$ x $$
 foo`,
-			` x 
+			` x 
 

foo

`, }, @@ -173,7 +173,7 @@ foo`, "indent-3-oneline", ` $$ x $$ foo`, - ` x + ` x

foo

`, }, @@ -188,10 +188,10 @@ foo`, > \] `, `
-

+

 a
 
-

+

 b
 
@@ -207,7 +207,7 @@ b 2. b`, `
  1. a -
    
    +
    
     x
     
  2. diff --git a/modules/markup/markdown/math/block_renderer.go b/modules/markup/markdown/math/block_renderer.go index a770efa01c7..c29f0618821 100644 --- a/modules/markup/markdown/math/block_renderer.go +++ b/modules/markup/markdown/math/block_renderer.go @@ -12,6 +12,17 @@ import ( "github.com/yuin/goldmark/util" ) +// Block render output: +//
    ...
    +// +// Keep in mind that there is another "code block" render in "func (r *GlodmarkRender) highlightingRenderer" +// "highlightingRenderer" outputs the math block with extra "chroma" class: +//
    ...
    +// +// Special classes: +// * "is-loading": show a loading indicator +// * "display": used by JS to decide to render as a block, otherwise render as inline + // BlockRenderer represents a renderer for math Blocks type BlockRenderer struct { renderInternal *internal.RenderInternal @@ -38,7 +49,7 @@ func (r *BlockRenderer) writeLines(w util.BufWriter, source []byte, n gast.Node) func (r *BlockRenderer) renderBlock(w util.BufWriter, source []byte, node gast.Node, entering bool) (gast.WalkStatus, error) { n := node.(*Block) if entering { - code := giteaUtil.Iif(n.Inline, "", `
    `) + ``
    +		code := giteaUtil.Iif(n.Inline, "", `
    `) + ``
     		_ = r.renderInternal.FormatWithSafeAttrs(w, code)
     		r.writeLines(w, source, n)
     	} else {
    diff --git a/modules/markup/markdown/math/inline_renderer.go b/modules/markup/markdown/math/inline_renderer.go
    index 0cff4f1e74e..4e0531cf404 100644
    --- a/modules/markup/markdown/math/inline_renderer.go
    +++ b/modules/markup/markdown/math/inline_renderer.go
    @@ -13,6 +13,9 @@ import (
     	"github.com/yuin/goldmark/util"
     )
     
    +// Inline render output:
    +// ...
    +
     // InlineRenderer is an inline renderer
     type InlineRenderer struct {
     	renderInternal *internal.RenderInternal
    @@ -25,11 +28,7 @@ func NewInlineRenderer(renderInternal *internal.RenderInternal) renderer.NodeRen
     
     func (r *InlineRenderer) renderInline(w util.BufWriter, source []byte, n ast.Node, entering bool) (ast.WalkStatus, error) {
     	if entering {
    -		extraClass := ""
    -		if _, ok := n.(*InlineBlock); ok {
    -			extraClass = "display "
    -		}
    -		_ = r.renderInternal.FormatWithSafeAttrs(w, ``, extraClass)
    +		_ = r.renderInternal.FormatWithSafeAttrs(w, ``)
     		for c := n.FirstChild(); c != nil; c = c.NextSibling() {
     			segment := c.(*ast.Text).Segment
     			value := util.EscapeHTML(segment.Value(source))
    diff --git a/templates/repo/view_list.tmpl b/templates/repo/view_list.tmpl
    index ea61c3736ae..0fdb45e574c 100644
    --- a/templates/repo/view_list.tmpl
    +++ b/templates/repo/view_list.tmpl
    @@ -1,6 +1,6 @@
     {{/* use grid layout, still use the old ID because there are many other CSS styles depending on this ID */}}
     
    -
    +
    {{template "repo/latest_commit" .}}
    {{if and .LatestCommit .LatestCommit.Committer}}{{DateUtils.TimeSince .LatestCommit.Committer.When}}{{end}}
    diff --git a/web_src/css/repo/clone.css b/web_src/css/repo/clone.css index 15709a78f65..3f6a1323fea 100644 --- a/web_src/css/repo/clone.css +++ b/web_src/css/repo/clone.css @@ -1,11 +1,14 @@ /* only used by "repo/empty.tmpl" */ .clone-buttons-combo { + display: flex; + align-items: center; flex: 1; } .clone-buttons-combo input { border-left: none !important; border-radius: 0 !important; + height: 30px; } /* used by the clone-panel popup */ diff --git a/web_src/css/repo/home-file-list.css b/web_src/css/repo/home-file-list.css index eab2124d6f4..ecb26fa6629 100644 --- a/web_src/css/repo/home-file-list.css +++ b/web_src/css/repo/home-file-list.css @@ -44,6 +44,10 @@ padding: 6px 10px; } +#repo-files-table .repo-file-last-commit { + background: var(--color-box-header); +} + #repo-files-table .repo-file-cell.name { max-width: 300px; white-space: nowrap; @@ -59,6 +63,7 @@ } #repo-files-table .repo-file-cell.age { + text-align: right; white-space: nowrap; color: var(--color-text-light-1); } diff --git a/web_src/js/markup/math.ts b/web_src/js/markup/math.ts index 6a1ca2f2e38..22a4de38e98 100644 --- a/web_src/js/markup/math.ts +++ b/web_src/js/markup/math.ts @@ -1,9 +1,8 @@ import {displayError} from './common.ts'; function targetElement(el: Element) { - // The target element is either the current element if it has the - // `is-loading` class or the pre that contains it - return el.classList.contains('is-loading') ? el : el.closest('pre'); + // The target element is either the parent "code block with loading indicator", or itself + return el.closest('.code-block.is-loading') ?? el; } export async function renderMath(): Promise { From a66c16dc1bca44d480317c6319144d33949bc724 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 14 Dec 2024 09:39:05 +0800 Subject: [PATCH 02/19] Allow to fork repository into the same owner (#32819) This feature is experimental, not fully tested, and may be changed in the future. It is only designed for users who really need it: set `[repository].ALLOW_FORK_INTO_SAME_OWNER=true` in your app.ini Doc: https://gitea.com/gitea/docs/pulls/122 ![image](https://github.com/user-attachments/assets/38d08c23-9cfc-49d8-9321-ff81edf65395) --- custom/conf/app.example.ini | 6 +++++- modules/repository/fork.go | 10 +++++++++- modules/repository/fork_test.go | 25 +++++++++++++++++++++++++ modules/setting/repository.go | 1 + routers/web/repo/fork.go | 7 ++++--- 5 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 modules/repository/fork_test.go diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 5c23f70d7ca..6377ebf9d2a 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1040,9 +1040,13 @@ LEVEL = Info ;; Don't allow download source archive files from UI ;DISABLE_DOWNLOAD_SOURCE_ARCHIVES = false -;; Allow fork repositories without maximum number limit +;; Allow to fork repositories without maximum number limit ;ALLOW_FORK_WITHOUT_MAXIMUM_LIMIT = true +;; Allow to fork repositories into the same owner (user or organization) +;; This feature is experimental, not fully tested, and may be changed in the future +;ALLOW_FORK_INTO_SAME_OWNER = false + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;[repository.editor] diff --git a/modules/repository/fork.go b/modules/repository/fork.go index fbf00087167..d5306340716 100644 --- a/modules/repository/fork.go +++ b/modules/repository/fork.go @@ -9,14 +9,22 @@ import ( "code.gitea.io/gitea/models/organization" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" ) +func CanUserForkBetweenOwners(id1, id2 int64) bool { + if id1 != id2 { + return true + } + return setting.Repository.AllowForkIntoSameOwner +} + // CanUserForkRepo returns true if specified user can fork repository. func CanUserForkRepo(ctx context.Context, user *user_model.User, repo *repo_model.Repository) (bool, error) { if user == nil { return false, nil } - if repo.OwnerID != user.ID && !repo_model.HasForkedRepo(ctx, user.ID, repo.ID) { + if CanUserForkBetweenOwners(repo.OwnerID, user.ID) && !repo_model.HasForkedRepo(ctx, user.ID, repo.ID) { return true, nil } ownedOrgs, err := organization.GetOrgsCanCreateRepoByUserID(ctx, user.ID) diff --git a/modules/repository/fork_test.go b/modules/repository/fork_test.go new file mode 100644 index 00000000000..f8c76d942d1 --- /dev/null +++ b/modules/repository/fork_test.go @@ -0,0 +1,25 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repository + +import ( + "testing" + + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" + + "github.com/stretchr/testify/assert" +) + +func TestCanUserForkBetweenOwners(t *testing.T) { + defer test.MockVariableValue(&setting.Repository.AllowForkIntoSameOwner) + + setting.Repository.AllowForkIntoSameOwner = true + assert.True(t, CanUserForkBetweenOwners(1, 1)) + assert.True(t, CanUserForkBetweenOwners(1, 2)) + + setting.Repository.AllowForkIntoSameOwner = false + assert.False(t, CanUserForkBetweenOwners(1, 1)) + assert.True(t, CanUserForkBetweenOwners(1, 2)) +} diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 14cf5805c02..c5619d0f048 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -53,6 +53,7 @@ var ( AllowDeleteOfUnadoptedRepositories bool DisableDownloadSourceArchives bool AllowForkWithoutMaximumLimit bool + AllowForkIntoSameOwner bool // Repository editor settings Editor struct { diff --git a/routers/web/repo/fork.go b/routers/web/repo/fork.go index 27e42a8f98e..86af7056176 100644 --- a/routers/web/repo/fork.go +++ b/routers/web/repo/fork.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/web" @@ -48,7 +49,7 @@ func getForkRepository(ctx *context.Context) *repo_model.Repository { ctx.Data["repo_name"] = forkRepo.Name ctx.Data["description"] = forkRepo.Description ctx.Data["IsPrivate"] = forkRepo.IsPrivate || forkRepo.Owner.Visibility == structs.VisibleTypePrivate - canForkToUser := forkRepo.OwnerID != ctx.Doer.ID && !repo_model.HasForkedRepo(ctx, ctx.Doer.ID, forkRepo.ID) + canForkToUser := repository.CanUserForkBetweenOwners(forkRepo.OwnerID, ctx.Doer.ID) && !repo_model.HasForkedRepo(ctx, ctx.Doer.ID, forkRepo.ID) ctx.Data["ForkRepo"] = forkRepo @@ -66,7 +67,7 @@ func getForkRepository(ctx *context.Context) *repo_model.Repository { traverseParentRepo := forkRepo for { - if ctx.Doer.ID == traverseParentRepo.OwnerID { + if !repository.CanUserForkBetweenOwners(ctx.Doer.ID, traverseParentRepo.OwnerID) { canForkToUser = false } else { for i, org := range orgs { @@ -162,7 +163,7 @@ func ForkPost(ctx *context.Context) { var err error traverseParentRepo := forkRepo for { - if ctxUser.ID == traverseParentRepo.OwnerID { + if !repository.CanUserForkBetweenOwners(ctxUser.ID, traverseParentRepo.OwnerID) { ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplFork, &form) return } From 7269130d2878d51dcdf11f7081a591f85bd493e8 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Sat, 14 Dec 2024 10:22:30 +0800 Subject: [PATCH 03/19] Fix missing outputs for jobs with matrix (#32823) Fix #32795 If a job uses a matrix, multiple `ActionRunJobs` may have the same `JobID`. We need to merge the outputs of these jobs to make them available to the jobs that need them. --- models/actions/run_job.go | 4 +- models/fixtures/action_run.yml | 19 ++++++++ models/fixtures/action_run_job.yml | 43 +++++++++++++++++ models/fixtures/action_task.yml | 60 ++++++++++++++++++++++++ models/fixtures/action_task_output.yml | 20 ++++++++ routers/api/actions/runner/main_test.go | 14 ++++++ routers/api/actions/runner/utils.go | 60 +++++++++++++++++------- routers/api/actions/runner/utils_test.go | 28 +++++++++++ 8 files changed, 230 insertions(+), 18 deletions(-) create mode 100644 models/fixtures/action_task_output.yml create mode 100644 routers/api/actions/runner/main_test.go create mode 100644 routers/api/actions/runner/utils_test.go diff --git a/models/actions/run_job.go b/models/actions/run_job.go index 4b8664077dc..2319af8e085 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -137,7 +137,7 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col if err != nil { return 0, err } - run.Status = aggregateJobStatus(jobs) + run.Status = AggregateJobStatus(jobs) if run.Started.IsZero() && run.Status.IsRunning() { run.Started = timeutil.TimeStampNow() } @@ -152,7 +152,7 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col return affected, nil } -func aggregateJobStatus(jobs []*ActionRunJob) Status { +func AggregateJobStatus(jobs []*ActionRunJob) Status { allDone := true allWaiting := true hasFailure := false diff --git a/models/fixtures/action_run.yml b/models/fixtures/action_run.yml index a42ab77ca5b..0747c46d2f0 100644 --- a/models/fixtures/action_run.yml +++ b/models/fixtures/action_run.yml @@ -36,3 +36,22 @@ updated: 1683636626 need_approval: 0 approved_by: 0 +- + id: 793 + title: "job output" + repo_id: 4 + owner_id: 1 + workflow_id: "test.yaml" + index: 189 + trigger_user_id: 1 + ref: "refs/heads/master" + commit_sha: "c2d72f548424103f01ee1dc02889c1e2bff816b0" + event: "push" + is_fork_pull_request: 0 + status: 1 + started: 1683636528 + stopped: 1683636626 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 diff --git a/models/fixtures/action_run_job.yml b/models/fixtures/action_run_job.yml index fd90f4fd5d2..9b6f5b9a887 100644 --- a/models/fixtures/action_run_job.yml +++ b/models/fixtures/action_run_job.yml @@ -26,3 +26,46 @@ status: 1 started: 1683636528 stopped: 1683636626 +- + id: 194 + run_id: 793 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + name: job1 (1) + attempt: 1 + job_id: job1 + task_id: 49 + status: 1 + started: 1683636528 + stopped: 1683636626 +- + id: 195 + run_id: 793 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + name: job1 (2) + attempt: 1 + job_id: job1 + task_id: 50 + status: 1 + started: 1683636528 + stopped: 1683636626 +- + id: 196 + run_id: 793 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + name: job2 + attempt: 1 + job_id: job2 + needs: [job1] + task_id: 51 + status: 5 + started: 1683636528 + stopped: 1683636626 diff --git a/models/fixtures/action_task.yml b/models/fixtures/action_task.yml index d88a8ed8a91..506a47d8a04 100644 --- a/models/fixtures/action_task.yml +++ b/models/fixtures/action_task.yml @@ -57,3 +57,63 @@ log_length: 707 log_size: 90179 log_expired: 0 +- + id: 49 + job_id: 194 + attempt: 1 + runner_id: 1 + status: 1 # success + started: 1683636528 + stopped: 1683636626 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + token_hash: b8d3962425466b6709b9ac51446f93260c54afe8e7b6d3686e34f991fb8a8953822b0deed86fe41a103f34bc48dbc4784220 + token_salt: ffffffffff + token_last_eight: ffffffff + log_filename: artifact-test2/2f/47.log + log_in_storage: 1 + log_length: 707 + log_size: 90179 + log_expired: 0 +- + id: 50 + job_id: 195 + attempt: 1 + runner_id: 1 + status: 1 # success + started: 1683636528 + stopped: 1683636626 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + token_hash: b8d3962425466b6709b9ac51446f93260c54afe8e7b6d3686e34f991fb8a8953822b0deed86fe41a103f34bc48dbc4784221 + token_salt: ffffffffff + token_last_eight: ffffffff + log_filename: artifact-test2/2f/47.log + log_in_storage: 1 + log_length: 707 + log_size: 90179 + log_expired: 0 +- + id: 51 + job_id: 196 + attempt: 1 + runner_id: 1 + status: 6 # running + started: 1683636528 + stopped: 1683636626 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + token_hash: b8d3962425466b6709b9ac51446f93260c54afe8e7b6d3686e34f991fb8a8953822b0deed86fe41a103f34bc48dbc4784222 + token_salt: ffffffffff + token_last_eight: ffffffff + log_filename: artifact-test2/2f/47.log + log_in_storage: 1 + log_length: 707 + log_size: 90179 + log_expired: 0 diff --git a/models/fixtures/action_task_output.yml b/models/fixtures/action_task_output.yml new file mode 100644 index 00000000000..314e9f7115b --- /dev/null +++ b/models/fixtures/action_task_output.yml @@ -0,0 +1,20 @@ +- + id: 1 + task_id: 49 + output_key: output_a + output_value: abc +- + id: 2 + task_id: 49 + output_key: output_b + output_value: '' +- + id: 3 + task_id: 50 + output_key: output_a + output_value: '' +- + id: 4 + task_id: 50 + output_key: output_b + output_value: bbb diff --git a/routers/api/actions/runner/main_test.go b/routers/api/actions/runner/main_test.go new file mode 100644 index 00000000000..1e80a4f5caf --- /dev/null +++ b/routers/api/actions/runner/main_test.go @@ -0,0 +1,14 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package runner + +import ( + "testing" + + "code.gitea.io/gitea/models/unittest" +) + +func TestMain(m *testing.M) { + unittest.MainTest(m) +} diff --git a/routers/api/actions/runner/utils.go b/routers/api/actions/runner/utils.go index ff6ec5bd54c..539be8d8890 100644 --- a/routers/api/actions/runner/utils.go +++ b/routers/api/actions/runner/utils.go @@ -162,28 +162,56 @@ func findTaskNeeds(ctx context.Context, task *actions_model.ActionTask) (map[str return nil, fmt.Errorf("FindRunJobs: %w", err) } - ret := make(map[string]*runnerv1.TaskNeed, len(needs)) + jobIDJobs := make(map[string][]*actions_model.ActionRunJob) for _, job := range jobs { - if !needs.Contains(job.JobID) { + jobIDJobs[job.JobID] = append(jobIDJobs[job.JobID], job) + } + + ret := make(map[string]*runnerv1.TaskNeed, len(needs)) + for jobID, jobsWithSameID := range jobIDJobs { + if !needs.Contains(jobID) { continue } - if job.TaskID == 0 || !job.Status.IsDone() { - // it shouldn't happen, or the job has been rerun - continue + var jobOutputs map[string]string + for _, job := range jobsWithSameID { + if job.TaskID == 0 || !job.Status.IsDone() { + // it shouldn't happen, or the job has been rerun + continue + } + got, err := actions_model.FindTaskOutputByTaskID(ctx, job.TaskID) + if err != nil { + return nil, fmt.Errorf("FindTaskOutputByTaskID: %w", err) + } + outputs := make(map[string]string, len(got)) + for _, v := range got { + outputs[v.OutputKey] = v.OutputValue + } + if len(jobOutputs) == 0 { + jobOutputs = outputs + } else { + jobOutputs = mergeTwoOutputs(outputs, jobOutputs) + } } - outputs := make(map[string]string) - got, err := actions_model.FindTaskOutputByTaskID(ctx, job.TaskID) - if err != nil { - return nil, fmt.Errorf("FindTaskOutputByTaskID: %w", err) - } - for _, v := range got { - outputs[v.OutputKey] = v.OutputValue - } - ret[job.JobID] = &runnerv1.TaskNeed{ - Outputs: outputs, - Result: runnerv1.Result(job.Status), + ret[jobID] = &runnerv1.TaskNeed{ + Outputs: jobOutputs, + Result: runnerv1.Result(actions_model.AggregateJobStatus(jobsWithSameID)), } } return ret, nil } + +// mergeTwoOutputs merges two outputs from two different ActionRunJobs +// Values with the same output name may be overridden. The user should ensure the output names are unique. +// See https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#using-job-outputs-in-a-matrix-job +func mergeTwoOutputs(o1, o2 map[string]string) map[string]string { + ret := make(map[string]string, len(o1)) + for k1, v1 := range o1 { + if len(v1) > 0 { + ret[k1] = v1 + } else { + ret[k1] = o2[k1] + } + } + return ret +} diff --git a/routers/api/actions/runner/utils_test.go b/routers/api/actions/runner/utils_test.go new file mode 100644 index 00000000000..d7a6f84550f --- /dev/null +++ b/routers/api/actions/runner/utils_test.go @@ -0,0 +1,28 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package runner + +import ( + "context" + "testing" + + actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func Test_findTaskNeeds(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: 51}) + + ret, err := findTaskNeeds(context.Background(), task) + assert.NoError(t, err) + assert.Len(t, ret, 1) + assert.Contains(t, ret, "job1") + assert.Len(t, ret["job1"].Outputs, 2) + assert.Equal(t, "abc", ret["job1"].Outputs["output_a"]) + assert.Equal(t, "bbb", ret["job1"].Outputs["output_b"]) +} From 2ee4aa89989f50182d70fc0d0aec7a032a43debe Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Sat, 14 Dec 2024 04:34:03 +0200 Subject: [PATCH 04/19] Upgrade htmx to 2.0.4 (#32834) Release notes: https://github.com/bigskysoftware/htmx/releases/tag/v2.0.4 Tested `Star`, `Watch`, and the admin dashboard page. All functionality remains unchanged. Signed-off-by: Yarden Shoham --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 53bd5bc4f1f..4764282f65e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -30,7 +30,7 @@ "esbuild-loader": "4.2.2", "escape-goat": "4.0.0", "fast-glob": "3.3.2", - "htmx.org": "2.0.3", + "htmx.org": "2.0.4", "idiomorph": "0.3.0", "jquery": "3.7.1", "katex": "0.16.11", @@ -10557,9 +10557,9 @@ } }, "node_modules/htmx.org": { - "version": "2.0.3", - "resolved": "https://registry.npmjs.org/htmx.org/-/htmx.org-2.0.3.tgz", - "integrity": "sha512-AeoJUAjkCVVajbfKX+3sVQBTCt8Ct4lif1T+z/tptTXo8+8yyq3QIMQQe/IT+R8ssfrO1I0DeX4CAronzCL6oA==", + "version": "2.0.4", + "resolved": "https://registry.npmjs.org/htmx.org/-/htmx.org-2.0.4.tgz", + "integrity": "sha512-HLxMCdfXDOJirs3vBZl/ZLoY+c7PfM4Ahr2Ad4YXh6d22T5ltbTXFFkpx9Tgb2vvmWFMbIc3LqN2ToNkZJvyYQ==", "license": "0BSD" }, "node_modules/iconv-lite": { diff --git a/package.json b/package.json index 3a81e648228..275ca898e29 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "esbuild-loader": "4.2.2", "escape-goat": "4.0.0", "fast-glob": "3.3.2", - "htmx.org": "2.0.3", + "htmx.org": "2.0.4", "idiomorph": "0.3.0", "jquery": "3.7.1", "katex": "0.16.11", From bed563e57490024e43db46ea2ca48094cd4fa7d2 Mon Sep 17 00:00:00 2001 From: silverwind Date: Sat, 14 Dec 2024 04:10:20 +0100 Subject: [PATCH 05/19] Improve JSX/TSX support in code editor (#32833) Two tweaks to Monaco to improve JSX/TSX support. 1. Certain language features like JSX/TSX only work when passing `uri` (containing the filename), do this. 2. Set the `jsx` compiler option to avoid error annotations Before: Screenshot 2024-12-13 at 15 11 33 After: Screenshot 2024-12-13 at 15 10 46 --- web_src/js/features/codeeditor.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/web_src/js/features/codeeditor.ts b/web_src/js/features/codeeditor.ts index 62bfccd1393..af9830a4db2 100644 --- a/web_src/js/features/codeeditor.ts +++ b/web_src/js/features/codeeditor.ts @@ -58,6 +58,12 @@ function initLanguages(monaco: Monaco): void { for (const extension of extensions || []) { languagesByExt[extension] = id; } + if (id === 'typescript') { + monaco.languages.typescript.typescriptDefaults.setCompilerOptions({ + // this is needed to suppress error annotations in tsx regarding missing --jsx flag. + jsx: monaco.languages.typescript.JsxEmit.Preserve, + }); + } } } @@ -72,6 +78,8 @@ function updateEditor(monaco: Monaco, editor: IStandaloneCodeEditor, filename: s const language = model.getLanguageId(); const newLanguage = getLanguage(filename); if (language !== newLanguage) monaco.editor.setModelLanguage(model, newLanguage); + // TODO: Need to update the model uri with the new filename, but there is no easy way currently, see + // https://github.com/microsoft/monaco-editor/discussions/3751 } // export editor for customization - https://github.com/go-gitea/gitea/issues/10409 @@ -135,10 +143,11 @@ export async function createMonaco(textarea: HTMLTextAreaElement, filename: stri }); updateTheme(monaco); + const model = monaco.editor.createModel(textarea.value, language, monaco.Uri.file(filename)); + const editor = monaco.editor.create(container, { - value: textarea.value, + model, theme: 'gitea', - language, ...other, }); @@ -146,8 +155,6 @@ export async function createMonaco(textarea: HTMLTextAreaElement, filename: stri {keybinding: monaco.KeyCode.Enter, command: null}, // disable enter from accepting code completion ]); - const model = editor.getModel(); - if (!model) throw new Error('Unable to get editor model'); model.onDidChangeContent(() => { textarea.value = editor.getValue({ preserveBOM: true, From 82c59d52ea650ce42bbca2c6740d9449d06e77be Mon Sep 17 00:00:00 2001 From: hiifong Date: Sat, 14 Dec 2024 11:35:19 +0800 Subject: [PATCH 06/19] Add User-Agent for gitea's self-implemented lfs client. (#32832) --- modules/lfs/shared.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/modules/lfs/shared.go b/modules/lfs/shared.go index 40ad789c1d9..cd9488e3dbf 100644 --- a/modules/lfs/shared.go +++ b/modules/lfs/shared.go @@ -14,9 +14,12 @@ import ( const ( // MediaType contains the media type for LFS server requests MediaType = "application/vnd.git-lfs+json" - // Some LFS servers offer content with other types, so fallback to '*/*' if application/vnd.git-lfs+json cannot be served - AcceptHeader = "application/vnd.git-lfs+json;q=0.9, */*;q=0.8" - UserAgentHeader = "git-lfs" + // AcceptHeader Some LFS servers offer content with other types, so fallback to '*/*' if application/vnd.git-lfs+json cannot be served + AcceptHeader = "application/vnd.git-lfs+json;q=0.9, */*;q=0.8" + // UserAgentHeader Add User-Agent for gitea's self-implemented lfs client, + // and the version is consistent with the latest version of git lfs can be avoided incompatibilities. + // Some lfs servers will check this + UserAgentHeader = "git-lfs/3.6.0 (Gitea)" ) // BatchRequest contains multiple requests processed in one batch operation. From cc5ff98e0d510c1923ad7cabc3e339f9cf0b570f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 14 Dec 2024 13:43:05 +0800 Subject: [PATCH 07/19] Refactor markdown math render (#32831) Add more tests --- modules/markup/markdown/markdown.go | 28 +++--- modules/markup/markdown/markdown_math_test.go | 20 ++++- modules/markup/markdown/math/block_parser.go | 25 ++++-- .../markup/markdown/math/inline_block_node.go | 31 ------- modules/markup/markdown/math/inline_node.go | 2 +- modules/markup/markdown/math/inline_parser.go | 90 +++++++++++-------- .../markup/markdown/math/inline_renderer.go | 1 - modules/markup/markdown/math/math.go | 68 +++++--------- web_src/css/repo/home-file-list.css | 3 +- web_src/js/markup/math.ts | 20 +++-- 10 files changed, 137 insertions(+), 151 deletions(-) delete mode 100644 modules/markup/markdown/math/inline_block_node.go diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go index f77db9eb38e..a14c0cad597 100644 --- a/modules/markup/markdown/markdown.go +++ b/modules/markup/markdown/markdown.go @@ -78,26 +78,23 @@ func (r *GlodmarkRender) Renderer() renderer.Renderer { func (r *GlodmarkRender) highlightingRenderer(w util.BufWriter, c highlighting.CodeBlockContext, entering bool) { if entering { - language, _ := c.Language() - if language == nil { - language = []byte("text") - } + languageBytes, _ := c.Language() + languageStr := giteautil.IfZero(string(languageBytes), "text") - languageStr := string(language) - - preClasses := []string{"code-block"} + preClasses := "code-block" if languageStr == "mermaid" || languageStr == "math" { - preClasses = append(preClasses, "is-loading") + preClasses += " is-loading" } - err := r.ctx.RenderInternal.FormatWithSafeAttrs(w, `
    `, strings.Join(preClasses, " "))
    +		err := r.ctx.RenderInternal.FormatWithSafeAttrs(w, `
    `, preClasses)
     		if err != nil {
     			return
     		}
     
    -		// include language-x class as part of commonmark spec
    -		// the "display" class is used by "js/markup/math.js" to render the code element as a block
    -		err = r.ctx.RenderInternal.FormatWithSafeAttrs(w, ``, string(language))
    +		// include language-x class as part of commonmark spec, "chroma" class is used to highlight the code
    +		// the "display" class is used by "js/markup/math.ts" to render the code element as a block
    +		// the "math.ts" strictly depends on the structure: 
    ...
    + err = r.ctx.RenderInternal.FormatWithSafeAttrs(w, ``, languageStr) if err != nil { return } @@ -128,7 +125,12 @@ func SpecializedMarkdown(ctx *markup.RenderContext) *GlodmarkRender { ), highlighting.WithWrapperRenderer(r.highlightingRenderer), ), - math.NewExtension(&ctx.RenderInternal, math.Enabled(setting.Markdown.EnableMath)), + math.NewExtension(&ctx.RenderInternal, math.Options{ + Enabled: setting.Markdown.EnableMath, + ParseDollarInline: true, + ParseDollarBlock: true, + ParseSquareBlock: true, // TODO: this is a bad syntax, it should be deprecated in the future (by some config options) + }), meta.Meta, ), goldmark.WithParserOptions( diff --git a/modules/markup/markdown/markdown_math_test.go b/modules/markup/markdown/markdown_math_test.go index a2213b2ce76..813f050965a 100644 --- a/modules/markup/markdown/markdown_math_test.go +++ b/modules/markup/markdown/markdown_math_test.go @@ -12,8 +12,9 @@ import ( "github.com/stretchr/testify/assert" ) +const nl = "\n" + func TestMathRender(t *testing.T) { - const nl = "\n" testcases := []struct { testcase string expected string @@ -86,6 +87,18 @@ func TestMathRender(t *testing.T) { `$\text{$b$}$`, `

    \text{$b$}

    ` + nl, }, + { + "a$`b`$c", + `

    abc

    ` + nl, + }, + { + "a $`b`$ c", + `

    a b c

    ` + nl, + }, + { + "a$``b``$c x$```y```$z", + `

    abc xyz

    ` + nl, + }, } for _, test := range testcases { @@ -215,6 +228,11 @@ x
`, }, + { + "inline-non-math", + `\[x]`, + `

[x]

` + nl, + }, } for _, test := range testcases { diff --git a/modules/markup/markdown/math/block_parser.go b/modules/markup/markdown/math/block_parser.go index 3f37ce83332..2c5553550a7 100644 --- a/modules/markup/markdown/math/block_parser.go +++ b/modules/markup/markdown/math/block_parser.go @@ -16,16 +16,18 @@ import ( type blockParser struct { parseDollars bool + parseSquare bool endBytesDollars []byte - endBytesBracket []byte + endBytesSquare []byte } // NewBlockParser creates a new math BlockParser -func NewBlockParser(parseDollarBlocks bool) parser.BlockParser { +func NewBlockParser(parseDollars, parseSquare bool) parser.BlockParser { return &blockParser{ - parseDollars: parseDollarBlocks, + parseDollars: parseDollars, + parseSquare: parseSquare, endBytesDollars: []byte{'$', '$'}, - endBytesBracket: []byte{'\\', ']'}, + endBytesSquare: []byte{'\\', ']'}, } } @@ -40,7 +42,7 @@ func (b *blockParser) Open(parent ast.Node, reader text.Reader, pc parser.Contex var dollars bool if b.parseDollars && line[pos] == '$' && line[pos+1] == '$' { dollars = true - } else if line[pos] == '\\' && line[pos+1] == '[' { + } else if b.parseSquare && line[pos] == '\\' && line[pos+1] == '[' { if len(line[pos:]) >= 3 && line[pos+2] == '!' && bytes.Contains(line[pos:], []byte(`\]`)) { // do not process escaped attention block: "> \[!NOTE\]" return nil, parser.NoChildren @@ -53,10 +55,10 @@ func (b *blockParser) Open(parent ast.Node, reader text.Reader, pc parser.Contex node := NewBlock(dollars, pos) // Now we need to check if the ending block is on the segment... - endBytes := giteaUtil.Iif(dollars, b.endBytesDollars, b.endBytesBracket) + endBytes := giteaUtil.Iif(dollars, b.endBytesDollars, b.endBytesSquare) idx := bytes.Index(line[pos+2:], endBytes) if idx >= 0 { - // for case $$ ... $$ any other text + // for case: "$$ ... $$ any other text" (this case will be handled by the inline parser) for i := pos + 2 + idx + 2; i < len(line); i++ { if line[i] != ' ' && line[i] != '\n' { return nil, parser.NoChildren @@ -70,6 +72,13 @@ func (b *blockParser) Open(parent ast.Node, reader text.Reader, pc parser.Contex return node, parser.Close | parser.NoChildren } + // for case "\[ ... ]" (no close marker on the same line) + for i := pos + 2 + idx + 2; i < len(line); i++ { + if line[i] != ' ' && line[i] != '\n' { + return nil, parser.NoChildren + } + } + segment.Start += pos + 2 node.Lines().Append(segment) return node, parser.NoChildren @@ -85,7 +94,7 @@ func (b *blockParser) Continue(node ast.Node, reader text.Reader, pc parser.Cont line, segment := reader.PeekLine() w, pos := util.IndentWidth(line, reader.LineOffset()) if w < 4 { - endBytes := giteaUtil.Iif(block.Dollars, b.endBytesDollars, b.endBytesBracket) + endBytes := giteaUtil.Iif(block.Dollars, b.endBytesDollars, b.endBytesSquare) if bytes.HasPrefix(line[pos:], endBytes) && util.IsBlank(line[pos+len(endBytes):]) { if util.IsBlank(line[pos+len(endBytes):]) { newline := giteaUtil.Iif(line[len(line)-1] != '\n', 0, 1) diff --git a/modules/markup/markdown/math/inline_block_node.go b/modules/markup/markdown/math/inline_block_node.go deleted file mode 100644 index c92d0c8d84b..00000000000 --- a/modules/markup/markdown/math/inline_block_node.go +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2024 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package math - -import ( - "github.com/yuin/goldmark/ast" -) - -// InlineBlock represents inline math e.g. $$...$$ -type InlineBlock struct { - Inline -} - -// InlineBlock implements InlineBlock. -func (n *InlineBlock) InlineBlock() {} - -// KindInlineBlock is the kind for math inline block -var KindInlineBlock = ast.NewNodeKind("MathInlineBlock") - -// Kind returns KindInlineBlock -func (n *InlineBlock) Kind() ast.NodeKind { - return KindInlineBlock -} - -// NewInlineBlock creates a new ast math inline block node -func NewInlineBlock() *InlineBlock { - return &InlineBlock{ - Inline{}, - } -} diff --git a/modules/markup/markdown/math/inline_node.go b/modules/markup/markdown/math/inline_node.go index 2221a251bf1..1e4034d54b9 100644 --- a/modules/markup/markdown/math/inline_node.go +++ b/modules/markup/markdown/math/inline_node.go @@ -8,7 +8,7 @@ import ( "github.com/yuin/goldmark/util" ) -// Inline represents inline math e.g. $...$ or \(...\) +// Inline struct represents inline math e.g. $...$ or \(...\) type Inline struct { ast.BaseInline } diff --git a/modules/markup/markdown/math/inline_parser.go b/modules/markup/markdown/math/inline_parser.go index 191d1e5a315..a57abe9f9b0 100644 --- a/modules/markup/markdown/math/inline_parser.go +++ b/modules/markup/markdown/math/inline_parser.go @@ -12,31 +12,25 @@ import ( ) type inlineParser struct { - start []byte - end []byte + trigger []byte + endBytesSingleDollar []byte + endBytesDoubleDollar []byte + endBytesBracket []byte } var defaultInlineDollarParser = &inlineParser{ - start: []byte{'$'}, - end: []byte{'$'}, -} - -var defaultDualDollarParser = &inlineParser{ - start: []byte{'$', '$'}, - end: []byte{'$', '$'}, + trigger: []byte{'$'}, + endBytesSingleDollar: []byte{'$'}, + endBytesDoubleDollar: []byte{'$', '$'}, } func NewInlineDollarParser() parser.InlineParser { return defaultInlineDollarParser } -func NewInlineDualDollarParser() parser.InlineParser { - return defaultDualDollarParser -} - var defaultInlineBracketParser = &inlineParser{ - start: []byte{'\\', '('}, - end: []byte{'\\', ')'}, + trigger: []byte{'\\', '('}, + endBytesBracket: []byte{'\\', ')'}, } func NewInlineBracketParser() parser.InlineParser { @@ -45,7 +39,7 @@ func NewInlineBracketParser() parser.InlineParser { // Trigger triggers this parser on $ or \ func (parser *inlineParser) Trigger() []byte { - return parser.start + return parser.trigger } func isPunctuation(b byte) bool { @@ -64,33 +58,60 @@ func isAlphanumeric(b byte) bool { func (parser *inlineParser) Parse(parent ast.Node, block text.Reader, pc parser.Context) ast.Node { line, _ := block.PeekLine() - if !bytes.HasPrefix(line, parser.start) { + if !bytes.HasPrefix(line, parser.trigger) { // We'll catch this one on the next time round return nil } - precedingCharacter := block.PrecendingCharacter() - if precedingCharacter < 256 && (isAlphanumeric(byte(precedingCharacter)) || isPunctuation(byte(precedingCharacter))) { - // need to exclude things like `a$` from being considered a start - return nil + var startMarkLen int + var stopMark []byte + checkSurrounding := true + if line[0] == '$' { + startMarkLen = 1 + stopMark = parser.endBytesSingleDollar + if len(line) > 1 { + if line[1] == '$' { + startMarkLen = 2 + stopMark = parser.endBytesDoubleDollar + } else if line[1] == '`' { + pos := 1 + for ; pos < len(line) && line[pos] == '`'; pos++ { + } + startMarkLen = pos + stopMark = bytes.Repeat([]byte{'`'}, pos) + stopMark[len(stopMark)-1] = '$' + checkSurrounding = false + } + } + } else { + startMarkLen = 2 + stopMark = parser.endBytesBracket + } + + if checkSurrounding { + precedingCharacter := block.PrecendingCharacter() + if precedingCharacter < 256 && (isAlphanumeric(byte(precedingCharacter)) || isPunctuation(byte(precedingCharacter))) { + // need to exclude things like `a$` from being considered a start + return nil + } } // move the opener marker point at the start of the text - opener := len(parser.start) + opener := startMarkLen // Now look for an ending line depth := 0 ender := -1 for i := opener; i < len(line); i++ { - if depth == 0 && bytes.HasPrefix(line[i:], parser.end) { + if depth == 0 && bytes.HasPrefix(line[i:], stopMark) { succeedingCharacter := byte(0) - if i+len(parser.end) < len(line) { - succeedingCharacter = line[i+len(parser.end)] + if i+len(stopMark) < len(line) { + succeedingCharacter = line[i+len(stopMark)] } // check valid ending character isValidEndingChar := isPunctuation(succeedingCharacter) || isBracket(succeedingCharacter) || succeedingCharacter == ' ' || succeedingCharacter == '\n' || succeedingCharacter == 0 - if !isValidEndingChar { + if checkSurrounding && !isValidEndingChar { break } ender = i @@ -112,21 +133,12 @@ func (parser *inlineParser) Parse(parent ast.Node, block text.Reader, pc parser. block.Advance(opener) _, pos := block.Position() - var node ast.Node - if parser == defaultDualDollarParser { - node = NewInlineBlock() - } else { - node = NewInline() - } + node := NewInline() + segment := pos.WithStop(pos.Start + ender - opener) node.AppendChild(node, ast.NewRawTextSegment(segment)) - block.Advance(ender - opener + len(parser.end)) - - if parser == defaultDualDollarParser { - trimBlock(&(node.(*InlineBlock)).Inline, block) - } else { - trimBlock(node.(*Inline), block) - } + block.Advance(ender - opener + len(stopMark)) + trimBlock(node, block) return node } diff --git a/modules/markup/markdown/math/inline_renderer.go b/modules/markup/markdown/math/inline_renderer.go index 4e0531cf404..d000a7b317a 100644 --- a/modules/markup/markdown/math/inline_renderer.go +++ b/modules/markup/markdown/math/inline_renderer.go @@ -50,5 +50,4 @@ func (r *InlineRenderer) renderInline(w util.BufWriter, source []byte, n ast.Nod // RegisterFuncs registers the renderer for inline math nodes func (r *InlineRenderer) RegisterFuncs(reg renderer.NodeRendererFuncRegisterer) { reg.Register(KindInline, r.renderInline) - reg.Register(KindInlineBlock, r.renderInline) } diff --git a/modules/markup/markdown/math/math.go b/modules/markup/markdown/math/math.go index 7e8defcd4a1..a6ff593d626 100644 --- a/modules/markup/markdown/math/math.go +++ b/modules/markup/markdown/math/math.go @@ -5,6 +5,7 @@ package math import ( "code.gitea.io/gitea/modules/markup/internal" + giteaUtil "code.gitea.io/gitea/modules/util" "github.com/yuin/goldmark" "github.com/yuin/goldmark/parser" @@ -12,70 +13,45 @@ import ( "github.com/yuin/goldmark/util" ) +type Options struct { + Enabled bool + ParseDollarInline bool + ParseDollarBlock bool + ParseSquareBlock bool +} + // Extension is a math extension type Extension struct { - renderInternal *internal.RenderInternal - enabled bool - parseDollarInline bool - parseDollarBlock bool -} - -// Option is the interface Options should implement -type Option interface { - SetOption(e *Extension) -} - -type extensionFunc func(e *Extension) - -func (fn extensionFunc) SetOption(e *Extension) { - fn(e) -} - -// Enabled enables or disables this extension -func Enabled(enable ...bool) Option { - value := true - if len(enable) > 0 { - value = enable[0] - } - return extensionFunc(func(e *Extension) { - e.enabled = value - }) + renderInternal *internal.RenderInternal + options Options } // NewExtension creates a new math extension with the provided options -func NewExtension(renderInternal *internal.RenderInternal, opts ...Option) *Extension { +func NewExtension(renderInternal *internal.RenderInternal, opts ...Options) *Extension { + opt := giteaUtil.OptionalArg(opts) r := &Extension{ - renderInternal: renderInternal, - enabled: true, - parseDollarBlock: true, - parseDollarInline: true, - } - - for _, o := range opts { - o.SetOption(r) + renderInternal: renderInternal, + options: opt, } return r } // Extend extends goldmark with our parsers and renderers func (e *Extension) Extend(m goldmark.Markdown) { - if !e.enabled { + if !e.options.Enabled { return } - m.Parser().AddOptions(parser.WithBlockParsers( - util.Prioritized(NewBlockParser(e.parseDollarBlock), 701), - )) - - inlines := []util.PrioritizedValue{ - util.Prioritized(NewInlineBracketParser(), 501), - } - if e.parseDollarInline { - inlines = append(inlines, util.Prioritized(NewInlineDollarParser(), 503), - util.Prioritized(NewInlineDualDollarParser(), 502)) + inlines := []util.PrioritizedValue{util.Prioritized(NewInlineBracketParser(), 501)} + if e.options.ParseDollarInline { + inlines = append(inlines, util.Prioritized(NewInlineDollarParser(), 502)) } m.Parser().AddOptions(parser.WithInlineParsers(inlines...)) + m.Parser().AddOptions(parser.WithBlockParsers( + util.Prioritized(NewBlockParser(e.options.ParseDollarBlock, e.options.ParseSquareBlock), 701), + )) + m.Renderer().AddOptions(renderer.WithNodeRenderers( util.Prioritized(NewBlockRenderer(e.renderInternal), 501), util.Prioritized(NewInlineRenderer(e.renderInternal), 502), diff --git a/web_src/css/repo/home-file-list.css b/web_src/css/repo/home-file-list.css index ecb26fa6629..285b823d57a 100644 --- a/web_src/css/repo/home-file-list.css +++ b/web_src/css/repo/home-file-list.css @@ -29,7 +29,7 @@ #repo-files-table .repo-file-line, #repo-files-table .repo-file-cell { border-top: 1px solid var(--color-light-border); - padding: 6px 10px; + padding: 8px 10px; } #repo-files-table .repo-file-line:first-child { @@ -41,7 +41,6 @@ display: flex; align-items: center; gap: 0.5em; - padding: 6px 10px; } #repo-files-table .repo-file-last-commit { diff --git a/web_src/js/markup/math.ts b/web_src/js/markup/math.ts index 22a4de38e98..4777805e3ca 100644 --- a/web_src/js/markup/math.ts +++ b/web_src/js/markup/math.ts @@ -1,8 +1,14 @@ import {displayError} from './common.ts'; -function targetElement(el: Element) { +function targetElement(el: Element): {target: Element, displayAsBlock: boolean} { // The target element is either the parent "code block with loading indicator", or itself - return el.closest('.code-block.is-loading') ?? el; + // It is designed to work for 2 cases (guaranteed by backend code): + // *
...
+ // * ... + return { + target: el.closest('.code-block.is-loading') ?? el, + displayAsBlock: el.classList.contains('display'), + }; } export async function renderMath(): Promise { @@ -19,7 +25,7 @@ export async function renderMath(): Promise { const MAX_EXPAND = 1000; for (const el of els) { - const target = targetElement(el); + const {target, displayAsBlock} = targetElement(el); if (target.hasAttribute('data-render-done')) continue; const source = el.textContent; @@ -27,16 +33,12 @@ export async function renderMath(): Promise { displayError(target, new Error(`Math source of ${source.length} characters exceeds the maximum allowed length of ${MAX_CHARS}.`)); continue; } - - const displayMode = el.classList.contains('display'); - const nodeName = displayMode ? 'p' : 'span'; - try { - const tempEl = document.createElement(nodeName); + const tempEl = document.createElement(displayAsBlock ? 'p' : 'span'); katex.render(source, tempEl, { maxSize: MAX_SIZE, maxExpand: MAX_EXPAND, - displayMode, + displayMode: displayAsBlock, // katex: true for display (block) mode, false for inline mode }); target.replaceWith(tempEl); } catch (error) { From 1a07ebe5492d0c07bfc474441502362f678dba5a Mon Sep 17 00:00:00 2001 From: silverwind Date: Sat, 14 Dec 2024 07:50:12 +0100 Subject: [PATCH 08/19] Fix overflow on org header (#32837) --- templates/org/header.tmpl | 4 ++-- web_src/css/org.css | 5 ----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/templates/org/header.tmpl b/templates/org/header.tmpl index 7361df99eaf..80519361fdb 100644 --- a/templates/org/header.tmpl +++ b/templates/org/header.tmpl @@ -1,6 +1,6 @@
{{ctx.AvatarUtils.Avatar .Org 100 "org-avatar"}} -
+
{{.Org.DisplayName}} @@ -18,7 +18,7 @@ {{end}}
- {{if .RenderedDescription}}
{{.RenderedDescription}}
{{end}} + {{if .RenderedDescription}}
{{.RenderedDescription}}
{{end}}
{{if .Org.Location}}
{{svg "octicon-location"}} {{.Org.Location}}
{{end}} {{if .Org.Website}}
{{svg "octicon-link"}} {{.Org.Website}}
{{end}} diff --git a/web_src/css/org.css b/web_src/css/org.css index 90e5d7ad0e8..10826250419 100644 --- a/web_src/css/org.css +++ b/web_src/css/org.css @@ -93,11 +93,6 @@ margin-right: 15px; } -.page-content.organization #org-info { - overflow-wrap: anywhere; - flex: 1; -} - .page-content.organization #org-info .ui.header { display: flex; align-items: center; From 32059158da93dc85eff4a0e9bacb474adf7a46a2 Mon Sep 17 00:00:00 2001 From: mevius4 Date: Sun, 15 Dec 2024 10:41:36 +0900 Subject: [PATCH 09/19] Fix SSPI button visibility when SSPI is the only enabled method (#32841) --- templates/user/auth/signin_inner.tmpl | 2 +- templates/user/auth/signup_inner.tmpl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/user/auth/signin_inner.tmpl b/templates/user/auth/signin_inner.tmpl index 3124048d36e..9daa051e061 100644 --- a/templates/user/auth/signin_inner.tmpl +++ b/templates/user/auth/signin_inner.tmpl @@ -48,7 +48,7 @@
{{end}}{{/*if .EnablePasswordSignInForm*/}} - {{$showOAuth2Methods := or .OAuth2Providers .EnableOpenIDSignIn}} + {{$showOAuth2Methods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}} {{if and $showOAuth2Methods .EnablePasswordSignInForm}}
{{ctx.Locale.Tr "sign_in_or"}}
{{end}} diff --git a/templates/user/auth/signup_inner.tmpl b/templates/user/auth/signup_inner.tmpl index 41d0cd49b52..ea8d0bafe40 100644 --- a/templates/user/auth/signup_inner.tmpl +++ b/templates/user/auth/signup_inner.tmpl @@ -47,7 +47,7 @@
{{end}} - {{$showOAuth2Methods := or .OAuth2Providers .EnableOpenIDSignIn}} + {{$showOAuth2Methods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}} {{if $showOAuth2Methods}}
{{ctx.Locale.Tr "sign_in_or"}}
{{template "user/auth/oauth_container" .}} From 7616aeb2ea2a02c15480dcd4a232e98081569690 Mon Sep 17 00:00:00 2001 From: hiifong Date: Sun, 15 Dec 2024 10:06:21 +0800 Subject: [PATCH 10/19] In some lfs server implementations, they require the ref attribute. (#32838) Fix: #32611 In some lfs server implementations, they require the ref attribute. --------- Co-authored-by: wxiaoguang --- modules/lfs/http_client.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/lfs/http_client.go b/modules/lfs/http_client.go index 50f0e7a8d88..3acd23b8f73 100644 --- a/modules/lfs/http_client.go +++ b/modules/lfs/http_client.go @@ -72,7 +72,10 @@ func (c *HTTPClient) batch(ctx context.Context, operation string, objects []Poin url := fmt.Sprintf("%s/objects/batch", c.endpoint) - request := &BatchRequest{operation, c.transferNames(), nil, objects} + // `ref` is an "optional object describing the server ref that the objects belong to" + // but some (incorrect) lfs servers require it, so maybe adding an empty ref here doesn't break the correct ones. + // https://github.com/git-lfs/git-lfs/blob/a32a02b44bf8a511aa14f047627c49e1a7fd5021/docs/api/batch.md?plain=1#L37 + request := &BatchRequest{operation, c.transferNames(), &Reference{}, objects} payload := new(bytes.Buffer) err := json.NewEncoder(payload).Encode(request) if err != nil { From 1cfb718976e2db517da0e76bda835edd9df1fabd Mon Sep 17 00:00:00 2001 From: silverwind Date: Sun, 15 Dec 2024 03:31:07 +0100 Subject: [PATCH 11/19] Update golangci-lint to v1.62.2, fix issues (#32845) Update it and fix new issues related to `redefines-builtin-id` --- Makefile | 2 +- models/issues/issue_index.go | 6 +++--- modules/auth/password/password.go | 4 ++-- modules/git/repo_commit.go | 10 +++++----- modules/graceful/manager.go | 6 +++--- modules/indexer/internal/bleve/query.go | 10 +++++----- modules/indexer/internal/paginator.go | 6 +++--- modules/log/event_format.go | 4 ++-- modules/references/references.go | 6 +++--- modules/templates/util_date.go | 6 +++--- modules/templates/util_string.go | 4 ++-- routers/api/v1/repo/issue_dependency.go | 4 ++-- routers/api/v1/repo/wiki.go | 4 ++-- 13 files changed, 36 insertions(+), 36 deletions(-) diff --git a/Makefile b/Makefile index 0cd6936b3bb..d5b779f1e59 100644 --- a/Makefile +++ b/Makefile @@ -28,7 +28,7 @@ XGO_VERSION := go-1.23.x AIR_PACKAGE ?= github.com/air-verse/air@v1 EDITORCONFIG_CHECKER_PACKAGE ?= github.com/editorconfig-checker/editorconfig-checker/cmd/editorconfig-checker@2.7.0 GOFUMPT_PACKAGE ?= mvdan.cc/gofumpt@v0.7.0 -GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/cmd/golangci-lint@v1.60.3 +GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/cmd/golangci-lint@v1.62.2 GXZ_PACKAGE ?= github.com/ulikunitz/xz/cmd/gxz@v0.5.11 MISSPELL_PACKAGE ?= github.com/golangci/misspell/cmd/misspell@v0.5.1 SWAGGER_PACKAGE ?= github.com/go-swagger/go-swagger/cmd/swagger@v0.31.0 diff --git a/models/issues/issue_index.go b/models/issues/issue_index.go index 16274d0ef09..2eb61858bfc 100644 --- a/models/issues/issue_index.go +++ b/models/issues/issue_index.go @@ -18,12 +18,12 @@ func RecalculateIssueIndexForRepo(ctx context.Context, repoID int64) error { } defer committer.Close() - var max int64 - if _, err = db.GetEngine(ctx).Select(" MAX(`index`)").Table("issue").Where("repo_id=?", repoID).Get(&max); err != nil { + var maxIndex int64 + if _, err = db.GetEngine(ctx).Select(" MAX(`index`)").Table("issue").Where("repo_id=?", repoID).Get(&maxIndex); err != nil { return err } - if err = db.SyncMaxResourceIndex(ctx, "issue_index", repoID, max); err != nil { + if err = db.SyncMaxResourceIndex(ctx, "issue_index", repoID, maxIndex); err != nil { return err } diff --git a/modules/auth/password/password.go b/modules/auth/password/password.go index 85f9780709c..c66b62937fd 100644 --- a/modules/auth/password/password.go +++ b/modules/auth/password/password.go @@ -99,10 +99,10 @@ func IsComplexEnough(pwd string) bool { func Generate(n int) (string, error) { NewComplexity() buffer := make([]byte, n) - max := big.NewInt(int64(len(validChars))) + maxInt := big.NewInt(int64(len(validChars))) for { for j := 0; j < n; j++ { - rnd, err := rand.Int(rand.Reader, max) + rnd, err := rand.Int(rand.Reader, maxInt) if err != nil { return "", err } diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 9405634df12..9ffadb833d8 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -465,15 +465,15 @@ func (repo *Repository) getBranches(env []string, commitID string, limit int) ([ refs := strings.Split(stdout, "\n") - var max int + var maxNum int if len(refs) > limit { - max = limit + maxNum = limit } else { - max = len(refs) - 1 + maxNum = len(refs) - 1 } - branches := make([]string, max) - for i, ref := range refs[:max] { + branches := make([]string, maxNum) + for i, ref := range refs[:maxNum] { parts := strings.Fields(ref) branches[i] = parts[len(parts)-1] diff --git a/modules/graceful/manager.go b/modules/graceful/manager.go index 3f1115066a0..991b2f2b7af 100644 --- a/modules/graceful/manager.go +++ b/modules/graceful/manager.go @@ -218,13 +218,13 @@ func (g *Manager) ServerDone() { g.runningServerWaitGroup.Done() } -func (g *Manager) setStateTransition(old, new state) bool { +func (g *Manager) setStateTransition(oldState, newState state) bool { g.lock.Lock() - if g.state != old { + if g.state != oldState { g.lock.Unlock() return false } - g.state = new + g.state = newState g.lock.Unlock() return true } diff --git a/modules/indexer/internal/bleve/query.go b/modules/indexer/internal/bleve/query.go index 21422b281c4..1b18ca1a779 100644 --- a/modules/indexer/internal/bleve/query.go +++ b/modules/indexer/internal/bleve/query.go @@ -35,18 +35,18 @@ func BoolFieldQuery(value bool, field string) *query.BoolFieldQuery { return q } -func NumericRangeInclusiveQuery(min, max optional.Option[int64], field string) *query.NumericRangeQuery { +func NumericRangeInclusiveQuery(minOption, maxOption optional.Option[int64], field string) *query.NumericRangeQuery { var minF, maxF *float64 var minI, maxI *bool - if min.Has() { + if minOption.Has() { minF = new(float64) - *minF = float64(min.Value()) + *minF = float64(minOption.Value()) minI = new(bool) *minI = true } - if max.Has() { + if maxOption.Has() { maxF = new(float64) - *maxF = float64(max.Value()) + *maxF = float64(maxOption.Value()) maxI = new(bool) *maxI = true } diff --git a/modules/indexer/internal/paginator.go b/modules/indexer/internal/paginator.go index ee204bf0471..f1e19740eb7 100644 --- a/modules/indexer/internal/paginator.go +++ b/modules/indexer/internal/paginator.go @@ -10,12 +10,12 @@ import ( ) // ParsePaginator parses a db.Paginator into a skip and limit -func ParsePaginator(paginator *db.ListOptions, max ...int) (int, int) { +func ParsePaginator(paginator *db.ListOptions, maxNums ...int) (int, int) { // Use a very large number to indicate no limit unlimited := math.MaxInt32 - if len(max) > 0 { + if len(maxNums) > 0 { // Some indexer engines have a limit on the page size, respect that - unlimited = max[0] + unlimited = maxNums[0] } if paginator == nil || paginator.IsListAll() { diff --git a/modules/log/event_format.go b/modules/log/event_format.go index d9dbebf8315..0b8d1cec791 100644 --- a/modules/log/event_format.go +++ b/modules/log/event_format.go @@ -110,10 +110,10 @@ func EventFormatTextMessage(mode *WriterMode, event *Event, msgFormat string, ms buf = append(buf, ' ') } if flags&(Ltime|Lmicroseconds) != 0 { - hour, min, sec := t.Clock() + hour, minNum, sec := t.Clock() buf = itoa(buf, hour, 2) buf = append(buf, ':') - buf = itoa(buf, min, 2) + buf = itoa(buf, minNum, 2) buf = append(buf, ':') buf = itoa(buf, sec, 2) if flags&Lmicroseconds != 0 { diff --git a/modules/references/references.go b/modules/references/references.go index 2889430bcf6..6e549cb8758 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -164,9 +164,9 @@ func newKeywords() { }) } -func doNewKeywords(close, reopen []string) { - issueCloseKeywordsPat = makeKeywordsPat(close) - issueReopenKeywordsPat = makeKeywordsPat(reopen) +func doNewKeywords(closeKeywords, reopenKeywords []string) { + issueCloseKeywordsPat = makeKeywordsPat(closeKeywords) + issueReopenKeywordsPat = makeKeywordsPat(reopenKeywords) } // getGiteaHostName returns a normalized string with the local host name, with no scheme or port information diff --git a/modules/templates/util_date.go b/modules/templates/util_date.go index 66f83d23fe5..658691ee40c 100644 --- a/modules/templates/util_date.go +++ b/modules/templates/util_date.go @@ -53,8 +53,8 @@ func parseLegacy(datetime string) time.Time { return t } -func anyToTime(any any) (t time.Time, isZero bool) { - switch v := any.(type) { +func anyToTime(value any) (t time.Time, isZero bool) { + switch v := value.(type) { case nil: // it is zero case *time.Time: @@ -72,7 +72,7 @@ func anyToTime(any any) (t time.Time, isZero bool) { case int64: t = timeutil.TimeStamp(v).AsTime() default: - panic(fmt.Sprintf("Unsupported time type %T", any)) + panic(fmt.Sprintf("Unsupported time type %T", value)) } return t, t.IsZero() || t.Unix() == 0 } diff --git a/modules/templates/util_string.go b/modules/templates/util_string.go index 479b755da1c..2ae27d08336 100644 --- a/modules/templates/util_string.go +++ b/modules/templates/util_string.go @@ -53,8 +53,8 @@ func (su *StringUtils) Cut(s, sep string) []any { return []any{before, after, found} } -func (su *StringUtils) EllipsisString(s string, max int) string { - return base.EllipsisString(s, max) +func (su *StringUtils) EllipsisString(s string, maxLength int) string { + return base.EllipsisString(s, maxLength) } func (su *StringUtils) ToUpper(s string) string { diff --git a/routers/api/v1/repo/issue_dependency.go b/routers/api/v1/repo/issue_dependency.go index 712c71a6823..ae7502c661b 100644 --- a/routers/api/v1/repo/issue_dependency.go +++ b/routers/api/v1/repo/issue_dependency.go @@ -338,7 +338,7 @@ func GetIssueBlocks(ctx *context.APIContext) { } skip := (page - 1) * limit - max := page * limit + maxNum := page * limit deps, err := issue.BlockingDependencies(ctx) if err != nil { @@ -352,7 +352,7 @@ func GetIssueBlocks(ctx *context.APIContext) { repoPerms[ctx.Repo.Repository.ID] = ctx.Repo.Permission for i, depMeta := range deps { - if i < skip || i >= max { + if i < skip || i >= maxNum { continue } diff --git a/routers/api/v1/repo/wiki.go b/routers/api/v1/repo/wiki.go index c7065c1d9df..f9906ed250d 100644 --- a/routers/api/v1/repo/wiki.go +++ b/routers/api/v1/repo/wiki.go @@ -308,7 +308,7 @@ func ListWikiPages(ctx *context.APIContext) { } skip := (page - 1) * limit - max := page * limit + maxNum := page * limit entries, err := commit.ListEntries() if err != nil { @@ -317,7 +317,7 @@ func ListWikiPages(ctx *context.APIContext) { } pages := make([]*api.WikiPageMetaData, 0, len(entries)) for i, entry := range entries { - if i < skip || i >= max || !entry.IsRegular() { + if i < skip || i >= maxNum || !entry.IsRegular() { continue } c, err := wikiRepo.GetCommitByPath(entry.Name()) From d1c1e3cbccf88a9180e9a130a81342e0acb87fde Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 15 Dec 2024 14:07:50 +0800 Subject: [PATCH 12/19] Fine tune ssh related comments and code (#32846) Add more comments to explain the ssh problem, and rename `sshConn` to `sshSession` --- modules/ssh/ssh.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/ssh/ssh.go b/modules/ssh/ssh.go index 6d0695ee163..7479cfbd95a 100644 --- a/modules/ssh/ssh.go +++ b/modules/ssh/ssh.go @@ -13,7 +13,6 @@ import ( "errors" "fmt" "io" - "maps" "net" "os" "os/exec" @@ -49,6 +48,10 @@ import ( // Then sessionHandler should only use the "verified keyID" from the original ssh conn, but not the ctx one. // Otherwise, if a user provides 2 keys A (a correct one) and B (public key matches but no private key), // then only A succeeds to authenticate, sessionHandler will see B's keyID +// +// After x/crypto >= 0.31.0 (fix CVE-2024-45337), the PublicKeyCallback will be called again for the verified key, +// it mitigates the misuse for most cases, it's still good for us to make sure we don't rely on that mitigation +// and do not misuse the PublicKeyCallback: we should only use the verified keyID from the verified ssh conn. const giteaPermissionExtensionKeyID = "gitea-perm-ext-key-id" @@ -100,8 +103,8 @@ func ptr[T any](intf any) *T { func sessionHandler(session ssh.Session) { // here can't use session.Permissions() because it only uses the value from ctx, which might not be the authenticated one. // so we must use the original ssh conn, which always contains the correct (verified) keyID. - sshConn := ptr[sessionPartial](session) - keyID := sshConn.conn.Permissions.Extensions[giteaPermissionExtensionKeyID] + sshSession := ptr[sessionPartial](session) + keyID := sshSession.conn.Permissions.Extensions[giteaPermissionExtensionKeyID] command := session.RawCommand() @@ -210,10 +213,7 @@ func publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool { // first, reset the ctx permissions (just like https://github.com/gliderlabs/ssh/pull/243 does) // it shouldn't be reused across different ssh conn (sessions), each pub key should have its own "Permissions" - oldCtxPerm := ctx.Permissions().Permissions ctx.Permissions().Permissions = &gossh.Permissions{} - ctx.Permissions().Permissions.CriticalOptions = maps.Clone(oldCtxPerm.CriticalOptions) - setPermExt := func(keyID int64) { ctx.Permissions().Permissions.Extensions = map[string]string{ giteaPermissionExtensionKeyID: fmt.Sprint(keyID), From 92648112175753c4eaa13cc6f8e3cc6a3cef9ad5 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 14 Dec 2024 23:44:13 -0800 Subject: [PATCH 13/19] Remove translation to issue add time because the format is fixed should not be translated (#32850) The input content should always be `1h 2m 3s` and will be the same on different UI languages. So the translation is wrong. --- options/locale/locale_en-US.ini | 1 - templates/repo/issue/sidebar/stopwatch_timetracker.tmpl | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index f50ad1f2981..74ba70b8c8f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1680,7 +1680,6 @@ issues.timetracker_timer_stop = Stop timer issues.timetracker_timer_discard = Discard timer issues.timetracker_timer_manually_add = Add Time -issues.time_estimate_placeholder = 1h 2m issues.time_estimate_set = Set estimated time issues.time_estimate_display = Estimate: %s issues.change_time_estimate_at = changed time estimate to %s %s diff --git a/templates/repo/issue/sidebar/stopwatch_timetracker.tmpl b/templates/repo/issue/sidebar/stopwatch_timetracker.tmpl index 1acf56d7b20..f107dc5ef5a 100644 --- a/templates/repo/issue/sidebar/stopwatch_timetracker.tmpl +++ b/templates/repo/issue/sidebar/stopwatch_timetracker.tmpl @@ -44,7 +44,7 @@
{{$.CsrfTokenHtml}} - +
From df9a78cd04e364264c103cf3a92d94179cc1dd4f Mon Sep 17 00:00:00 2001 From: silverwind Date: Sun, 15 Dec 2024 11:01:46 +0100 Subject: [PATCH 14/19] Tweak repo sidebar (#32847) Before and after: Screenshot 2024-12-15 at 04 53 53 Screenshot 2024-12-15 at 04 53 41 Diff without whitespace: https://github.com/go-gitea/gitea/pull/32847/files?diff=unified&w=1 The `tw-mt-2` is fine even if the element renders empty: image --------- Co-authored-by: wxiaoguang --- options/locale/locale_en-US.ini | 1 + templates/repo/home.tmpl | 6 +- templates/repo/home_sidebar_bottom.tmpl | 94 +++++++++--------- templates/repo/home_sidebar_top.tmpl | 124 ++++++++++++------------ templates/repo/release/label.tmpl | 14 +++ templates/repo/release/list.tmpl | 8 +- web_src/css/repo.css | 5 - web_src/css/repo/home.css | 14 +-- web_src/js/features/repo-home.ts | 6 +- 9 files changed, 141 insertions(+), 131 deletions(-) create mode 100644 templates/repo/release/label.tmpl diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 74ba70b8c8f..92ce4f2db9f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2632,6 +2632,7 @@ release.new_release = New Release release.draft = Draft release.prerelease = Pre-Release release.stable = Stable +release.latest = Latest release.compare = Compare release.edit = edit release.ahead.commits = %d commits diff --git a/templates/repo/home.tmpl b/templates/repo/home.tmpl index 4e6d375b517..d73b7470bc5 100644 --- a/templates/repo/home.tmpl +++ b/templates/repo/home.tmpl @@ -18,7 +18,7 @@ {{$treeNamesLen := len .TreeNames}} {{$isTreePathRoot := eq $treeNamesLen 0}} - {{$showSidebar := $isTreePathRoot}} + {{$showSidebar := and $isTreePathRoot (not .HideRepoInfo) (not .IsBlame)}}
{{template "repo/sub_menu" .}} @@ -130,8 +130,8 @@
{{if $showSidebar}} -
{{template "repo/home_sidebar_top" .}}
-
{{template "repo/home_sidebar_bottom" .}}
+ {{template "repo/home_sidebar_top" .}} + {{template "repo/home_sidebar_bottom" .}} {{end}}
diff --git a/templates/repo/home_sidebar_bottom.tmpl b/templates/repo/home_sidebar_bottom.tmpl index 57b4a95ddcc..f780dc122d5 100644 --- a/templates/repo/home_sidebar_bottom.tmpl +++ b/templates/repo/home_sidebar_bottom.tmpl @@ -1,59 +1,61 @@ -
- {{if .LatestRelease}} -
-
- -
-
- {{svg "octicon-tag" 16}} +
+
+ {{if .LatestRelease}} +
+
+ -
-
-
- {{.LatestRelease.Title}} - {{ctx.Locale.Tr "latest"}} -
+
+
+ {{svg "octicon-tag" 16}}
-
- {{DateUtils.TimeSince .LatestRelease.CreatedUnix}} +
+
+
+ {{.LatestRelease.Title}} + {{template "repo/release/label" (dict "Release" .LatestRelease "IsLatest" true)}} +
+
+
+ {{DateUtils.TimeSince .LatestRelease.CreatedUnix}} +
-
- {{end}} + {{end}} - {{if and (not .IsEmptyRepo) .LanguageStats}} -
-
-
- {{ctx.Locale.Tr "repo.repo_lang"}} -
- -
-
- {{range .LanguageStats}} -
- {{end}} + {{if and (not .IsEmptyRepo) .LanguageStats}} +
+
+
+ {{ctx.Locale.Tr "repo.repo_lang"}}
-
- {{range .LanguageStats}} -
- - - {{Iif (eq .Language "other") (ctx.Locale.Tr "repo.language_other") .Language}} - - {{.Percentage}}% -
- {{end}} + +
+
+ {{range .LanguageStats}} +
+ {{end}} +
+
+ {{range .LanguageStats}} +
+ + + {{Iif (eq .Language "other") (ctx.Locale.Tr "repo.language_other") .Language}} + + {{.Percentage}}% +
+ {{end}} +
+ {{end}}
- {{end}}
diff --git a/templates/repo/home_sidebar_top.tmpl b/templates/repo/home_sidebar_top.tmpl index 4b0ebcd3906..607dc62e2e7 100644 --- a/templates/repo/home_sidebar_top.tmpl +++ b/templates/repo/home_sidebar_top.tmpl @@ -1,68 +1,70 @@ - -
- - {{template "shared/search/button"}} -
- +
+
+
+ {{template "shared/search/button"}} +
+
-
-
-
-
- {{ctx.Locale.Tr "repo.repo_desc"}} -
- {{if and (not .HideRepoInfo) (not .IsBlame)}} -
- {{- $description := .Repository.DescriptionHTML ctx -}} - {{if $description}}{{$description | RenderCodeBlock}}{{else}}{{ctx.Locale.Tr "repo.repo_no_desc"}}{{end}} - {{if .Repository.Website}}{{svg "octicon-link"}}{{.Repository.Website}}{{end}} -
-
- {{/* !!!! it SHOULD and MUST match the code in issue-home.js */}} - {{range .Topics}}{{.Name}}{{end}} -
- {{if and .Permission.IsAdmin (not .Repository.IsArchived)}} - - {{end}} - {{end}} - {{if and .Permission.IsAdmin (not .Repository.IsArchived)}} -
- diff --git a/templates/repo/release/label.tmpl b/templates/repo/release/label.tmpl new file mode 100644 index 00000000000..eacb3e36f4c --- /dev/null +++ b/templates/repo/release/label.tmpl @@ -0,0 +1,14 @@ +{{/* +Template Attributes: +* Release: the release +* IsLatest: boolean indicating whether this is the latest release, optional +*/}} +{{if .IsLatest}} + {{ctx.Locale.Tr "repo.release.latest"}} +{{else if .Release.IsDraft}} + {{ctx.Locale.Tr "repo.release.draft"}} +{{else if .Release.IsPrerelease}} + {{ctx.Locale.Tr "repo.release.prerelease"}} +{{else if (not .Release.IsTag)}} + {{ctx.Locale.Tr "repo.release.stable"}} +{{end}} diff --git a/templates/repo/release/list.tmpl b/templates/repo/release/list.tmpl index efaac4432a6..99934d21182 100644 --- a/templates/repo/release/list.tmpl +++ b/templates/repo/release/list.tmpl @@ -33,13 +33,7 @@

{{if $.PageIsSingleTag}}{{$release.Title}}{{else}}{{$release.Title}}{{end}} {{template "repo/commit_statuses" dict "Status" $info.CommitStatus "Statuses" $info.CommitStatuses "AdditionalClasses" "tw-flex"}} - {{if $release.IsDraft}} - {{ctx.Locale.Tr "repo.release.draft"}} - {{else if $release.IsPrerelease}} - {{ctx.Locale.Tr "repo.release.prerelease"}} - {{else if (not $release.IsTag)}} - {{ctx.Locale.Tr "repo.release.stable"}} - {{end}} + {{template "repo/release/label" (dict "Release" $release)}}

{{if and $.CanCreateRelease (not $.PageIsSingleTag)}} diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 9a43e10e826..6fdc9ec2a88 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -101,11 +101,6 @@ margin-bottom: 12px; } -.repository .repo-description { - font-size: 16px; - margin-bottom: 5px; -} - .commit-summary { flex: 1; overflow-wrap: anywhere; diff --git a/web_src/css/repo/home.css b/web_src/css/repo/home.css index ca5b432804f..65005e22635 100644 --- a/web_src/css/repo/home.css +++ b/web_src/css/repo/home.css @@ -4,22 +4,24 @@ grid-template-rows: auto auto 1fr; } -.repo-grid-filelist-sidebar .repo-home-filelist { +.repo-home-filelist { min-width: 0; grid-column: 1; grid-row: 1 / 4; } -.repo-grid-filelist-sidebar .repo-home-sidebar-top { +.repo-home-sidebar-top { grid-column: 2; grid-row: 1; padding-left: 1em; } -.repo-grid-filelist-sidebar .repo-home-sidebar-bottom { + +.repo-home-sidebar-bottom { grid-column: 2; grid-row: 2; padding-left: 1em; } + .repo-home-sidebar-bottom .flex-list > :first-child { border-top: 1px solid var(--color-secondary); /* same to .flex-list > .flex-item + .flex-item */ } @@ -29,16 +31,16 @@ grid-template-columns: 100%; grid-template-rows: auto auto auto; } - .repo-grid-filelist-sidebar .repo-home-filelist { + .repo-home-filelist { grid-column: 1; grid-row: 2; } - .repo-grid-filelist-sidebar .repo-home-sidebar-top { + .repo-home-sidebar-top { grid-column: 1; grid-row: 1; padding-left: 0; } - .repo-grid-filelist-sidebar .repo-home-sidebar-bottom { + .repo-home-sidebar-bottom { grid-column: 1; grid-row: 3; padding-left: 0; diff --git a/web_src/js/features/repo-home.ts b/web_src/js/features/repo-home.ts index 4c69a004344..abda29cc52e 100644 --- a/web_src/js/features/repo-home.ts +++ b/web_src/js/features/repo-home.ts @@ -16,7 +16,7 @@ export function initRepoTopicBar() { let lastErrorToast: Toast; mgrBtn.addEventListener('click', () => { - hideElem(viewDiv); + hideElem([viewDiv, mgrBtn]); showElem(editDiv); topicDropdown.querySelector('input.search').focus(); }); @@ -24,7 +24,7 @@ export function initRepoTopicBar() { document.querySelector('#cancel_topic_edit').addEventListener('click', () => { lastErrorToast?.hideToast(); hideElem(editDiv); - showElem(viewDiv); + showElem([viewDiv, mgrBtn]); mgrBtn.focus(); }); @@ -55,7 +55,7 @@ export function initRepoTopicBar() { } } hideElem(editDiv); - showElem(viewDiv); + showElem([viewDiv, mgrBtn]); } } else if (response.status === 422) { // how to test: input topic like " invalid topic " (with spaces), and select it from the list, then "Save" From 33e8e82c4b528db8efb1cf9fc4dbab2d9e21ace8 Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Sun, 15 Dec 2024 11:41:29 +0100 Subject: [PATCH 15/19] Enable tenv and testifylint rules (#32852) Enables tenv and testifylint linters closes: https://github.com/go-gitea/gitea/issues/32842 --- .golangci.yml | 6 +++ models/actions/runner_token_test.go | 6 +-- models/activities/user_heatmap_test.go | 5 +-- models/auth/oauth2_test.go | 4 +- models/db/iterate_test.go | 2 - models/git/commit_status_test.go | 4 +- models/git/protected_branch_test.go | 3 +- models/issues/comment_test.go | 2 +- models/issues/issue_test.go | 2 +- models/issues/issue_watch_test.go | 6 +-- models/issues/label_test.go | 8 ++-- models/issues/milestone_test.go | 2 +- models/issues/pull_list_test.go | 2 +- models/issues/pull_test.go | 2 +- models/issues/stopwatch_test.go | 2 +- models/issues/tracked_time_test.go | 8 ++-- models/migrations/v1_16/v193_test.go | 8 ++-- models/migrations/v1_22/v286_test.go | 4 +- models/migrations/v1_22/v294_test.go | 2 +- models/organization/org_list_test.go | 2 +- models/organization/org_test.go | 2 +- models/perm/access_mode_test.go | 2 +- models/project/column_test.go | 5 +-- models/repo/repo_test.go | 12 +++--- models/repo/star_test.go | 4 +- models/repo/user_repo_test.go | 2 +- models/repo/watch_test.go | 4 +- models/unittest/unit_tests.go | 2 +- models/user/email_address_test.go | 3 +- models/user/setting_test.go | 2 +- models/user/user_test.go | 18 ++++----- models/webhook/webhook_test.go | 4 +- modules/activitypub/client_test.go | 5 +-- modules/assetfs/layered_test.go | 2 +- modules/auth/pam/pam_test.go | 2 +- modules/auth/password/hash/dummy_test.go | 2 +- modules/base/tool_test.go | 7 ++-- modules/dump/dumper_test.go | 2 +- modules/git/commit_sha256_test.go | 2 +- modules/git/commit_test.go | 6 +-- modules/git/grep_test.go | 4 +- modules/git/parse_nogogit_test.go | 2 +- modules/git/repo_branch_test.go | 4 +- modules/git/repo_compare_test.go | 4 +- .../indexer/issues/internal/tests/tests.go | 38 +++++++++---------- modules/lfs/transferadapter_test.go | 6 +-- modules/log/logger_test.go | 4 +- modules/markup/html_internal_test.go | 4 +- .../packages/conan/conanfile_parser_test.go | 2 +- .../packages/conan/conaninfo_parser_test.go | 2 +- modules/queue/base_test.go | 6 +-- modules/queue/workerqueue_test.go | 4 +- modules/references/references_test.go | 2 +- modules/repository/repo_test.go | 6 +-- modules/setting/cron_test.go | 2 +- modules/setting/oauth2_test.go | 2 +- modules/setting/storage_test.go | 10 ++--- modules/user/user_test.go | 3 +- modules/util/color_test.go | 6 +-- modules/util/keypair_test.go | 5 +-- modules/util/time_str_test.go | 4 +- modules/util/util_test.go | 16 ++++---- routers/private/hook_post_receive_test.go | 2 +- routers/web/repo/wiki_test.go | 4 +- services/actions/auth_test.go | 12 +++--- services/auth/oauth2_test.go | 2 +- .../auth/source/oauth2/source_sync_test.go | 8 ++-- services/convert/pull_review_test.go | 2 +- services/cron/tasks_test.go | 2 +- services/feed/feed_test.go | 6 +-- services/gitdiff/gitdiff_test.go | 5 +-- services/migrations/gitlab_test.go | 2 +- services/org/team_test.go | 2 +- services/pull/reviewer_test.go | 4 +- services/release/release_test.go | 2 +- services/repository/archiver/archiver_test.go | 5 +-- services/repository/license_test.go | 2 +- services/repository/transfer_test.go | 2 +- services/webhook/packagist_test.go | 30 +++++++-------- services/webhook/webhook_test.go | 4 +- .../integration/api_actions_artifact_test.go | 4 +- tests/integration/api_branch_test.go | 4 +- tests/integration/api_issue_config_test.go | 6 +-- tests/integration/api_issue_pin_test.go | 4 +- tests/integration/api_issue_stopwatch_test.go | 2 +- tests/integration/api_issue_test.go | 2 +- tests/integration/api_keys_test.go | 6 +-- tests/integration/api_notification_test.go | 4 +- tests/integration/api_oauth2_apps_test.go | 14 +++---- tests/integration/api_packages_npm_test.go | 2 +- tests/integration/api_pull_test.go | 6 +-- .../integration/api_repo_git_commits_test.go | 10 ++--- tests/integration/api_repo_lfs_locks_test.go | 2 +- tests/integration/api_repo_teams_test.go | 2 +- tests/integration/api_user_orgs_test.go | 2 +- tests/integration/api_user_search_test.go | 8 ++-- tests/integration/auth_ldap_test.go | 2 +- .../git_helper_for_declarative_test.go | 6 +-- tests/integration/git_push_test.go | 3 +- tests/integration/gpg_git_test.go | 5 +-- tests/integration/integration_test.go | 10 ++--- .../migration-test/migration_test.go | 2 +- tests/integration/mirror_push_test.go | 2 +- tests/integration/oauth_test.go | 22 +++++------ tests/integration/org_count_test.go | 2 +- tests/integration/pull_compare_test.go | 6 +-- tests/integration/pull_merge_test.go | 2 +- tests/integration/repo_archive_test.go | 2 +- tests/integration/repo_fork_test.go | 2 +- tests/integration/repo_generate_test.go | 2 +- tests/integration/repo_test.go | 2 +- tests/integration/session_test.go | 4 +- tests/integration/user_test.go | 2 +- 113 files changed, 272 insertions(+), 282 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 37617ad3652..c39d7ac5f2f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -19,6 +19,8 @@ linters: - revive - staticcheck - stylecheck + - tenv + - testifylint - typecheck - unconvert - unused @@ -34,6 +36,10 @@ output: show-stats: true linters-settings: + testifylint: + disable: + - go-require + - require-error stylecheck: checks: ["all", "-ST1005", "-ST1003"] nakedret: diff --git a/models/actions/runner_token_test.go b/models/actions/runner_token_test.go index e85e99abe53..159805e5f7c 100644 --- a/models/actions/runner_token_test.go +++ b/models/actions/runner_token_test.go @@ -17,7 +17,7 @@ func TestGetLatestRunnerToken(t *testing.T) { token := unittest.AssertExistsAndLoadBean(t, &ActionRunnerToken{ID: 3}) expectedToken, err := GetLatestRunnerToken(db.DefaultContext, 1, 0) assert.NoError(t, err) - assert.EqualValues(t, token, expectedToken) + assert.EqualValues(t, expectedToken, token) } func TestNewRunnerToken(t *testing.T) { @@ -26,7 +26,7 @@ func TestNewRunnerToken(t *testing.T) { assert.NoError(t, err) expectedToken, err := GetLatestRunnerToken(db.DefaultContext, 1, 0) assert.NoError(t, err) - assert.EqualValues(t, token, expectedToken) + assert.EqualValues(t, expectedToken, token) } func TestUpdateRunnerToken(t *testing.T) { @@ -36,5 +36,5 @@ func TestUpdateRunnerToken(t *testing.T) { assert.NoError(t, UpdateRunnerToken(db.DefaultContext, token)) expectedToken, err := GetLatestRunnerToken(db.DefaultContext, 1, 0) assert.NoError(t, err) - assert.EqualValues(t, token, expectedToken) + assert.EqualValues(t, expectedToken, token) } diff --git a/models/activities/user_heatmap_test.go b/models/activities/user_heatmap_test.go index b7babcbde1d..a039fd36132 100644 --- a/models/activities/user_heatmap_test.go +++ b/models/activities/user_heatmap_test.go @@ -4,7 +4,6 @@ package activities_test import ( - "fmt" "testing" "time" @@ -91,11 +90,11 @@ func TestGetUserHeatmapDataByUser(t *testing.T) { assert.NoError(t, err) assert.Len(t, actions, contributions, "invalid action count: did the test data became too old?") assert.Equal(t, count, int64(contributions)) - assert.Equal(t, tc.CountResult, contributions, fmt.Sprintf("testcase '%s'", tc.desc)) + assert.Equal(t, tc.CountResult, contributions, "testcase '%s'", tc.desc) // Test JSON rendering jsonData, err := json.Marshal(heatmap) assert.NoError(t, err) - assert.Equal(t, tc.JSONResult, string(jsonData)) + assert.JSONEq(t, tc.JSONResult, string(jsonData)) } } diff --git a/models/auth/oauth2_test.go b/models/auth/oauth2_test.go index 0829d31d51b..43daa0b5ec1 100644 --- a/models/auth/oauth2_test.go +++ b/models/auth/oauth2_test.go @@ -18,7 +18,7 @@ func TestOAuth2Application_GenerateClientSecret(t *testing.T) { app := unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Application{ID: 1}) secret, err := app.GenerateClientSecret(db.DefaultContext) assert.NoError(t, err) - assert.True(t, len(secret) > 0) + assert.NotEmpty(t, secret) unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Application{ID: 1, ClientSecret: app.ClientSecret}) } @@ -165,7 +165,7 @@ func TestOAuth2Grant_GenerateNewAuthorizationCode(t *testing.T) { code, err := grant.GenerateNewAuthorizationCode(db.DefaultContext, "https://example2.com/callback", "CjvyTLSdR47G5zYenDA-eDWW4lRrO8yvjcWwbD_deOg", "S256") assert.NoError(t, err) assert.NotNil(t, code) - assert.True(t, len(code.Code) > 32) // secret length > 32 + assert.Greater(t, len(code.Code), 32) // secret length > 32 } func TestOAuth2Grant_TableName(t *testing.T) { diff --git a/models/db/iterate_test.go b/models/db/iterate_test.go index 0f6ba2cc94a..e9f27906711 100644 --- a/models/db/iterate_test.go +++ b/models/db/iterate_test.go @@ -38,8 +38,6 @@ func TestIterate(t *testing.T) { if !has { return db.ErrNotExist{Resource: "repo_unit", ID: repoUnit.ID} } - assert.EqualValues(t, repoUnit.RepoID, repoUnit.RepoID) - assert.EqualValues(t, repoUnit.CreatedUnix, repoUnit.CreatedUnix) return nil }) assert.NoError(t, err) diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index 7ac4da68103..37d785e9385 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -34,7 +34,7 @@ func TestGetCommitStatuses(t *testing.T) { SHA: sha1, }) assert.NoError(t, err) - assert.Equal(t, int(maxResults), 5) + assert.Equal(t, 5, int(maxResults)) assert.Len(t, statuses, 5) assert.Equal(t, "ci/awesomeness", statuses[0].Context) @@ -63,7 +63,7 @@ func TestGetCommitStatuses(t *testing.T) { SHA: sha1, }) assert.NoError(t, err) - assert.Equal(t, int(maxResults), 5) + assert.Equal(t, 5, int(maxResults)) assert.Empty(t, statuses) } diff --git a/models/git/protected_branch_test.go b/models/git/protected_branch_test.go index 49d433f845d..e1c91d927d8 100644 --- a/models/git/protected_branch_test.go +++ b/models/git/protected_branch_test.go @@ -4,7 +4,6 @@ package git import ( - "fmt" "testing" "code.gitea.io/gitea/models/db" @@ -76,7 +75,7 @@ func TestBranchRuleMatch(t *testing.T) { infact = " not" } assert.EqualValues(t, kase.ExpectedMatch, pb.Match(kase.BranchName), - fmt.Sprintf("%s should%s match %s but it is%s", kase.BranchName, should, kase.Rule, infact), + "%s should%s match %s but it is%s", kase.BranchName, should, kase.Rule, infact, ) } } diff --git a/models/issues/comment_test.go b/models/issues/comment_test.go index c5bbfdedc28..d81f33f953d 100644 --- a/models/issues/comment_test.go +++ b/models/issues/comment_test.go @@ -64,7 +64,7 @@ func TestFetchCodeComments(t *testing.T) { } func TestAsCommentType(t *testing.T) { - assert.Equal(t, issues_model.CommentType(0), issues_model.CommentTypeComment) + assert.Equal(t, issues_model.CommentTypeComment, issues_model.CommentType(0)) assert.Equal(t, issues_model.CommentTypeUndefined, issues_model.AsCommentType("")) assert.Equal(t, issues_model.CommentTypeUndefined, issues_model.AsCommentType("nonsense")) assert.Equal(t, issues_model.CommentTypeComment, issues_model.AsCommentType("comment")) diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go index 548f137f394..dbbb1e41790 100644 --- a/models/issues/issue_test.go +++ b/models/issues/issue_test.go @@ -434,7 +434,7 @@ func assertCreateIssues(t *testing.T, isPull bool) { owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) label := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1}) milestone := unittest.AssertExistsAndLoadBean(t, &issues_model.Milestone{ID: 1}) - assert.EqualValues(t, milestone.ID, 1) + assert.EqualValues(t, 1, milestone.ID) reaction := &issues_model.Reaction{ Type: "heart", UserID: owner.ID, diff --git a/models/issues/issue_watch_test.go b/models/issues/issue_watch_test.go index d4ce8d8d3d6..fad94e243e6 100644 --- a/models/issues/issue_watch_test.go +++ b/models/issues/issue_watch_test.go @@ -48,17 +48,17 @@ func TestGetIssueWatchers(t *testing.T) { iws, err := issues_model.GetIssueWatchers(db.DefaultContext, 1, db.ListOptions{}) assert.NoError(t, err) // Watcher is inactive, thus 0 - assert.Len(t, iws, 0) + assert.Empty(t, iws) iws, err = issues_model.GetIssueWatchers(db.DefaultContext, 2, db.ListOptions{}) assert.NoError(t, err) // Watcher is explicit not watching - assert.Len(t, iws, 0) + assert.Empty(t, iws) iws, err = issues_model.GetIssueWatchers(db.DefaultContext, 5, db.ListOptions{}) assert.NoError(t, err) // Issue has no Watchers - assert.Len(t, iws, 0) + assert.Empty(t, iws) iws, err = issues_model.GetIssueWatchers(db.DefaultContext, 7, db.ListOptions{}) assert.NoError(t, err) diff --git a/models/issues/label_test.go b/models/issues/label_test.go index c2ff084c236..1d4b6f4684c 100644 --- a/models/issues/label_test.go +++ b/models/issues/label_test.go @@ -31,12 +31,12 @@ func TestLabel_LoadSelectedLabelsAfterClick(t *testing.T) { // First test : with negative and scope label.LoadSelectedLabelsAfterClick([]int64{1, -8}, []string{"", "scope"}) assert.Equal(t, "1", label.QueryString) - assert.Equal(t, true, label.IsSelected) + assert.True(t, label.IsSelected) // Second test : with duplicates label.LoadSelectedLabelsAfterClick([]int64{1, 7, 1, 7, 7}, []string{"", "scope", "", "scope", "scope"}) assert.Equal(t, "1,8", label.QueryString) - assert.Equal(t, false, label.IsSelected) + assert.False(t, label.IsSelected) // Third test : empty set label.LoadSelectedLabelsAfterClick([]int64{}, []string{}) @@ -248,7 +248,7 @@ func TestGetLabelsByIssueID(t *testing.T) { labels, err = issues_model.GetLabelsByIssueID(db.DefaultContext, unittest.NonexistentID) assert.NoError(t, err) - assert.Len(t, labels, 0) + assert.Empty(t, labels) } func TestUpdateLabel(t *testing.T) { @@ -271,7 +271,7 @@ func TestUpdateLabel(t *testing.T) { assert.EqualValues(t, label.Color, newLabel.Color) assert.EqualValues(t, label.Name, newLabel.Name) assert.EqualValues(t, label.Description, newLabel.Description) - assert.EqualValues(t, newLabel.ArchivedUnix, 0) + assert.EqualValues(t, 0, newLabel.ArchivedUnix) unittest.CheckConsistencyFor(t, &issues_model.Label{}, &repo_model.Repository{}) } diff --git a/models/issues/milestone_test.go b/models/issues/milestone_test.go index e5f6f15ca2a..28cd0c028b8 100644 --- a/models/issues/milestone_test.go +++ b/models/issues/milestone_test.go @@ -87,7 +87,7 @@ func TestGetMilestonesByRepoID(t *testing.T) { IsClosed: optional.Some(false), }) assert.NoError(t, err) - assert.Len(t, milestones, 0) + assert.Empty(t, milestones) } func TestGetMilestones(t *testing.T) { diff --git a/models/issues/pull_list_test.go b/models/issues/pull_list_test.go index 8b814a0d0fc..c7a898ca4e8 100644 --- a/models/issues/pull_list_test.go +++ b/models/issues/pull_list_test.go @@ -40,7 +40,7 @@ func TestPullRequestList_LoadReviewCommentsCounts(t *testing.T) { assert.NoError(t, err) assert.Len(t, reviewComments, 2) for _, pr := range prs { - assert.EqualValues(t, reviewComments[pr.IssueID], 1) + assert.EqualValues(t, 1, reviewComments[pr.IssueID]) } } diff --git a/models/issues/pull_test.go b/models/issues/pull_test.go index cb7b47263d8..090659864a8 100644 --- a/models/issues/pull_test.go +++ b/models/issues/pull_test.go @@ -83,7 +83,7 @@ func TestLoadRequestedReviewers(t *testing.T) { assert.NoError(t, pull.LoadIssue(db.DefaultContext)) issue := pull.Issue assert.NoError(t, issue.LoadRepo(db.DefaultContext)) - assert.Len(t, pull.RequestedReviewers, 0) + assert.Empty(t, pull.RequestedReviewers) user1, err := user_model.GetUserByID(db.DefaultContext, 1) assert.NoError(t, err) diff --git a/models/issues/stopwatch_test.go b/models/issues/stopwatch_test.go index 39958a7f36b..a1bf9dc931f 100644 --- a/models/issues/stopwatch_test.go +++ b/models/issues/stopwatch_test.go @@ -32,7 +32,7 @@ func TestCancelStopwatch(t *testing.T) { _ = unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{Type: issues_model.CommentTypeCancelTracking, PosterID: user1.ID, IssueID: issue1.ID}) - assert.Nil(t, issues_model.CancelStopwatch(db.DefaultContext, user1, issue2)) + assert.NoError(t, issues_model.CancelStopwatch(db.DefaultContext, user1, issue2)) } func TestStopwatchExists(t *testing.T) { diff --git a/models/issues/tracked_time_test.go b/models/issues/tracked_time_test.go index d82bff967a9..44054a1b836 100644 --- a/models/issues/tracked_time_test.go +++ b/models/issues/tracked_time_test.go @@ -50,7 +50,7 @@ func TestGetTrackedTimes(t *testing.T) { times, err = issues_model.GetTrackedTimes(db.DefaultContext, &issues_model.FindTrackedTimesOptions{IssueID: -1}) assert.NoError(t, err) - assert.Len(t, times, 0) + assert.Empty(t, times) // by User times, err = issues_model.GetTrackedTimes(db.DefaultContext, &issues_model.FindTrackedTimesOptions{UserID: 1}) @@ -60,7 +60,7 @@ func TestGetTrackedTimes(t *testing.T) { times, err = issues_model.GetTrackedTimes(db.DefaultContext, &issues_model.FindTrackedTimesOptions{UserID: 3}) assert.NoError(t, err) - assert.Len(t, times, 0) + assert.Empty(t, times) // by Repo times, err = issues_model.GetTrackedTimes(db.DefaultContext, &issues_model.FindTrackedTimesOptions{RepositoryID: 2}) @@ -69,7 +69,7 @@ func TestGetTrackedTimes(t *testing.T) { assert.Equal(t, int64(1), times[0].Time) issue, err := issues_model.GetIssueByID(db.DefaultContext, times[0].IssueID) assert.NoError(t, err) - assert.Equal(t, issue.RepoID, int64(2)) + assert.Equal(t, int64(2), issue.RepoID) times, err = issues_model.GetTrackedTimes(db.DefaultContext, &issues_model.FindTrackedTimesOptions{RepositoryID: 1}) assert.NoError(t, err) @@ -77,7 +77,7 @@ func TestGetTrackedTimes(t *testing.T) { times, err = issues_model.GetTrackedTimes(db.DefaultContext, &issues_model.FindTrackedTimesOptions{RepositoryID: 10}) assert.NoError(t, err) - assert.Len(t, times, 0) + assert.Empty(t, times) } func TestTotalTimesForEachUser(t *testing.T) { diff --git a/models/migrations/v1_16/v193_test.go b/models/migrations/v1_16/v193_test.go index d99bbc29620..b279967a2c0 100644 --- a/models/migrations/v1_16/v193_test.go +++ b/models/migrations/v1_16/v193_test.go @@ -56,8 +56,8 @@ func Test_AddRepoIDForAttachment(t *testing.T) { err := x.Table("attachment").Where("issue_id > 0").Find(&issueAttachments) assert.NoError(t, err) for _, attach := range issueAttachments { - assert.Greater(t, attach.RepoID, int64(0)) - assert.Greater(t, attach.IssueID, int64(0)) + assert.Positive(t, attach.RepoID) + assert.Positive(t, attach.IssueID) var issue Issue has, err := x.ID(attach.IssueID).Get(&issue) assert.NoError(t, err) @@ -69,8 +69,8 @@ func Test_AddRepoIDForAttachment(t *testing.T) { err = x.Table("attachment").Where("release_id > 0").Find(&releaseAttachments) assert.NoError(t, err) for _, attach := range releaseAttachments { - assert.Greater(t, attach.RepoID, int64(0)) - assert.Greater(t, attach.ReleaseID, int64(0)) + assert.Positive(t, attach.RepoID) + assert.Positive(t, attach.ReleaseID) var release Release has, err := x.ID(attach.ReleaseID).Get(&release) assert.NoError(t, err) diff --git a/models/migrations/v1_22/v286_test.go b/models/migrations/v1_22/v286_test.go index a19c9396e2e..1f213ddb6e0 100644 --- a/models/migrations/v1_22/v286_test.go +++ b/models/migrations/v1_22/v286_test.go @@ -107,12 +107,12 @@ func Test_RepositoryFormat(t *testing.T) { repo = new(Repository) ok, err := x.ID(2).Get(repo) assert.NoError(t, err) - assert.EqualValues(t, true, ok) + assert.True(t, ok) assert.EqualValues(t, "sha1", repo.ObjectFormatName) repo = new(Repository) ok, err = x.ID(id).Get(repo) assert.NoError(t, err) - assert.EqualValues(t, true, ok) + assert.True(t, ok) assert.EqualValues(t, "sha256", repo.ObjectFormatName) } diff --git a/models/migrations/v1_22/v294_test.go b/models/migrations/v1_22/v294_test.go index 82a3bcd602e..a1d702cb77d 100644 --- a/models/migrations/v1_22/v294_test.go +++ b/models/migrations/v1_22/v294_test.go @@ -39,7 +39,7 @@ func Test_AddUniqueIndexForProjectIssue(t *testing.T) { tables, err := x.DBMetas() assert.NoError(t, err) - assert.EqualValues(t, 1, len(tables)) + assert.Len(t, tables, 1) found := false for _, index := range tables[0].Indexes { if index.Type == schemas.UniqueType { diff --git a/models/organization/org_list_test.go b/models/organization/org_list_test.go index edc8996f3ec..0f0f8a4bcd0 100644 --- a/models/organization/org_list_test.go +++ b/models/organization/org_list_test.go @@ -40,7 +40,7 @@ func TestFindOrgs(t *testing.T) { IncludePrivate: false, }) assert.NoError(t, err) - assert.Len(t, orgs, 0) + assert.Empty(t, orgs) total, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{ UserID: 4, diff --git a/models/organization/org_test.go b/models/organization/org_test.go index 7159f0fc465..5e99e88689e 100644 --- a/models/organization/org_test.go +++ b/models/organization/org_test.go @@ -283,7 +283,7 @@ func TestGetOrgUsersByOrgID(t *testing.T) { OrgID: unittest.NonexistentID, }) assert.NoError(t, err) - assert.Len(t, orgUsers, 0) + assert.Empty(t, orgUsers) } func TestChangeOrgUserStatus(t *testing.T) { diff --git a/models/perm/access_mode_test.go b/models/perm/access_mode_test.go index 982fceee5a5..c4c7d483fb2 100644 --- a/models/perm/access_mode_test.go +++ b/models/perm/access_mode_test.go @@ -15,7 +15,7 @@ func TestAccessMode(t *testing.T) { m := ParseAccessMode(name) assert.Equal(t, AccessMode(i), m) } - assert.Equal(t, AccessMode(4), AccessModeOwner) + assert.Equal(t, AccessModeOwner, AccessMode(4)) assert.Equal(t, "owner", AccessModeOwner.ToString()) assert.Equal(t, AccessModeNone, ParseAccessMode("owner")) assert.Equal(t, AccessModeNone, ParseAccessMode("invalid")) diff --git a/models/project/column_test.go b/models/project/column_test.go index 911649fb726..566667e45d1 100644 --- a/models/project/column_test.go +++ b/models/project/column_test.go @@ -5,7 +5,6 @@ package project import ( "fmt" - "strings" "testing" "code.gitea.io/gitea/models/db" @@ -66,7 +65,7 @@ func Test_moveIssuesToAnotherColumn(t *testing.T) { issues, err = column1.GetIssues(db.DefaultContext) assert.NoError(t, err) - assert.Len(t, issues, 0) + assert.Empty(t, issues) issues, err = column2.GetIssues(db.DefaultContext) assert.NoError(t, err) @@ -123,5 +122,5 @@ func Test_NewColumn(t *testing.T) { ProjectID: project1.ID, }) assert.Error(t, err) - assert.True(t, strings.Contains(err.Error(), "maximum number of columns reached")) + assert.Contains(t, err.Error(), "maximum number of columns reached") } diff --git a/models/repo/repo_test.go b/models/repo/repo_test.go index 6468e0f6058..6d88d170da3 100644 --- a/models/repo/repo_test.go +++ b/models/repo/repo_test.go @@ -144,8 +144,8 @@ func TestGetRepositoryByURL(t *testing.T) { assert.NotNil(t, repo) assert.NoError(t, err) - assert.Equal(t, repo.ID, int64(2)) - assert.Equal(t, repo.OwnerID, int64(2)) + assert.Equal(t, int64(2), repo.ID) + assert.Equal(t, int64(2), repo.OwnerID) } test(t, "https://try.gitea.io/user2/repo2") @@ -159,8 +159,8 @@ func TestGetRepositoryByURL(t *testing.T) { assert.NotNil(t, repo) assert.NoError(t, err) - assert.Equal(t, repo.ID, int64(2)) - assert.Equal(t, repo.OwnerID, int64(2)) + assert.Equal(t, int64(2), repo.ID) + assert.Equal(t, int64(2), repo.OwnerID) } test(t, "git+ssh://sshuser@try.gitea.io/user2/repo2") @@ -177,8 +177,8 @@ func TestGetRepositoryByURL(t *testing.T) { assert.NotNil(t, repo) assert.NoError(t, err) - assert.Equal(t, repo.ID, int64(2)) - assert.Equal(t, repo.OwnerID, int64(2)) + assert.Equal(t, int64(2), repo.ID) + assert.Equal(t, int64(2), repo.OwnerID) } test(t, "sshuser@try.gitea.io:user2/repo2") diff --git a/models/repo/star_test.go b/models/repo/star_test.go index aaac89d975d..b540f54310c 100644 --- a/models/repo/star_test.go +++ b/models/repo/star_test.go @@ -52,7 +52,7 @@ func TestRepository_GetStargazers2(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) gazers, err := repo_model.GetStargazers(db.DefaultContext, repo, db.ListOptions{Page: 0}) assert.NoError(t, err) - assert.Len(t, gazers, 0) + assert.Empty(t, gazers) } func TestClearRepoStars(t *testing.T) { @@ -71,5 +71,5 @@ func TestClearRepoStars(t *testing.T) { gazers, err := repo_model.GetStargazers(db.DefaultContext, repo, db.ListOptions{Page: 0}) assert.NoError(t, err) - assert.Len(t, gazers, 0) + assert.Empty(t, gazers) } diff --git a/models/repo/user_repo_test.go b/models/repo/user_repo_test.go index f2abc2ffa01..44ebe5f214c 100644 --- a/models/repo/user_repo_test.go +++ b/models/repo/user_repo_test.go @@ -21,7 +21,7 @@ func TestRepoAssignees(t *testing.T) { users, err := repo_model.GetRepoAssignees(db.DefaultContext, repo2) assert.NoError(t, err) assert.Len(t, users, 1) - assert.Equal(t, users[0].ID, int64(2)) + assert.Equal(t, int64(2), users[0].ID) repo21 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 21}) users, err = repo_model.GetRepoAssignees(db.DefaultContext, repo21) diff --git a/models/repo/watch_test.go b/models/repo/watch_test.go index a95a2679616..c39ef607e8b 100644 --- a/models/repo/watch_test.go +++ b/models/repo/watch_test.go @@ -41,7 +41,7 @@ func TestGetWatchers(t *testing.T) { watches, err = repo_model.GetWatchers(db.DefaultContext, unittest.NonexistentID) assert.NoError(t, err) - assert.Len(t, watches, 0) + assert.Empty(t, watches) } func TestRepository_GetWatchers(t *testing.T) { @@ -58,7 +58,7 @@ func TestRepository_GetWatchers(t *testing.T) { repo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 9}) watchers, err = repo_model.GetRepoWatchers(db.DefaultContext, repo.ID, db.ListOptions{Page: 1}) assert.NoError(t, err) - assert.Len(t, watchers, 0) + assert.Empty(t, watchers) } func TestWatchIfAuto(t *testing.T) { diff --git a/models/unittest/unit_tests.go b/models/unittest/unit_tests.go index 3b0f28d3a60..4ac858e04ea 100644 --- a/models/unittest/unit_tests.go +++ b/models/unittest/unit_tests.go @@ -79,7 +79,7 @@ func AssertExistsAndLoadMap(t assert.TestingT, table string, conditions ...any) e := db.GetEngine(db.DefaultContext).Table(table) res, err := whereOrderConditions(e, conditions).Query() assert.NoError(t, err) - assert.True(t, len(res) == 1, + assert.Len(t, res, 1, "Expected to find one row in %s (with conditions %+v), but found %d", table, conditions, len(res), ) diff --git a/models/user/email_address_test.go b/models/user/email_address_test.go index c2e010d95b3..d72d873de2c 100644 --- a/models/user/email_address_test.go +++ b/models/user/email_address_test.go @@ -97,8 +97,7 @@ func TestListEmails(t *testing.T) { } emails, count, err := user_model.SearchEmails(db.DefaultContext, opts) assert.NoError(t, err) - assert.NotEqual(t, int64(0), count) - assert.True(t, count > 5) + assert.Greater(t, count, int64(5)) contains := func(match func(s *user_model.SearchEmailResult) bool) bool { for _, v := range emails { diff --git a/models/user/setting_test.go b/models/user/setting_test.go index c56fe930750..c607d9fd008 100644 --- a/models/user/setting_test.go +++ b/models/user/setting_test.go @@ -56,5 +56,5 @@ func TestSettings(t *testing.T) { assert.NoError(t, err) settings, err = user_model.GetUserAllSettings(db.DefaultContext, 99) assert.NoError(t, err) - assert.Len(t, settings, 0) + assert.Empty(t, settings) } diff --git a/models/user/user_test.go b/models/user/user_test.go index 6701be39a55..7ebc64f69e7 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -201,7 +201,7 @@ func TestNewGitSig(t *testing.T) { assert.NotContains(t, sig.Name, "<") assert.NotContains(t, sig.Name, ">") assert.NotContains(t, sig.Name, "\n") - assert.NotEqual(t, len(strings.TrimSpace(sig.Name)), 0) + assert.NotEmpty(t, strings.TrimSpace(sig.Name)) } } @@ -216,7 +216,7 @@ func TestDisplayName(t *testing.T) { if len(strings.TrimSpace(user.FullName)) == 0 { assert.Equal(t, user.Name, displayName) } - assert.NotEqual(t, len(strings.TrimSpace(displayName)), 0) + assert.NotEmpty(t, strings.TrimSpace(displayName)) } } @@ -322,15 +322,15 @@ func TestGetMaileableUsersByIDs(t *testing.T) { assert.NoError(t, err) assert.Len(t, results, 1) if len(results) > 1 { - assert.Equal(t, results[0].ID, 1) + assert.Equal(t, 1, results[0].ID) } results, err = user_model.GetMaileableUsersByIDs(db.DefaultContext, []int64{1, 4}, true) assert.NoError(t, err) assert.Len(t, results, 2) if len(results) > 2 { - assert.Equal(t, results[0].ID, 1) - assert.Equal(t, results[1].ID, 4) + assert.Equal(t, 1, results[0].ID) + assert.Equal(t, 4, results[1].ID) } } @@ -499,7 +499,7 @@ func Test_ValidateUser(t *testing.T) { {ID: 2, Visibility: structs.VisibleTypePrivate}: true, } for kase, expected := range kases { - assert.EqualValues(t, expected, nil == user_model.ValidateUser(kase), fmt.Sprintf("case: %+v", kase)) + assert.EqualValues(t, expected, nil == user_model.ValidateUser(kase), "case: %+v", kase) } } @@ -570,11 +570,11 @@ func TestDisabledUserFeatures(t *testing.T) { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - assert.Len(t, setting.Admin.UserDisabledFeatures.Values(), 0) + assert.Empty(t, setting.Admin.UserDisabledFeatures.Values()) // no features should be disabled with a plain login type assert.LessOrEqual(t, user.LoginType, auth.Plain) - assert.Len(t, user_model.DisabledFeaturesWithLoginType(user).Values(), 0) + assert.Empty(t, user_model.DisabledFeaturesWithLoginType(user).Values()) for _, f := range testValues.Values() { assert.False(t, user_model.IsFeatureDisabledWithLoginType(user, f)) } @@ -600,5 +600,5 @@ func TestGetInactiveUsers(t *testing.T) { interval := time.Now().Unix() - 1730468968 + 3600*24 users, err = user_model.GetInactiveUsers(db.DefaultContext, time.Duration(interval*int64(time.Second))) assert.NoError(t, err) - assert.Len(t, users, 0) + assert.Empty(t, users) } diff --git a/models/webhook/webhook_test.go b/models/webhook/webhook_test.go index f4403776cec..c6c3f40d46e 100644 --- a/models/webhook/webhook_test.go +++ b/models/webhook/webhook_test.go @@ -43,7 +43,7 @@ func TestWebhook_History(t *testing.T) { webhook = unittest.AssertExistsAndLoadBean(t, &Webhook{ID: 2}) tasks, err = webhook.History(db.DefaultContext, 0) assert.NoError(t, err) - assert.Len(t, tasks, 0) + assert.Empty(t, tasks) } func TestWebhook_UpdateEvent(t *testing.T) { @@ -206,7 +206,7 @@ func TestHookTasks(t *testing.T) { hookTasks, err = HookTasks(db.DefaultContext, unittest.NonexistentID, 1) assert.NoError(t, err) - assert.Len(t, hookTasks, 0) + assert.Empty(t, hookTasks) } func TestCreateHookTask(t *testing.T) { diff --git a/modules/activitypub/client_test.go b/modules/activitypub/client_test.go index 65ea8d4d5bf..d0c48454457 100644 --- a/modules/activitypub/client_test.go +++ b/modules/activitypub/client_test.go @@ -8,7 +8,6 @@ import ( "io" "net/http" "net/http/httptest" - "regexp" "testing" "code.gitea.io/gitea/models/db" @@ -28,9 +27,9 @@ func TestActivityPubSignedPost(t *testing.T) { expected := "BODY" srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Regexp(t, regexp.MustCompile("^"+setting.Federation.DigestAlgorithm), r.Header.Get("Digest")) + assert.Regexp(t, "^"+setting.Federation.DigestAlgorithm, r.Header.Get("Digest")) assert.Contains(t, r.Header.Get("Signature"), pubID) - assert.Equal(t, r.Header.Get("Content-Type"), ActivityStreamsContentType) + assert.Equal(t, ActivityStreamsContentType, r.Header.Get("Content-Type")) body, err := io.ReadAll(r.Body) assert.NoError(t, err) assert.Equal(t, expected, string(body)) diff --git a/modules/assetfs/layered_test.go b/modules/assetfs/layered_test.go index b82111e745e..03a3ae0d7cc 100644 --- a/modules/assetfs/layered_test.go +++ b/modules/assetfs/layered_test.go @@ -58,7 +58,7 @@ func TestLayered(t *testing.T) { assertRead := func(expected string, expectedErr error, elems ...string) { bs, err := assets.ReadFile(elems...) if err != nil { - assert.ErrorAs(t, err, &expectedErr) + assert.ErrorIs(t, err, expectedErr) } else { assert.NoError(t, err) assert.Equal(t, expected, string(bs)) diff --git a/modules/auth/pam/pam_test.go b/modules/auth/pam/pam_test.go index c277d59c415..7265b5d0c15 100644 --- a/modules/auth/pam/pam_test.go +++ b/modules/auth/pam/pam_test.go @@ -15,5 +15,5 @@ func TestPamAuth(t *testing.T) { result, err := Auth("gitea", "user1", "false-pwd") assert.Error(t, err) assert.EqualError(t, err, "Authentication failure") - assert.Len(t, result, 0) + assert.Len(t, result) } diff --git a/modules/auth/password/hash/dummy_test.go b/modules/auth/password/hash/dummy_test.go index f3b36df6250..e56e3f1a7f7 100644 --- a/modules/auth/password/hash/dummy_test.go +++ b/modules/auth/password/hash/dummy_test.go @@ -18,7 +18,7 @@ func TestDummyHasher(t *testing.T) { password, salt := "password", "ZogKvWdyEx" hash, err := dummy.Hash(password, salt) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, hash, salt+":"+password) assert.True(t, dummy.VerifyPassword(password, hash, salt)) diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go index 86cccdf2092..f63679048e3 100644 --- a/modules/base/tool_test.go +++ b/modules/base/tool_test.go @@ -6,7 +6,6 @@ package base import ( "crypto/sha1" "fmt" - "os" "testing" "time" @@ -157,7 +156,7 @@ func TestStringsToInt64s(t *testing.T) { testSuccess([]string{"1", "4", "16", "64", "256"}, []int64{1, 4, 16, 64, 256}) ints, err := StringsToInt64s([]string{"-1", "a"}) - assert.Len(t, ints, 0) + assert.Empty(t, ints) assert.Error(t, err) } @@ -172,9 +171,9 @@ func TestInt64sToStrings(t *testing.T) { // TODO: Test EntryIcon func TestSetupGiteaRoot(t *testing.T) { - _ = os.Setenv("GITEA_ROOT", "test") + t.Setenv("GITEA_ROOT", "test") assert.Equal(t, "test", SetupGiteaRoot()) - _ = os.Setenv("GITEA_ROOT", "") + t.Setenv("GITEA_ROOT", "") assert.NotEqual(t, "test", SetupGiteaRoot()) } diff --git a/modules/dump/dumper_test.go b/modules/dump/dumper_test.go index b444fa2de53..2db3a598a4c 100644 --- a/modules/dump/dumper_test.go +++ b/modules/dump/dumper_test.go @@ -25,7 +25,7 @@ func TestPrepareFileNameAndType(t *testing.T) { assert.Equal(t, fmt.Sprintf("outFile=%s, outType=%s", expFile, expType), fmt.Sprintf("outFile=%s, outType=%s", outFile, outType), - fmt.Sprintf("argFile=%s, argType=%s", argFile, argType), + "argFile=%s, argType=%s", argFile, argType, ) } diff --git a/modules/git/commit_sha256_test.go b/modules/git/commit_sha256_test.go index 3b8b6d3763a..2184a9c47cf 100644 --- a/modules/git/commit_sha256_test.go +++ b/modules/git/commit_sha256_test.go @@ -146,7 +146,7 @@ func TestHasPreviousCommitSha256(t *testing.T) { parentSHA := MustIDFromString("b0ec7af4547047f12d5093e37ef8f1b3b5415ed8ee17894d43a34d7d34212e9c") notParentSHA := MustIDFromString("42e334efd04cd36eea6da0599913333c26116e1a537ca76e5b6e4af4dda00236") assert.Equal(t, objectFormat, parentSHA.Type()) - assert.Equal(t, objectFormat.Name(), "sha256") + assert.Equal(t, "sha256", objectFormat.Name()) haz, err := commit.HasPreviousCommit(parentSHA) assert.NoError(t, err) diff --git a/modules/git/commit_test.go b/modules/git/commit_test.go index bf381a53501..6ac65564dc1 100644 --- a/modules/git/commit_test.go +++ b/modules/git/commit_test.go @@ -343,9 +343,9 @@ func TestGetCommitFileStatusMerges(t *testing.T) { }, } - assert.Equal(t, commitFileStatus.Added, expected.Added) - assert.Equal(t, commitFileStatus.Removed, expected.Removed) - assert.Equal(t, commitFileStatus.Modified, expected.Modified) + assert.Equal(t, expected.Added, commitFileStatus.Added) + assert.Equal(t, expected.Removed, commitFileStatus.Removed) + assert.Equal(t, expected.Modified, commitFileStatus.Modified) } func Test_GetCommitBranchStart(t *testing.T) { diff --git a/modules/git/grep_test.go b/modules/git/grep_test.go index 6a99f804070..005d5397267 100644 --- a/modules/git/grep_test.go +++ b/modules/git/grep_test.go @@ -73,9 +73,9 @@ func TestGrepSearch(t *testing.T) { res, err = GrepSearch(context.Background(), repo, "no-such-content", GrepOptions{}) assert.NoError(t, err) - assert.Len(t, res, 0) + assert.Empty(t, res) res, err = GrepSearch(context.Background(), &Repository{Path: "no-such-git-repo"}, "no-such-content", GrepOptions{}) assert.Error(t, err) - assert.Len(t, res, 0) + assert.Empty(t, res) } diff --git a/modules/git/parse_nogogit_test.go b/modules/git/parse_nogogit_test.go index 23fddb014c1..a4436ce499a 100644 --- a/modules/git/parse_nogogit_test.go +++ b/modules/git/parse_nogogit_test.go @@ -100,5 +100,5 @@ func TestParseTreeEntriesInvalid(t *testing.T) { // there was a panic: "runtime error: slice bounds out of range" when the input was invalid: #20315 entries, err := ParseTreeEntries([]byte("100644 blob ea0d83c9081af9500ac9f804101b3fd0a5c293af")) assert.Error(t, err) - assert.Len(t, entries, 0) + assert.Empty(t, entries) } diff --git a/modules/git/repo_branch_test.go b/modules/git/repo_branch_test.go index 009c545832a..5d3b8abb3a8 100644 --- a/modules/git/repo_branch_test.go +++ b/modules/git/repo_branch_test.go @@ -34,7 +34,7 @@ func TestRepository_GetBranches(t *testing.T) { branches, countAll, err = bareRepo1.GetBranchNames(5, 1) assert.NoError(t, err) - assert.Len(t, branches, 0) + assert.Empty(t, branches) assert.EqualValues(t, 3, countAll) assert.ElementsMatch(t, []string{}, branches) } @@ -66,7 +66,7 @@ func TestGetRefsBySha(t *testing.T) { // do not exist branches, err := bareRepo5.GetRefsBySha("8006ff9adbf0cb94da7dad9e537e53817f9fa5c0", "") assert.NoError(t, err) - assert.Len(t, branches, 0) + assert.Empty(t, branches) // refs/pull/1/head branches, err = bareRepo5.GetRefsBySha("c83380d7056593c51a699d12b9c00627bd5743e9", PullPrefix) diff --git a/modules/git/repo_compare_test.go b/modules/git/repo_compare_test.go index 99838731867..454ed6b9f85 100644 --- a/modules/git/repo_compare_test.go +++ b/modules/git/repo_compare_test.go @@ -72,7 +72,7 @@ func TestReadPatch(t *testing.T) { assert.Empty(t, noFile) assert.Empty(t, noCommit) assert.Len(t, oldCommit, 40) - assert.True(t, oldCommit == "6e8e2a6f9efd71dbe6917816343ed8415ad696c3") + assert.Equal(t, "6e8e2a6f9efd71dbe6917816343ed8415ad696c3", oldCommit) } func TestReadWritePullHead(t *testing.T) { @@ -113,7 +113,7 @@ func TestReadWritePullHead(t *testing.T) { } assert.Len(t, headContents, 40) - assert.True(t, headContents == newCommit) + assert.Equal(t, headContents, newCommit) // Remove file after the test err = repo.RemoveReference(PullPrefix + "1/head") diff --git a/modules/indexer/issues/internal/tests/tests.go b/modules/indexer/issues/internal/tests/tests.go index 16f0a78ec04..94ce8520bf6 100644 --- a/modules/indexer/issues/internal/tests/tests.go +++ b/modules/indexer/issues/internal/tests/tests.go @@ -113,7 +113,7 @@ var cases = []*testIndexerCase{ }, }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) assert.Equal(t, len(data), int(result.Total)) }, }, @@ -176,7 +176,7 @@ var cases = []*testIndexerCase{ IsPull: optional.Some(false), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.False(t, data[v.ID].IsPull) } @@ -192,7 +192,7 @@ var cases = []*testIndexerCase{ IsPull: optional.Some(true), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.True(t, data[v.ID].IsPull) } @@ -208,7 +208,7 @@ var cases = []*testIndexerCase{ IsClosed: optional.Some(false), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.False(t, data[v.ID].IsClosed) } @@ -224,7 +224,7 @@ var cases = []*testIndexerCase{ IsClosed: optional.Some(true), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.True(t, data[v.ID].IsClosed) } @@ -274,7 +274,7 @@ var cases = []*testIndexerCase{ MilestoneIDs: []int64{1, 2, 6}, }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Contains(t, []int64{1, 2, 6}, data[v.ID].MilestoneID) } @@ -292,7 +292,7 @@ var cases = []*testIndexerCase{ MilestoneIDs: []int64{0}, }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Equal(t, int64(0), data[v.ID].MilestoneID) } @@ -310,7 +310,7 @@ var cases = []*testIndexerCase{ ProjectID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Equal(t, int64(1), data[v.ID].ProjectID) } @@ -328,7 +328,7 @@ var cases = []*testIndexerCase{ ProjectID: optional.Some(int64(0)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Equal(t, int64(0), data[v.ID].ProjectID) } @@ -346,7 +346,7 @@ var cases = []*testIndexerCase{ ProjectColumnID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Equal(t, int64(1), data[v.ID].ProjectColumnID) } @@ -364,7 +364,7 @@ var cases = []*testIndexerCase{ ProjectColumnID: optional.Some(int64(0)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Equal(t, int64(0), data[v.ID].ProjectColumnID) } @@ -382,7 +382,7 @@ var cases = []*testIndexerCase{ PosterID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Equal(t, int64(1), data[v.ID].PosterID) } @@ -400,7 +400,7 @@ var cases = []*testIndexerCase{ AssigneeID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Equal(t, int64(1), data[v.ID].AssigneeID) } @@ -418,7 +418,7 @@ var cases = []*testIndexerCase{ AssigneeID: optional.Some(int64(0)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Equal(t, int64(0), data[v.ID].AssigneeID) } @@ -436,7 +436,7 @@ var cases = []*testIndexerCase{ MentionID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Contains(t, data[v.ID].MentionIDs, int64(1)) } @@ -454,7 +454,7 @@ var cases = []*testIndexerCase{ ReviewedID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Contains(t, data[v.ID].ReviewedIDs, int64(1)) } @@ -472,7 +472,7 @@ var cases = []*testIndexerCase{ ReviewRequestedID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Contains(t, data[v.ID].ReviewRequestedIDs, int64(1)) } @@ -490,7 +490,7 @@ var cases = []*testIndexerCase{ SubscriberID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.Contains(t, data[v.ID].SubscriberIDs, int64(1)) } @@ -509,7 +509,7 @@ var cases = []*testIndexerCase{ UpdatedBeforeUnix: optional.Some(int64(30)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { - assert.Equal(t, 5, len(result.Hits)) + assert.Len(t, result.Hits, 5) for _, v := range result.Hits { assert.GreaterOrEqual(t, data[v.ID].UpdatedUnix, int64(20)) assert.LessOrEqual(t, data[v.ID].UpdatedUnix, int64(30)) diff --git a/modules/lfs/transferadapter_test.go b/modules/lfs/transferadapter_test.go index 7fec137efe5..a430b71a5f1 100644 --- a/modules/lfs/transferadapter_test.go +++ b/modules/lfs/transferadapter_test.go @@ -96,7 +96,7 @@ func TestBasicTransferAdapter(t *testing.T) { for n, c := range cases { _, err := a.Download(context.Background(), c.link) if len(c.expectederror) > 0 { - assert.True(t, strings.Contains(err.Error(), c.expectederror), "case %d: '%s' should contain '%s'", n, err.Error(), c.expectederror) + assert.Contains(t, err.Error(), c.expectederror, "case %d: '%s' should contain '%s'", n, err.Error(), c.expectederror) } else { assert.NoError(t, err, "case %d", n) } @@ -129,7 +129,7 @@ func TestBasicTransferAdapter(t *testing.T) { for n, c := range cases { err := a.Upload(context.Background(), c.link, p, bytes.NewBufferString("dummy")) if len(c.expectederror) > 0 { - assert.True(t, strings.Contains(err.Error(), c.expectederror), "case %d: '%s' should contain '%s'", n, err.Error(), c.expectederror) + assert.Contains(t, err.Error(), c.expectederror, "case %d: '%s' should contain '%s'", n, err.Error(), c.expectederror) } else { assert.NoError(t, err, "case %d", n) } @@ -162,7 +162,7 @@ func TestBasicTransferAdapter(t *testing.T) { for n, c := range cases { err := a.Verify(context.Background(), c.link, p) if len(c.expectederror) > 0 { - assert.True(t, strings.Contains(err.Error(), c.expectederror), "case %d: '%s' should contain '%s'", n, err.Error(), c.expectederror) + assert.Contains(t, err.Error(), c.expectederror, "case %d: '%s' should contain '%s'", n, err.Error(), c.expectederror) } else { assert.NoError(t, err, "case %d", n) } diff --git a/modules/log/logger_test.go b/modules/log/logger_test.go index 70222f64f5c..0de14eb411e 100644 --- a/modules/log/logger_test.go +++ b/modules/log/logger_test.go @@ -56,7 +56,7 @@ func TestLogger(t *testing.T) { logger := NewLoggerWithWriters(context.Background(), "test") dump := logger.DumpWriters() - assert.EqualValues(t, 0, len(dump)) + assert.Empty(t, dump) assert.EqualValues(t, NONE, logger.GetLevel()) assert.False(t, logger.IsEnabled()) @@ -69,7 +69,7 @@ func TestLogger(t *testing.T) { assert.EqualValues(t, DEBUG, logger.GetLevel()) dump = logger.DumpWriters() - assert.EqualValues(t, 2, len(dump)) + assert.Len(t, dump, 2) logger.Trace("trace-level") // this level is not logged logger.Debug("debug-level") diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index 9419350e615..159d7129557 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -278,12 +278,12 @@ func TestRender_AutoLink(t *testing.T) { test := func(input, expected string) { var buffer strings.Builder err := PostProcessDefault(NewTestRenderContext(localMetas), strings.NewReader(input), &buffer) - assert.Equal(t, err, nil) + assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String())) buffer.Reset() err = PostProcessDefault(NewTestRenderContext(localMetas), strings.NewReader(input), &buffer) - assert.Equal(t, err, nil) + assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String())) } diff --git a/modules/packages/conan/conanfile_parser_test.go b/modules/packages/conan/conanfile_parser_test.go index 58015701841..aabafd5f64f 100644 --- a/modules/packages/conan/conanfile_parser_test.go +++ b/modules/packages/conan/conanfile_parser_test.go @@ -40,7 +40,7 @@ class ConanPackageConan(ConanFile): func TestParseConanfile(t *testing.T) { metadata, err := ParseConanfile(strings.NewReader(contentConanfile)) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, license, metadata.License) assert.Equal(t, author, metadata.Author) assert.Equal(t, homepage, metadata.ProjectURL) diff --git a/modules/packages/conan/conaninfo_parser_test.go b/modules/packages/conan/conaninfo_parser_test.go index 556a4b939ee..f6510ca6670 100644 --- a/modules/packages/conan/conaninfo_parser_test.go +++ b/modules/packages/conan/conaninfo_parser_test.go @@ -50,7 +50,7 @@ const ( func TestParseConaninfo(t *testing.T) { info, err := ParseConaninfo(strings.NewReader(contentConaninfo)) assert.NotNil(t, info) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal( t, map[string]string{ diff --git a/modules/queue/base_test.go b/modules/queue/base_test.go index c5bf526ae67..01b52b3c16e 100644 --- a/modules/queue/base_test.go +++ b/modules/queue/base_test.go @@ -46,10 +46,10 @@ func testQueueBasic(t *testing.T, newFn func(cfg *BaseConfig) (baseQueue, error) assert.NoError(t, err) if !isUnique { assert.EqualValues(t, 2, cnt) - assert.EqualValues(t, false, has) // non-unique queues don't check for duplicates + assert.False(t, has) // non-unique queues don't check for duplicates } else { assert.EqualValues(t, 1, cnt) - assert.EqualValues(t, true, has) + assert.True(t, has) } // push another item @@ -101,7 +101,7 @@ func testQueueBasic(t *testing.T, newFn func(cfg *BaseConfig) (baseQueue, error) pushBlockTime = 30 * time.Millisecond err = q.PushItem(ctx, []byte("item-full")) assert.ErrorIs(t, err, context.DeadlineExceeded) - assert.True(t, time.Since(timeStart) >= pushBlockTime*2/3) + assert.GreaterOrEqual(t, time.Since(timeStart), pushBlockTime*2/3) pushBlockTime = oldPushBlockTime // remove all diff --git a/modules/queue/workerqueue_test.go b/modules/queue/workerqueue_test.go index d66253ff664..c0841a1752a 100644 --- a/modules/queue/workerqueue_test.go +++ b/modules/queue/workerqueue_test.go @@ -172,8 +172,8 @@ func testWorkerPoolQueuePersistence(t *testing.T, queueSetting setting.QueueSett q2() // restart the queue to continue to execute the tasks in it - assert.NotZero(t, len(tasksQ1)) - assert.NotZero(t, len(tasksQ2)) + assert.NotEmpty(t, tasksQ1) + assert.NotEmpty(t, tasksQ2) assert.EqualValues(t, testCount, len(tasksQ1)+len(tasksQ2)) } diff --git a/modules/references/references_test.go b/modules/references/references_test.go index e5a0d60fe3b..e224c919e92 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -526,7 +526,7 @@ func TestCustomizeCloseKeywords(t *testing.T) { func TestParseCloseKeywords(t *testing.T) { // Test parsing of CloseKeywords and ReopenKeywords - assert.Len(t, parseKeywords([]string{""}), 0) + assert.Empty(t, parseKeywords([]string{""})) assert.Len(t, parseKeywords([]string{" aa ", " bb ", "99", "#", "", "this is", "cc"}), 3) for _, test := range []struct { diff --git a/modules/repository/repo_test.go b/modules/repository/repo_test.go index 68980f92f94..f3e7be6d7d7 100644 --- a/modules/repository/repo_test.go +++ b/modules/repository/repo_test.go @@ -62,15 +62,15 @@ func Test_calcSync(t *testing.T) { } inserts, deletes, updates := calcSync(gitTags, dbReleases) - if assert.EqualValues(t, 1, len(inserts), "inserts") { + if assert.Len(t, inserts, 1, "inserts") { assert.EqualValues(t, *gitTags[2], *inserts[0], "inserts equal") } - if assert.EqualValues(t, 1, len(deletes), "deletes") { + if assert.Len(t, deletes, 1, "deletes") { assert.EqualValues(t, 1, deletes[0], "deletes equal") } - if assert.EqualValues(t, 1, len(updates), "updates") { + if assert.Len(t, updates, 1, "updates") { assert.EqualValues(t, *gitTags[1], *updates[0], "updates equal") } } diff --git a/modules/setting/cron_test.go b/modules/setting/cron_test.go index 3187ab18a25..55244d70757 100644 --- a/modules/setting/cron_test.go +++ b/modules/setting/cron_test.go @@ -38,6 +38,6 @@ EXTEND = true _, err = getCronSettings(cfg, "test", extended) assert.NoError(t, err) assert.True(t, extended.Base) - assert.EqualValues(t, extended.Second, "white rabbit") + assert.EqualValues(t, "white rabbit", extended.Second) assert.True(t, extended.Extend) } diff --git a/modules/setting/oauth2_test.go b/modules/setting/oauth2_test.go index 38ee4d248d6..d0e5ccf13d2 100644 --- a/modules/setting/oauth2_test.go +++ b/modules/setting/oauth2_test.go @@ -74,5 +74,5 @@ DEFAULT_APPLICATIONS = tea DEFAULT_APPLICATIONS = `) loadOAuth2From(cfg) - assert.Nil(t, nil, OAuth2.DefaultApplications) + assert.Nil(t, OAuth2.DefaultApplications) } diff --git a/modules/setting/storage_test.go b/modules/setting/storage_test.go index 8ee37fd2b6d..afff85537e3 100644 --- a/modules/setting/storage_test.go +++ b/modules/setting/storage_test.go @@ -447,7 +447,7 @@ MINIO_USE_SSL = true assert.NoError(t, loadRepoArchiveFrom(cfg)) assert.EqualValues(t, "my_access_key", RepoArchive.Storage.MinioConfig.AccessKeyID) assert.EqualValues(t, "my_secret_key", RepoArchive.Storage.MinioConfig.SecretAccessKey) - assert.EqualValues(t, true, RepoArchive.Storage.MinioConfig.UseSSL) + assert.True(t, RepoArchive.Storage.MinioConfig.UseSSL) assert.EqualValues(t, "repo-archive/", RepoArchive.Storage.MinioConfig.BasePath) } @@ -464,7 +464,7 @@ MINIO_BASE_PATH = /prefix assert.NoError(t, loadRepoArchiveFrom(cfg)) assert.EqualValues(t, "my_access_key", RepoArchive.Storage.MinioConfig.AccessKeyID) assert.EqualValues(t, "my_secret_key", RepoArchive.Storage.MinioConfig.SecretAccessKey) - assert.EqualValues(t, true, RepoArchive.Storage.MinioConfig.UseSSL) + assert.True(t, RepoArchive.Storage.MinioConfig.UseSSL) assert.EqualValues(t, "/prefix/repo-archive/", RepoArchive.Storage.MinioConfig.BasePath) cfg, err = NewConfigProviderFromData(` @@ -477,7 +477,7 @@ MINIO_BASE_PATH = /prefix assert.NoError(t, err) assert.NoError(t, loadRepoArchiveFrom(cfg)) assert.EqualValues(t, "127.0.0.1", RepoArchive.Storage.MinioConfig.IamEndpoint) - assert.EqualValues(t, true, RepoArchive.Storage.MinioConfig.UseSSL) + assert.True(t, RepoArchive.Storage.MinioConfig.UseSSL) assert.EqualValues(t, "/prefix/repo-archive/", RepoArchive.Storage.MinioConfig.BasePath) cfg, err = NewConfigProviderFromData(` @@ -495,7 +495,7 @@ MINIO_BASE_PATH = /lfs assert.NoError(t, loadLFSFrom(cfg)) assert.EqualValues(t, "my_access_key", LFS.Storage.MinioConfig.AccessKeyID) assert.EqualValues(t, "my_secret_key", LFS.Storage.MinioConfig.SecretAccessKey) - assert.EqualValues(t, true, LFS.Storage.MinioConfig.UseSSL) + assert.True(t, LFS.Storage.MinioConfig.UseSSL) assert.EqualValues(t, "/lfs", LFS.Storage.MinioConfig.BasePath) cfg, err = NewConfigProviderFromData(` @@ -513,7 +513,7 @@ MINIO_BASE_PATH = /lfs assert.NoError(t, loadLFSFrom(cfg)) assert.EqualValues(t, "my_access_key", LFS.Storage.MinioConfig.AccessKeyID) assert.EqualValues(t, "my_secret_key", LFS.Storage.MinioConfig.SecretAccessKey) - assert.EqualValues(t, true, LFS.Storage.MinioConfig.UseSSL) + assert.True(t, LFS.Storage.MinioConfig.UseSSL) assert.EqualValues(t, "/lfs", LFS.Storage.MinioConfig.BasePath) } diff --git a/modules/user/user_test.go b/modules/user/user_test.go index 9129ae79a13..372a675d342 100644 --- a/modules/user/user_test.go +++ b/modules/user/user_test.go @@ -4,7 +4,6 @@ package user import ( - "os" "os/exec" "runtime" "strings" @@ -36,7 +35,7 @@ func TestCurrentUsername(t *testing.T) { if user != whoami { t.Errorf("expected %s as user, got: %s", whoami, user) } - os.Setenv("USER", "spoofed") + t.Setenv("USER", "spoofed") user = CurrentUsername() if user != whoami { t.Errorf("expected %s as user, got: %s", whoami, user) diff --git a/modules/util/color_test.go b/modules/util/color_test.go index be6e6b122a5..abd55512184 100644 --- a/modules/util/color_test.go +++ b/modules/util/color_test.go @@ -27,9 +27,9 @@ func Test_HexToRBGColor(t *testing.T) { } for n, c := range cases { r, g, b := HexToRBGColor(c.colorString) - assert.Equal(t, c.expectedR, r, "case %d: error R should match: expected %f, but get %f", n, c.expectedR, r) - assert.Equal(t, c.expectedG, g, "case %d: error G should match: expected %f, but get %f", n, c.expectedG, g) - assert.Equal(t, c.expectedB, b, "case %d: error B should match: expected %f, but get %f", n, c.expectedB, b) + assert.InDelta(t, c.expectedR, r, 0, "case %d: error R should match: expected %f, but get %f", n, c.expectedR, r) + assert.InDelta(t, c.expectedG, g, 0, "case %d: error G should match: expected %f, but get %f", n, c.expectedG, g) + assert.InDelta(t, c.expectedB, b, 0, "case %d: error B should match: expected %f, but get %f", n, c.expectedB, b) } } diff --git a/modules/util/keypair_test.go b/modules/util/keypair_test.go index c6f68c845a4..2bade3bb28a 100644 --- a/modules/util/keypair_test.go +++ b/modules/util/keypair_test.go @@ -10,7 +10,6 @@ import ( "crypto/sha256" "crypto/x509" "encoding/pem" - "regexp" "testing" "github.com/stretchr/testify/assert" @@ -23,8 +22,8 @@ func TestKeygen(t *testing.T) { assert.NotEmpty(t, priv) assert.NotEmpty(t, pub) - assert.Regexp(t, regexp.MustCompile("^-----BEGIN RSA PRIVATE KEY-----.*"), priv) - assert.Regexp(t, regexp.MustCompile("^-----BEGIN PUBLIC KEY-----.*"), pub) + assert.Regexp(t, "^-----BEGIN RSA PRIVATE KEY-----.*", priv) + assert.Regexp(t, "^-----BEGIN PUBLIC KEY-----.*", pub) } func TestSignUsingKeys(t *testing.T) { diff --git a/modules/util/time_str_test.go b/modules/util/time_str_test.go index 67b7978d0bf..8d1de51c8e6 100644 --- a/modules/util/time_str_test.go +++ b/modules/util/time_str_test.go @@ -27,9 +27,9 @@ func TestTimeStr(t *testing.T) { t.Run(test.input, func(t *testing.T) { output, err := TimeEstimateParse(test.input) if test.err { - assert.NotNil(t, err) + assert.Error(t, err) } else { - assert.Nil(t, err) + assert.NoError(t, err) } assert.Equal(t, test.output, output) }) diff --git a/modules/util/util_test.go b/modules/util/util_test.go index 9ce72fb8669..5abce08b41f 100644 --- a/modules/util/util_test.go +++ b/modules/util/util_test.go @@ -122,8 +122,8 @@ func Test_NormalizeEOL(t *testing.T) { func Test_RandomInt(t *testing.T) { randInt, err := CryptoRandomInt(255) - assert.True(t, randInt >= 0) - assert.True(t, randInt <= 255) + assert.GreaterOrEqual(t, randInt, int64(0)) + assert.LessOrEqual(t, randInt, int64(255)) assert.NoError(t, err) } @@ -223,22 +223,22 @@ func BenchmarkToUpper(b *testing.B) { } func TestToTitleCase(t *testing.T) { - assert.Equal(t, ToTitleCase(`foo bar baz`), `Foo Bar Baz`) - assert.Equal(t, ToTitleCase(`FOO BAR BAZ`), `Foo Bar Baz`) + assert.Equal(t, `Foo Bar Baz`, ToTitleCase(`foo bar baz`)) + assert.Equal(t, `Foo Bar Baz`, ToTitleCase(`FOO BAR BAZ`)) } func TestToPointer(t *testing.T) { assert.Equal(t, "abc", *ToPointer("abc")) assert.Equal(t, 123, *ToPointer(123)) abc := "abc" - assert.False(t, &abc == ToPointer(abc)) + assert.NotSame(t, &abc, ToPointer(abc)) val123 := 123 - assert.False(t, &val123 == ToPointer(val123)) + assert.NotSame(t, &val123, ToPointer(val123)) } func TestReserveLineBreakForTextarea(t *testing.T) { - assert.Equal(t, ReserveLineBreakForTextarea("test\r\ndata"), "test\ndata") - assert.Equal(t, ReserveLineBreakForTextarea("test\r\ndata\r\n"), "test\ndata\n") + assert.Equal(t, "test\ndata", ReserveLineBreakForTextarea("test\r\ndata")) + assert.Equal(t, "test\ndata\n", ReserveLineBreakForTextarea("test\r\ndata\r\n")) } func TestOptionalArg(t *testing.T) { diff --git a/routers/private/hook_post_receive_test.go b/routers/private/hook_post_receive_test.go index 658557d3cfc..a089739d156 100644 --- a/routers/private/hook_post_receive_test.go +++ b/routers/private/hook_post_receive_test.go @@ -39,7 +39,7 @@ func TestHandlePullRequestMerging(t *testing.T) { }, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, []*repo_module.PushUpdateOptions{ {NewCommitID: "01234567"}, }) - assert.Equal(t, 0, len(resp.Body.String())) + assert.Empty(t, resp.Body.String()) pr, err = issues_model.GetPullRequestByID(db.DefaultContext, pr.ID) assert.NoError(t, err) assert.True(t, pr.HasMerged) diff --git a/routers/web/repo/wiki_test.go b/routers/web/repo/wiki_test.go index b81f2ea02e3..958ff802d40 100644 --- a/routers/web/repo/wiki_test.go +++ b/routers/web/repo/wiki_test.go @@ -135,7 +135,7 @@ func TestNewWikiPost(t *testing.T) { NewWikiPost(ctx) assert.EqualValues(t, http.StatusSeeOther, ctx.Resp.Status()) assertWikiExists(t, ctx.Repo.Repository, wiki_service.UserTitleToWebPath("", title)) - assert.Equal(t, wikiContent(t, ctx.Repo.Repository, wiki_service.UserTitleToWebPath("", title)), content) + assert.Equal(t, content, wikiContent(t, ctx.Repo.Repository, wiki_service.UserTitleToWebPath("", title))) } } @@ -194,7 +194,7 @@ func TestEditWikiPost(t *testing.T) { EditWikiPost(ctx) assert.EqualValues(t, http.StatusSeeOther, ctx.Resp.Status()) assertWikiExists(t, ctx.Repo.Repository, wiki_service.UserTitleToWebPath("", title)) - assert.Equal(t, wikiContent(t, ctx.Repo.Repository, wiki_service.UserTitleToWebPath("", title)), content) + assert.Equal(t, content, wikiContent(t, ctx.Repo.Repository, wiki_service.UserTitleToWebPath("", title))) if title != "Home" { assertWikiNotExists(t, ctx.Repo.Repository, "Home") } diff --git a/services/actions/auth_test.go b/services/actions/auth_test.go index 12db2bae565..85e74091052 100644 --- a/services/actions/auth_test.go +++ b/services/actions/auth_test.go @@ -17,19 +17,19 @@ import ( func TestCreateAuthorizationToken(t *testing.T) { var taskID int64 = 23 token, err := CreateAuthorizationToken(taskID, 1, 2) - assert.Nil(t, err) + assert.NoError(t, err) assert.NotEqual(t, "", token) claims := jwt.MapClaims{} _, err = jwt.ParseWithClaims(token, claims, func(t *jwt.Token) (any, error) { return setting.GetGeneralTokenSigningSecret(), nil }) - assert.Nil(t, err) + assert.NoError(t, err) scp, ok := claims["scp"] assert.True(t, ok, "Has scp claim in jwt token") assert.Contains(t, scp, "Actions.Results:1:2") taskIDClaim, ok := claims["TaskID"] assert.True(t, ok, "Has TaskID claim in jwt token") - assert.Equal(t, float64(taskID), taskIDClaim, "Supplied taskid must match stored one") + assert.InDelta(t, float64(taskID), taskIDClaim, 0, "Supplied taskid must match stored one") acClaim, ok := claims["ac"] assert.True(t, ok, "Has ac claim in jwt token") ac, ok := acClaim.(string) @@ -43,14 +43,14 @@ func TestCreateAuthorizationToken(t *testing.T) { func TestParseAuthorizationToken(t *testing.T) { var taskID int64 = 23 token, err := CreateAuthorizationToken(taskID, 1, 2) - assert.Nil(t, err) + assert.NoError(t, err) assert.NotEqual(t, "", token) headers := http.Header{} headers.Set("Authorization", "Bearer "+token) rTaskID, err := ParseAuthorizationToken(&http.Request{ Header: headers, }) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, taskID, rTaskID) } @@ -59,6 +59,6 @@ func TestParseAuthorizationTokenNoAuthHeader(t *testing.T) { rTaskID, err := ParseAuthorizationToken(&http.Request{ Header: headers, }) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, int64(0), rTaskID) } diff --git a/services/auth/oauth2_test.go b/services/auth/oauth2_test.go index 75c231ff7a4..b706847e8e1 100644 --- a/services/auth/oauth2_test.go +++ b/services/auth/oauth2_test.go @@ -28,7 +28,7 @@ func TestUserIDFromToken(t *testing.T) { o := OAuth2{} uid := o.userIDFromToken(context.Background(), token, ds) assert.Equal(t, int64(user_model.ActionsUserID), uid) - assert.Equal(t, ds["IsActionsToken"], true) + assert.Equal(t, true, ds["IsActionsToken"]) assert.Equal(t, ds["ActionsTaskID"], int64(RunningTaskID)) }) } diff --git a/services/auth/source/oauth2/source_sync_test.go b/services/auth/source/oauth2/source_sync_test.go index 25408e8727e..893ed625028 100644 --- a/services/auth/source/oauth2/source_sync_test.go +++ b/services/auth/source/oauth2/source_sync_test.go @@ -64,8 +64,8 @@ func TestSource(t *testing.T) { ok, err := user_model.GetExternalLogin(context.Background(), e) assert.NoError(t, err) assert.True(t, ok) - assert.Equal(t, e.RefreshToken, "refresh") - assert.Equal(t, e.AccessToken, "token") + assert.Equal(t, "refresh", e.RefreshToken) + assert.Equal(t, "token", e.AccessToken) u, err := user_model.GetUserByID(context.Background(), user.ID) assert.NoError(t, err) @@ -89,8 +89,8 @@ func TestSource(t *testing.T) { ok, err := user_model.GetExternalLogin(context.Background(), e) assert.NoError(t, err) assert.True(t, ok) - assert.Equal(t, e.RefreshToken, "") - assert.Equal(t, e.AccessToken, "") + assert.Equal(t, "", e.RefreshToken) + assert.Equal(t, "", e.AccessToken) u, err := user_model.GetUserByID(context.Background(), user.ID) assert.NoError(t, err) diff --git a/services/convert/pull_review_test.go b/services/convert/pull_review_test.go index 68869502802..a1296fafd4e 100644 --- a/services/convert/pull_review_test.go +++ b/services/convert/pull_review_test.go @@ -40,7 +40,7 @@ func Test_ToPullReview(t *testing.T) { user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) prList, err := ToPullReviewList(db.DefaultContext, reviewList, user4) assert.NoError(t, err) - assert.Len(t, prList, 0) + assert.Empty(t, prList) }) t.Run("Admin User", func(t *testing.T) { diff --git a/services/cron/tasks_test.go b/services/cron/tasks_test.go index 979371a0229..ab22403ede7 100644 --- a/services/cron/tasks_test.go +++ b/services/cron/tasks_test.go @@ -12,7 +12,7 @@ import ( ) func TestAddTaskToScheduler(t *testing.T) { - assert.Len(t, scheduler.Jobs(), 0) + assert.Empty(t, scheduler.Jobs()) defer scheduler.Clear() // no seconds diff --git a/services/feed/feed_test.go b/services/feed/feed_test.go index 6f1cb9a969b..1e4d029e18c 100644 --- a/services/feed/feed_test.go +++ b/services/feed/feed_test.go @@ -41,7 +41,7 @@ func TestGetFeeds(t *testing.T) { OnlyPerformedBy: false, }) assert.NoError(t, err) - assert.Len(t, actions, 0) + assert.Empty(t, actions) assert.Equal(t, int64(0), count) } @@ -57,7 +57,7 @@ func TestGetFeedsForRepos(t *testing.T) { IncludePrivate: true, }) assert.NoError(t, err) - assert.Len(t, actions, 0) + assert.Empty(t, actions) assert.Equal(t, int64(0), count) // public repo & no login @@ -119,7 +119,7 @@ func TestGetFeeds2(t *testing.T) { IncludeDeleted: true, }) assert.NoError(t, err) - assert.Len(t, actions, 0) + assert.Empty(t, actions) assert.Equal(t, int64(0), count) } diff --git a/services/gitdiff/gitdiff_test.go b/services/gitdiff/gitdiff_test.go index adcac355a7b..2351c5da879 100644 --- a/services/gitdiff/gitdiff_test.go +++ b/services/gitdiff/gitdiff_test.go @@ -5,7 +5,6 @@ package gitdiff import ( - "fmt" "strconv" "strings" "testing" @@ -643,9 +642,9 @@ func TestGetDiffRangeWithWhitespaceBehavior(t *testing.T) { MaxFiles: setting.Git.MaxGitDiffFiles, WhitespaceBehavior: behavior, }) - assert.NoError(t, err, fmt.Sprintf("Error when diff with %s", behavior)) + assert.NoError(t, err, "Error when diff with %s", behavior) for _, f := range diffs.Files { - assert.True(t, len(f.Sections) > 0, fmt.Sprintf("%s should have sections", f.Name)) + assert.NotEmpty(t, f.Sections, "%s should have sections", f.Name) } } } diff --git a/services/migrations/gitlab_test.go b/services/migrations/gitlab_test.go index 0b9eeaed540..eccfc4def1f 100644 --- a/services/migrations/gitlab_test.go +++ b/services/migrations/gitlab_test.go @@ -50,7 +50,7 @@ func TestGitlabDownloadRepo(t *testing.T) { topics, err := downloader.GetTopics() assert.NoError(t, err) - assert.True(t, len(topics) == 2) + assert.Len(t, topics, 2) assert.EqualValues(t, []string{"migration", "test"}, topics) milestones, err := downloader.GetMilestones() diff --git a/services/org/team_test.go b/services/org/team_test.go index 58b8e0803c4..98addac8f8a 100644 --- a/services/org/team_test.go +++ b/services/org/team_test.go @@ -121,7 +121,7 @@ func TestDeleteTeam(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) accessMode, err := access_model.AccessLevel(db.DefaultContext, user, repo) assert.NoError(t, err) - assert.True(t, accessMode < perm.AccessModeWrite) + assert.Less(t, accessMode, perm.AccessModeWrite) } func TestAddTeamMember(t *testing.T) { diff --git a/services/pull/reviewer_test.go b/services/pull/reviewer_test.go index 1ff373bafb7..b106e2e89f5 100644 --- a/services/pull/reviewer_test.go +++ b/services/pull/reviewer_test.go @@ -30,7 +30,7 @@ func TestRepoGetReviewers(t *testing.T) { // should not include doer and remove the poster reviewers, err = pull_service.GetReviewers(ctx, repo1, 11, 2) assert.NoError(t, err) - assert.Len(t, reviewers, 0) + assert.Empty(t, reviewers) // should not include PR poster, if PR poster would be otherwise eligible reviewers, err = pull_service.GetReviewers(ctx, repo1, 11, 4) @@ -43,7 +43,7 @@ func TestRepoGetReviewers(t *testing.T) { reviewers, err = pull_service.GetReviewers(ctx, repo2, 2, 4) assert.NoError(t, err) assert.Len(t, reviewers, 1) - assert.EqualValues(t, reviewers[0].ID, 2) + assert.EqualValues(t, 2, reviewers[0].ID) // test private org repo repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) diff --git a/services/release/release_test.go b/services/release/release_test.go index 3d0681f1e17..95a54832b9e 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -228,7 +228,7 @@ func TestRelease_Update(t *testing.T) { IsTag: false, } assert.NoError(t, CreateRelease(gitRepo, release, nil, "")) - assert.Greater(t, release.ID, int64(0)) + assert.Positive(t, release.ID) release.IsDraft = false tagName := release.TagName diff --git a/services/repository/archiver/archiver_test.go b/services/repository/archiver/archiver_test.go index 2ab18edf491..1d0c6e513d8 100644 --- a/services/repository/archiver/archiver_test.go +++ b/services/repository/archiver/archiver_test.go @@ -4,7 +4,6 @@ package archiver import ( - "errors" "testing" "time" @@ -121,7 +120,7 @@ func TestArchive_Basic(t *testing.T) { // It's fine to go ahead and set it to nil now. assert.Equal(t, zipReq, zipReq2) - assert.False(t, zipReq == zipReq2) + assert.NotSame(t, zipReq, zipReq2) // Same commit, different compression formats should have different names. // Ideally, the extension would match what we originally requested. @@ -131,5 +130,5 @@ func TestArchive_Basic(t *testing.T) { func TestErrUnknownArchiveFormat(t *testing.T) { err := ErrUnknownArchiveFormat{RequestFormat: "master"} - assert.True(t, errors.Is(err, ErrUnknownArchiveFormat{})) + assert.ErrorIs(t, err, ErrUnknownArchiveFormat{}) } diff --git a/services/repository/license_test.go b/services/repository/license_test.go index 39e9738145c..9d3e0f36e36 100644 --- a/services/repository/license_test.go +++ b/services/repository/license_test.go @@ -65,7 +65,7 @@ func Test_detectLicense(t *testing.T) { result, err := detectLicense(strings.NewReader(tests[2].arg + tests[3].arg + tests[4].arg)) assert.NoError(t, err) t.Run("multiple licenses test", func(t *testing.T) { - assert.Equal(t, 3, len(result)) + assert.Len(t, result, 3) assert.Contains(t, result, tests[2].want[0]) assert.Contains(t, result, tests[3].want[0]) assert.Contains(t, result, tests[4].want[0]) diff --git a/services/repository/transfer_test.go b/services/repository/transfer_test.go index 67799eddcc3..0401701ba55 100644 --- a/services/repository/transfer_test.go +++ b/services/repository/transfer_test.go @@ -103,7 +103,7 @@ func TestRepositoryTransfer(t *testing.T) { assert.NoError(t, models.CreatePendingRepositoryTransfer(db.DefaultContext, doer, user2, repo.ID, nil)) transfer, err = models.GetPendingRepositoryTransfer(db.DefaultContext, repo) - assert.Nil(t, err) + assert.NoError(t, err) assert.NoError(t, transfer.LoadAttributes(db.DefaultContext)) assert.Equal(t, "user2", transfer.Recipient.Name) diff --git a/services/webhook/packagist_test.go b/services/webhook/packagist_test.go index e9b0695baae..f47807fa6e2 100644 --- a/services/webhook/packagist_test.go +++ b/services/webhook/packagist_test.go @@ -25,7 +25,7 @@ func TestPackagistPayload(t *testing.T) { pl, err := pc.Create(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) }) t.Run("Delete", func(t *testing.T) { @@ -33,7 +33,7 @@ func TestPackagistPayload(t *testing.T) { pl, err := pc.Delete(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) }) t.Run("Fork", func(t *testing.T) { @@ -41,7 +41,7 @@ func TestPackagistPayload(t *testing.T) { pl, err := pc.Fork(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) }) t.Run("Push", func(t *testing.T) { @@ -59,12 +59,12 @@ func TestPackagistPayload(t *testing.T) { p.Action = api.HookIssueOpened pl, err := pc.Issue(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) p.Action = api.HookIssueClosed pl, err = pc.Issue(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) }) t.Run("IssueComment", func(t *testing.T) { @@ -72,7 +72,7 @@ func TestPackagistPayload(t *testing.T) { pl, err := pc.IssueComment(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) }) t.Run("PullRequest", func(t *testing.T) { @@ -80,7 +80,7 @@ func TestPackagistPayload(t *testing.T) { pl, err := pc.PullRequest(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) }) t.Run("PullRequestComment", func(t *testing.T) { @@ -88,7 +88,7 @@ func TestPackagistPayload(t *testing.T) { pl, err := pc.IssueComment(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) }) t.Run("Review", func(t *testing.T) { @@ -97,7 +97,7 @@ func TestPackagistPayload(t *testing.T) { pl, err := pc.Review(p, webhook_module.HookEventPullRequestReviewApproved) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) }) t.Run("Repository", func(t *testing.T) { @@ -105,7 +105,7 @@ func TestPackagistPayload(t *testing.T) { pl, err := pc.Repository(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) }) t.Run("Package", func(t *testing.T) { @@ -113,7 +113,7 @@ func TestPackagistPayload(t *testing.T) { pl, err := pc.Package(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) }) t.Run("Wiki", func(t *testing.T) { @@ -122,17 +122,17 @@ func TestPackagistPayload(t *testing.T) { p.Action = api.HookWikiCreated pl, err := pc.Wiki(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) p.Action = api.HookWikiEdited pl, err = pc.Wiki(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) p.Action = api.HookWikiDeleted pl, err = pc.Wiki(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) }) t.Run("Release", func(t *testing.T) { @@ -140,7 +140,7 @@ func TestPackagistPayload(t *testing.T) { pl, err := pc.Release(p) require.NoError(t, err) - require.Equal(t, pl, PackagistPayload{}) + require.Equal(t, PackagistPayload{}, pl) }) } diff --git a/services/webhook/webhook_test.go b/services/webhook/webhook_test.go index 5f5c1462323..63cbce17712 100644 --- a/services/webhook/webhook_test.go +++ b/services/webhook/webhook_test.go @@ -21,11 +21,11 @@ func TestWebhook_GetSlackHook(t *testing.T) { Meta: `{"channel": "foo", "username": "username", "color": "blue"}`, } slackHook := GetSlackHook(w) - assert.Equal(t, *slackHook, SlackMeta{ + assert.Equal(t, SlackMeta{ Channel: "foo", Username: "username", Color: "blue", - }) + }, *slackHook) } func TestPrepareWebhooks(t *testing.T) { diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go index 29e9930538e..6393fc53cc9 100644 --- a/tests/integration/api_actions_artifact_test.go +++ b/tests/integration/api_actions_artifact_test.go @@ -133,7 +133,7 @@ func TestActionsArtifactDownload(t *testing.T) { } } assert.NotNil(t, artifactIdx) - assert.Equal(t, listResp.Value[artifactIdx].Name, "artifact-download") + assert.Equal(t, "artifact-download", listResp.Value[artifactIdx].Name) assert.Contains(t, listResp.Value[artifactIdx].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/workflows/791/artifacts") idx := strings.Index(listResp.Value[artifactIdx].FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/") @@ -374,7 +374,7 @@ func TestActionsArtifactOverwrite(t *testing.T) { break } } - assert.Equal(t, uploadedItem.Name, "artifact-download") + assert.Equal(t, "artifact-download", uploadedItem.Name) idx := strings.Index(uploadedItem.FileContainerResourceURL, "/api/actions_pipeline/_apis/pipelines/") url := uploadedItem.FileContainerResourceURL[idx+1:] + "?itemPath=artifact-download" diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go index 24a041de17e..8a0bd2e4ffa 100644 --- a/tests/integration/api_branch_test.go +++ b/tests/integration/api_branch_test.go @@ -244,7 +244,7 @@ func TestAPIBranchProtection(t *testing.T) { StatusCheckContexts: []string{"test1"}, }, http.StatusOK) bp := testAPIGetBranchProtection(t, "master", http.StatusOK) - assert.Equal(t, true, bp.EnableStatusCheck) + assert.True(t, bp.EnableStatusCheck) assert.Equal(t, []string{"test1"}, bp.StatusCheckContexts) // disable status checks, clear the list of required checks @@ -253,7 +253,7 @@ func TestAPIBranchProtection(t *testing.T) { StatusCheckContexts: []string{}, }, http.StatusOK) bp = testAPIGetBranchProtection(t, "master", http.StatusOK) - assert.Equal(t, false, bp.EnableStatusCheck) + assert.False(t, bp.EnableStatusCheck) assert.Equal(t, []string{}, bp.StatusCheckContexts) testAPIDeleteBranchProtection(t, "master", http.StatusNoContent) diff --git a/tests/integration/api_issue_config_test.go b/tests/integration/api_issue_config_test.go index 745d0cb2a23..ad399654437 100644 --- a/tests/integration/api_issue_config_test.go +++ b/tests/integration/api_issue_config_test.go @@ -47,7 +47,7 @@ func TestAPIRepoGetIssueConfig(t *testing.T) { issueConfig := getIssueConfig(t, owner.Name, repo.Name) assert.True(t, issueConfig.BlankIssuesEnabled) - assert.Len(t, issueConfig.ContactLinks, 0) + assert.Empty(t, issueConfig.ContactLinks) }) t.Run("DisableBlankIssues", func(t *testing.T) { @@ -59,7 +59,7 @@ func TestAPIRepoGetIssueConfig(t *testing.T) { issueConfig := getIssueConfig(t, owner.Name, repo.Name) assert.False(t, issueConfig.BlankIssuesEnabled) - assert.Len(t, issueConfig.ContactLinks, 0) + assert.Empty(t, issueConfig.ContactLinks) }) t.Run("ContactLinks", func(t *testing.T) { @@ -135,7 +135,7 @@ func TestAPIRepoIssueConfigPaths(t *testing.T) { issueConfig := getIssueConfig(t, owner.Name, repo.Name) assert.False(t, issueConfig.BlankIssuesEnabled) - assert.Len(t, issueConfig.ContactLinks, 0) + assert.Empty(t, issueConfig.ContactLinks) _, err = deleteFileInBranch(owner, repo, fullPath, repo.DefaultBranch) assert.NoError(t, err) diff --git a/tests/integration/api_issue_pin_test.go b/tests/integration/api_issue_pin_test.go index 1cff937254b..c1bfa5aa0eb 100644 --- a/tests/integration/api_issue_pin_test.go +++ b/tests/integration/api_issue_pin_test.go @@ -153,7 +153,7 @@ func TestAPIListPinnedIssues(t *testing.T) { var issueList []api.Issue DecodeJSON(t, resp, &issueList) - assert.Equal(t, 1, len(issueList)) + assert.Len(t, issueList, 1) assert.Equal(t, issue.ID, issueList[0].ID) } @@ -169,7 +169,7 @@ func TestAPIListPinnedPullrequests(t *testing.T) { var prList []api.PullRequest DecodeJSON(t, resp, &prList) - assert.Equal(t, 0, len(prList)) + assert.Empty(t, prList) } func TestAPINewPinAllowed(t *testing.T) { diff --git a/tests/integration/api_issue_stopwatch_test.go b/tests/integration/api_issue_stopwatch_test.go index 23066782173..4765787e6f2 100644 --- a/tests/integration/api_issue_stopwatch_test.go +++ b/tests/integration/api_issue_stopwatch_test.go @@ -40,7 +40,7 @@ func TestAPIListStopWatches(t *testing.T) { assert.EqualValues(t, issue.Title, apiWatches[0].IssueTitle) assert.EqualValues(t, repo.Name, apiWatches[0].RepoName) assert.EqualValues(t, repo.OwnerName, apiWatches[0].RepoOwnerName) - assert.Greater(t, apiWatches[0].Seconds, int64(0)) + assert.Positive(t, apiWatches[0].Seconds) } } diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go index 9f75478ebfc..d8394a33d96 100644 --- a/tests/integration/api_issue_test.go +++ b/tests/integration/api_issue_test.go @@ -252,7 +252,7 @@ func TestAPIEditIssue(t *testing.T) { assert.Equal(t, api.StateClosed, apiIssue.State) assert.Equal(t, milestone, apiIssue.Milestone.ID) assert.Equal(t, body, apiIssue.Body) - assert.True(t, apiIssue.Deadline == nil) + assert.Nil(t, apiIssue.Deadline) assert.Equal(t, title, apiIssue.Title) // in database diff --git a/tests/integration/api_keys_test.go b/tests/integration/api_keys_test.go index 89ad1ec0df5..2276b955cfc 100644 --- a/tests/integration/api_keys_test.go +++ b/tests/integration/api_keys_test.go @@ -168,7 +168,7 @@ func TestCreateUserKey(t *testing.T) { resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &fingerprintPublicKeys) - assert.Len(t, fingerprintPublicKeys, 0) + assert.Empty(t, fingerprintPublicKeys) // Fail searching for wrong users key req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/users/%s/keys?fingerprint=%s", "user2", newPublicKey.Fingerprint)). @@ -176,7 +176,7 @@ func TestCreateUserKey(t *testing.T) { resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &fingerprintPublicKeys) - assert.Len(t, fingerprintPublicKeys, 0) + assert.Empty(t, fingerprintPublicKeys) // Now login as user 2 session2 := loginUser(t, "user2") @@ -208,5 +208,5 @@ func TestCreateUserKey(t *testing.T) { resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &fingerprintPublicKeys) - assert.Len(t, fingerprintPublicKeys, 0) + assert.Empty(t, fingerprintPublicKeys) } diff --git a/tests/integration/api_notification_test.go b/tests/integration/api_notification_test.go index abb9852eef4..dc4ba83ecc5 100644 --- a/tests/integration/api_notification_test.go +++ b/tests/integration/api_notification_test.go @@ -120,7 +120,7 @@ func TestAPINotification(t *testing.T) { AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &newStruct) - assert.True(t, newStruct.New > 0) + assert.Positive(t, newStruct.New) // -- mark notifications as read -- req = NewRequest(t, "GET", "/api/v1/notifications?status-types=unread"). @@ -154,7 +154,7 @@ func TestAPINotification(t *testing.T) { AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &newStruct) - assert.True(t, newStruct.New == 0) + assert.Zero(t, newStruct.New) } func TestAPINotificationPUT(t *testing.T) { diff --git a/tests/integration/api_oauth2_apps_test.go b/tests/integration/api_oauth2_apps_test.go index 0ea3dc72ff9..7a17b4ca880 100644 --- a/tests/integration/api_oauth2_apps_test.go +++ b/tests/integration/api_oauth2_apps_test.go @@ -74,9 +74,9 @@ func testAPIListOAuth2Applications(t *testing.T) { DecodeJSON(t, resp, &appList) expectedApp := appList[0] - assert.EqualValues(t, existApp.Name, expectedApp.Name) - assert.EqualValues(t, existApp.ClientID, expectedApp.ClientID) - assert.Equal(t, existApp.ConfidentialClient, expectedApp.ConfidentialClient) + assert.EqualValues(t, expectedApp.Name, existApp.Name) + assert.EqualValues(t, expectedApp.ClientID, existApp.ClientID) + assert.Equal(t, expectedApp.ConfidentialClient, existApp.ConfidentialClient) assert.Len(t, expectedApp.ClientID, 36) assert.Empty(t, expectedApp.ClientSecret) assert.EqualValues(t, existApp.RedirectURIs[0], expectedApp.RedirectURIs[0]) @@ -128,13 +128,13 @@ func testAPIGetOAuth2Application(t *testing.T) { DecodeJSON(t, resp, &app) expectedApp := app - assert.EqualValues(t, existApp.Name, expectedApp.Name) - assert.EqualValues(t, existApp.ClientID, expectedApp.ClientID) - assert.Equal(t, existApp.ConfidentialClient, expectedApp.ConfidentialClient) + assert.EqualValues(t, expectedApp.Name, existApp.Name) + assert.EqualValues(t, expectedApp.ClientID, existApp.ClientID) + assert.Equal(t, expectedApp.ConfidentialClient, existApp.ConfidentialClient) assert.Len(t, expectedApp.ClientID, 36) assert.Empty(t, expectedApp.ClientSecret) assert.Len(t, expectedApp.RedirectURIs, 1) - assert.EqualValues(t, existApp.RedirectURIs[0], expectedApp.RedirectURIs[0]) + assert.EqualValues(t, expectedApp.RedirectURIs[0], existApp.RedirectURIs[0]) unittest.AssertExistsAndLoadBean(t, &auth_model.OAuth2Application{ID: expectedApp.ID, Name: expectedApp.Name}) } diff --git a/tests/integration/api_packages_npm_test.go b/tests/integration/api_packages_npm_test.go index 9c888972ffb..b9660aeeb9a 100644 --- a/tests/integration/api_packages_npm_test.go +++ b/tests/integration/api_packages_npm_test.go @@ -325,7 +325,7 @@ func TestPackageNpm(t *testing.T) { pvs, err = packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeNpm) assert.NoError(t, err) - assert.Len(t, pvs, 0) + assert.Empty(t, pvs) }) }) } diff --git a/tests/integration/api_pull_test.go b/tests/integration/api_pull_test.go index d26b285a1a8..969e1108951 100644 --- a/tests/integration/api_pull_test.go +++ b/tests/integration/api_pull_test.go @@ -48,7 +48,7 @@ func TestAPIViewPulls(t *testing.T) { pull := pulls[0] assert.EqualValues(t, 1, pull.Poster.ID) assert.Len(t, pull.RequestedReviewers, 2) - assert.Len(t, pull.RequestedReviewersTeams, 0) + assert.Empty(t, pull.RequestedReviewersTeams) assert.EqualValues(t, 5, pull.RequestedReviewers[0].ID) assert.EqualValues(t, 6, pull.RequestedReviewers[1].ID) assert.EqualValues(t, 1, pull.ChangedFiles) @@ -83,7 +83,7 @@ func TestAPIViewPulls(t *testing.T) { pull = pulls[1] assert.EqualValues(t, 1, pull.Poster.ID) assert.Len(t, pull.RequestedReviewers, 4) - assert.Len(t, pull.RequestedReviewersTeams, 0) + assert.Empty(t, pull.RequestedReviewersTeams) assert.EqualValues(t, 3, pull.RequestedReviewers[0].ID) assert.EqualValues(t, 4, pull.RequestedReviewers[1].ID) assert.EqualValues(t, 2, pull.RequestedReviewers[2].ID) @@ -120,7 +120,7 @@ func TestAPIViewPulls(t *testing.T) { pull = pulls[2] assert.EqualValues(t, 1, pull.Poster.ID) assert.Len(t, pull.RequestedReviewers, 1) - assert.Len(t, pull.RequestedReviewersTeams, 0) + assert.Empty(t, pull.RequestedReviewersTeams) assert.EqualValues(t, 1, pull.RequestedReviewers[0].ID) assert.EqualValues(t, 0, pull.ChangedFiles) diff --git a/tests/integration/api_repo_git_commits_test.go b/tests/integration/api_repo_git_commits_test.go index 36552062073..c4c626eb496 100644 --- a/tests/integration/api_repo_git_commits_test.go +++ b/tests/integration/api_repo_git_commits_test.go @@ -77,7 +77,7 @@ func TestAPIReposGitCommitList(t *testing.T) { assert.EqualValues(t, "c8e31bc7688741a5287fcde4fbb8fc129ca07027", apiData[1].CommitMeta.SHA) compareCommitFiles(t, []string{"test.csv"}, apiData[1].Files) - assert.EqualValues(t, resp.Header().Get("X-Total"), "2") + assert.EqualValues(t, "2", resp.Header().Get("X-Total")) } func TestAPIReposGitCommitListNotMaster(t *testing.T) { @@ -103,7 +103,7 @@ func TestAPIReposGitCommitListNotMaster(t *testing.T) { assert.EqualValues(t, "5099b81332712fe655e34e8dd63574f503f61811", apiData[2].CommitMeta.SHA) compareCommitFiles(t, []string{"readme.md"}, apiData[2].Files) - assert.EqualValues(t, resp.Header().Get("X-Total"), "3") + assert.EqualValues(t, "3", resp.Header().Get("X-Total")) } func TestAPIReposGitCommitListPage2Empty(t *testing.T) { @@ -121,7 +121,7 @@ func TestAPIReposGitCommitListPage2Empty(t *testing.T) { var apiData []api.Commit DecodeJSON(t, resp, &apiData) - assert.Len(t, apiData, 0) + assert.Empty(t, apiData) } func TestAPIReposGitCommitListDifferentBranch(t *testing.T) { @@ -208,7 +208,7 @@ func TestGetFileHistory(t *testing.T) { assert.Equal(t, "f27c2b2b03dcab38beaf89b0ab4ff61f6de63441", apiData[0].CommitMeta.SHA) compareCommitFiles(t, []string{"readme.md"}, apiData[0].Files) - assert.EqualValues(t, resp.Header().Get("X-Total"), "1") + assert.EqualValues(t, "1", resp.Header().Get("X-Total")) } func TestGetFileHistoryNotOnMaster(t *testing.T) { @@ -229,5 +229,5 @@ func TestGetFileHistoryNotOnMaster(t *testing.T) { assert.Equal(t, "c8e31bc7688741a5287fcde4fbb8fc129ca07027", apiData[0].CommitMeta.SHA) compareCommitFiles(t, []string{"test.csv"}, apiData[0].Files) - assert.EqualValues(t, resp.Header().Get("X-Total"), "1") + assert.EqualValues(t, "1", resp.Header().Get("X-Total")) } diff --git a/tests/integration/api_repo_lfs_locks_test.go b/tests/integration/api_repo_lfs_locks_test.go index 427e0b9fb11..4ba01e6d9bd 100644 --- a/tests/integration/api_repo_lfs_locks_test.go +++ b/tests/integration/api_repo_lfs_locks_test.go @@ -176,6 +176,6 @@ func TestAPILFSLocksLogged(t *testing.T) { resp := session.MakeRequest(t, req, http.StatusOK) var lfsLocks api.LFSLockList DecodeJSON(t, resp, &lfsLocks) - assert.Len(t, lfsLocks.Locks, 0) + assert.Empty(t, lfsLocks.Locks) } } diff --git a/tests/integration/api_repo_teams_test.go b/tests/integration/api_repo_teams_test.go index 558bac81501..07d065b02ba 100644 --- a/tests/integration/api_repo_teams_test.go +++ b/tests/integration/api_repo_teams_test.go @@ -39,7 +39,7 @@ func TestAPIRepoTeams(t *testing.T) { if assert.Len(t, teams, 2) { assert.EqualValues(t, "Owners", teams[0].Name) assert.True(t, teams[0].CanCreateOrgRepo) - assert.True(t, util.SliceSortedEqual(unit.AllUnitKeyNames(), teams[0].Units), fmt.Sprintf("%v == %v", unit.AllUnitKeyNames(), teams[0].Units)) + assert.True(t, util.SliceSortedEqual(unit.AllUnitKeyNames(), teams[0].Units), "%v == %v", unit.AllUnitKeyNames(), teams[0].Units) assert.EqualValues(t, "owner", teams[0].Permission) assert.EqualValues(t, "test_team", teams[1].Name) diff --git a/tests/integration/api_user_orgs_test.go b/tests/integration/api_user_orgs_test.go index c656ded5ae9..9b8726c6c2e 100644 --- a/tests/integration/api_user_orgs_test.go +++ b/tests/integration/api_user_orgs_test.go @@ -76,7 +76,7 @@ func TestUserOrgs(t *testing.T) { // unrelated user should not get private org membership of privateMemberUsername orgs = getUserOrgs(t, unrelatedUsername, privateMemberUsername) - assert.Len(t, orgs, 0) + assert.Empty(t, orgs) // not authenticated call should not be allowed testUserOrgsUnauthenticated(t, privateMemberUsername) diff --git a/tests/integration/api_user_search_test.go b/tests/integration/api_user_search_test.go index e9805a51393..5604a14259b 100644 --- a/tests/integration/api_user_search_test.go +++ b/tests/integration/api_user_search_test.go @@ -49,7 +49,7 @@ func TestAPIUserSearchLoggedIn(t *testing.T) { for _, user := range results.Data { assert.Contains(t, user.UserName, query) assert.NotEmpty(t, user.Email) - assert.True(t, user.Visibility == "public") + assert.Equal(t, "public", user.Visibility) } } @@ -83,7 +83,7 @@ func TestAPIUserSearchSystemUsers(t *testing.T) { var results SearchResults DecodeJSON(t, resp, &results) assert.NotEmpty(t, results.Data) - if assert.EqualValues(t, 1, len(results.Data)) { + if assert.Len(t, results.Data, 1) { user := results.Data[0] assert.EqualValues(t, user.UserName, systemUser.Name) assert.EqualValues(t, user.ID, systemUser.ID) @@ -137,7 +137,7 @@ func TestAPIUserSearchByEmail(t *testing.T) { var results SearchResults DecodeJSON(t, resp, &results) - assert.Equal(t, 1, len(results.Data)) + assert.Len(t, results.Data, 1) assert.Equal(t, query, results.Data[0].Email) // no login user can not search user with private email @@ -155,6 +155,6 @@ func TestAPIUserSearchByEmail(t *testing.T) { resp = MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &results) - assert.Equal(t, 1, len(results.Data)) + assert.Len(t, results.Data, 1) assert.Equal(t, query, results.Data[0].Email) } diff --git a/tests/integration/auth_ldap_test.go b/tests/integration/auth_ldap_test.go index 00ef72c1c3f..1837e827956 100644 --- a/tests/integration/auth_ldap_test.go +++ b/tests/integration/auth_ldap_test.go @@ -265,7 +265,7 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) { htmlDoc := NewHTMLParser(t, resp.Body) tr := htmlDoc.doc.Find("table.table tbody tr") - assert.True(t, tr.Length() == 0) + assert.Equal(t, 0, tr.Length()) } for _, u := range gitLDAPUsers { diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index d1d935da4f9..43b151e0b6e 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -40,10 +40,10 @@ func withKeyFile(t *testing.T, keyname string, callback func(string)) { assert.NoError(t, err) // Setup ssh wrapper - os.Setenv("GIT_SSH", path.Join(tmpDir, "ssh")) - os.Setenv("GIT_SSH_COMMAND", + t.Setenv("GIT_SSH", path.Join(tmpDir, "ssh")) + t.Setenv("GIT_SSH_COMMAND", "ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -o IdentitiesOnly=yes -i \""+keyFile+"\"") - os.Setenv("GIT_SSH_VARIANT", "ssh") + t.Setenv("GIT_SSH_VARIANT", "ssh") callback(keyFile) } diff --git a/tests/integration/git_push_test.go b/tests/integration/git_push_test.go index dc0b52203a9..e68f8bfce2d 100644 --- a/tests/integration/git_push_test.go +++ b/tests/integration/git_push_test.go @@ -6,7 +6,6 @@ package integration import ( "fmt" "net/url" - "strings" "testing" auth_model "code.gitea.io/gitea/models/auth" @@ -211,6 +210,6 @@ func TestPushPullRefs(t *testing.T) { }) assert.Error(t, err) assert.Empty(t, stdout) - assert.False(t, strings.Contains(stderr, "[deleted]"), "stderr: %s", stderr) + assert.NotContains(t, stderr, "[deleted]", "stderr: %s", stderr) }) } diff --git a/tests/integration/gpg_git_test.go b/tests/integration/gpg_git_test.go index 047c049c7f4..acfe70026e1 100644 --- a/tests/integration/gpg_git_test.go +++ b/tests/integration/gpg_git_test.go @@ -29,10 +29,7 @@ func TestGPGGit(t *testing.T) { err := os.Chmod(tmpDir, 0o700) assert.NoError(t, err) - oldGNUPGHome := os.Getenv("GNUPGHOME") - err = os.Setenv("GNUPGHOME", tmpDir) - assert.NoError(t, err) - defer os.Setenv("GNUPGHOME", oldGNUPGHome) + t.Setenv("GNUPGHOME", tmpDir) // Need to create a root key rootKeyPair, err := importTestingKey() diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 6b1b6b8b21a..9b3b2f2b924 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -278,7 +278,7 @@ func getTokenForLoggedInUser(t testing.TB, session *TestSession, scopes ...auth. resp = session.MakeRequest(t, req, http.StatusSeeOther) // Log the flash values on failure - if !assert.Equal(t, resp.Result().Header["Location"], []string{"/user/settings/applications"}) { + if !assert.Equal(t, []string{"/user/settings/applications"}, resp.Result().Header["Location"]) { for _, cookie := range resp.Result().Cookies() { if cookie.Name != gitea_context.CookieNameFlash { continue @@ -453,16 +453,16 @@ func VerifyJSONSchema(t testing.TB, resp *httptest.ResponseRecorder, schemaFile schemaFilePath := filepath.Join(filepath.Dir(setting.AppPath), "tests", "integration", "schemas", schemaFile) _, schemaFileErr := os.Stat(schemaFilePath) - assert.Nil(t, schemaFileErr) + assert.NoError(t, schemaFileErr) schema, schemaFileReadErr := os.ReadFile(schemaFilePath) - assert.Nil(t, schemaFileReadErr) - assert.True(t, len(schema) > 0) + assert.NoError(t, schemaFileReadErr) + assert.NotEmpty(t, schema) nodeinfoSchema := gojsonschema.NewStringLoader(string(schema)) nodeinfoString := gojsonschema.NewStringLoader(resp.Body.String()) result, schemaValidationErr := gojsonschema.Validate(nodeinfoSchema, nodeinfoString) - assert.Nil(t, schemaValidationErr) + assert.NoError(t, schemaValidationErr) assert.Empty(t, result.Errors()) assert.True(t, result.Valid()) } diff --git a/tests/integration/migration-test/migration_test.go b/tests/integration/migration-test/migration_test.go index 627d1f89c41..462cb73eeed 100644 --- a/tests/integration/migration-test/migration_test.go +++ b/tests/integration/migration-test/migration_test.go @@ -59,7 +59,7 @@ func initMigrationTest(t *testing.T) func() { unittest.InitSettings() - assert.True(t, len(setting.RepoRootPath) != 0) + assert.NotEmpty(t, setting.RepoRootPath) assert.NoError(t, unittest.SyncDirs(path.Join(filepath.Dir(setting.AppPath), "tests/gitea-repositories-meta"), setting.RepoRootPath)) assert.NoError(t, git.InitFull(context.Background())) setting.LoadDBSetting() diff --git a/tests/integration/mirror_push_test.go b/tests/integration/mirror_push_test.go index 9ff4669befe..0dd8919bff9 100644 --- a/tests/integration/mirror_push_test.go +++ b/tests/integration/mirror_push_test.go @@ -78,7 +78,7 @@ func testMirrorPush(t *testing.T, u *url.URL) { assert.True(t, doRemovePushMirror(t, session, user.Name, srcRepo.Name, mirrors[0].ID)) mirrors, _, err = repo_model.GetPushMirrorsByRepoID(db.DefaultContext, srcRepo.ID, db.ListOptions{}) assert.NoError(t, err) - assert.Len(t, mirrors, 0) + assert.Empty(t, mirrors) } func testCreatePushMirror(t *testing.T, session *TestSession, owner, repo, address, username, password, interval string) { diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index f177bd3a23b..d6f1ba33ec8 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -89,7 +89,7 @@ func TestAuthorizeRedirectWithExistingGrant(t *testing.T) { u, err := resp.Result().Location() assert.NoError(t, err) assert.Equal(t, "thestate", u.Query().Get("state")) - assert.Truef(t, len(u.Query().Get("code")) > 30, "authorization code '%s' should be longer then 30", u.Query().Get("code")) + assert.Greaterf(t, len(u.Query().Get("code")), 30, "authorization code '%s' should be longer then 30", u.Query().Get("code")) u.RawQuery = "" assert.Equal(t, "https://example.com/xyzzy", u.String()) } @@ -125,8 +125,8 @@ func TestAccessTokenExchange(t *testing.T) { parsed := new(response) assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsed)) - assert.True(t, len(parsed.AccessToken) > 10) - assert.True(t, len(parsed.RefreshToken) > 10) + assert.Greater(t, len(parsed.AccessToken), 10) + assert.Greater(t, len(parsed.RefreshToken), 10) } func TestAccessTokenExchangeWithPublicClient(t *testing.T) { @@ -148,8 +148,8 @@ func TestAccessTokenExchangeWithPublicClient(t *testing.T) { parsed := new(response) assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsed)) - assert.True(t, len(parsed.AccessToken) > 10) - assert.True(t, len(parsed.RefreshToken) > 10) + assert.Greater(t, len(parsed.AccessToken), 10) + assert.Greater(t, len(parsed.RefreshToken), 10) } func TestAccessTokenExchangeJSON(t *testing.T) { @@ -172,8 +172,8 @@ func TestAccessTokenExchangeJSON(t *testing.T) { parsed := new(response) assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsed)) - assert.True(t, len(parsed.AccessToken) > 10) - assert.True(t, len(parsed.RefreshToken) > 10) + assert.Greater(t, len(parsed.AccessToken), 10) + assert.Greater(t, len(parsed.RefreshToken), 10) } func TestAccessTokenExchangeWithoutPKCE(t *testing.T) { @@ -289,8 +289,8 @@ func TestAccessTokenExchangeWithBasicAuth(t *testing.T) { parsed := new(response) assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsed)) - assert.True(t, len(parsed.AccessToken) > 10) - assert.True(t, len(parsed.RefreshToken) > 10) + assert.Greater(t, len(parsed.AccessToken), 10) + assert.Greater(t, len(parsed.RefreshToken), 10) // use wrong client_secret req = NewRequestWithValues(t, "POST", "/login/oauth/access_token", map[string]string{ @@ -449,8 +449,8 @@ func TestOAuthIntrospection(t *testing.T) { parsed := new(response) assert.NoError(t, json.Unmarshal(resp.Body.Bytes(), parsed)) - assert.True(t, len(parsed.AccessToken) > 10) - assert.True(t, len(parsed.RefreshToken) > 10) + assert.Greater(t, len(parsed.AccessToken), 10) + assert.Greater(t, len(parsed.RefreshToken), 10) // successful request with a valid client_id/client_secret and a valid token req = NewRequestWithValues(t, "POST", "/login/oauth/introspect", map[string]string{ diff --git a/tests/integration/org_count_test.go b/tests/integration/org_count_test.go index 6386f53f059..8a33c218bec 100644 --- a/tests/integration/org_count_test.go +++ b/tests/integration/org_count_test.go @@ -130,7 +130,7 @@ func doCheckOrgCounts(username string, orgCounts map[string]int, strict bool, ca calcOrgCounts[org.LowerName] = org.NumRepos count, ok := canonicalCounts[org.LowerName] if ok { - assert.True(t, count == org.NumRepos, "Number of Repos in %s is %d when we expected %d", org.Name, org.NumRepos, count) + assert.Equal(t, count, org.NumRepos, "Number of Repos in %s is %d when we expected %d", org.Name, org.NumRepos, count) } else { assert.False(t, strict, "Did not expect to see %s with count %d", org.Name, org.NumRepos) } diff --git a/tests/integration/pull_compare_test.go b/tests/integration/pull_compare_test.go index ad0be72dcbd..106774aa548 100644 --- a/tests/integration/pull_compare_test.go +++ b/tests/integration/pull_compare_test.go @@ -40,7 +40,7 @@ func TestPullCompare(t *testing.T) { resp = session.MakeRequest(t, req, http.StatusOK) doc := NewHTMLParser(t, resp.Body) editButtonCount := doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length() - assert.Greater(t, editButtonCount, 0, "Expected to find a button to edit a file in the PR diff view but there were none") + assert.Positive(t, editButtonCount, "Expected to find a button to edit a file in the PR diff view but there were none") onGiteaRun(t, func(t *testing.T, u *url.URL) { defer tests.PrepareTestEnv(t)() @@ -58,7 +58,7 @@ func TestPullCompare(t *testing.T) { resp = session.MakeRequest(t, req, http.StatusOK) doc := NewHTMLParser(t, resp.Body) editButtonCount := doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length() - assert.Greater(t, editButtonCount, 0, "Expected to find a button to edit a file in the PR diff view but there were none") + assert.Positive(t, editButtonCount, "Expected to find a button to edit a file in the PR diff view but there were none") repoForked := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: "repo1"}) user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) @@ -71,7 +71,7 @@ func TestPullCompare(t *testing.T) { resp = session.MakeRequest(t, req, http.StatusOK) doc = NewHTMLParser(t, resp.Body) editButtonCount = doc.doc.Find(".diff-file-header-actions a[href*='/_edit/']").Length() - assert.EqualValues(t, editButtonCount, 0, "Expected not to find a button to edit a file in the PR diff view because head repository has been deleted") + assert.EqualValues(t, 0, editButtonCount, "Expected not to find a button to edit a file in the PR diff view because head repository has been deleted") }) } diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index eb3743bc176..1521fcfe8a8 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -661,7 +661,7 @@ func TestPullMergeIndexerNotifier(t *testing.T) { searchIssuesResp := session.MakeRequest(t, NewRequest(t, "GET", link.String()), http.StatusOK) var apiIssuesBefore []*api.Issue DecodeJSON(t, searchIssuesResp, &apiIssuesBefore) - assert.Len(t, apiIssuesBefore, 0) + assert.Empty(t, apiIssuesBefore) // merge the pull request elem := strings.Split(test.RedirectURL(createPullResp), "/") diff --git a/tests/integration/repo_archive_test.go b/tests/integration/repo_archive_test.go index 664b04baf75..c64ad1193d9 100644 --- a/tests/integration/repo_archive_test.go +++ b/tests/integration/repo_archive_test.go @@ -29,5 +29,5 @@ func TestRepoDownloadArchive(t *testing.T) { bs, err := io.ReadAll(resp.Body) assert.NoError(t, err) assert.Empty(t, resp.Header().Get("Content-Encoding")) - assert.Equal(t, 320, len(bs)) + assert.Len(t, bs, 320) } diff --git a/tests/integration/repo_fork_test.go b/tests/integration/repo_fork_test.go index e7c98531795..267fd0d56e5 100644 --- a/tests/integration/repo_fork_test.go +++ b/tests/integration/repo_fork_test.go @@ -43,7 +43,7 @@ func testRepoFork(t *testing.T, session *TestSession, ownerName, repoName, forkO link, exists = htmlDoc.doc.Find(`form.ui.form[action*="/fork"]`).Attr("action") assert.True(t, exists, "The template has changed") _, exists = htmlDoc.doc.Find(fmt.Sprintf(".owner.dropdown .item[data-value=\"%d\"]", forkOwner.ID)).Attr("data-value") - assert.True(t, exists, fmt.Sprintf("Fork owner '%s' is not present in select box", forkOwnerName)) + assert.True(t, exists, "Fork owner '%s' is not present in select box", forkOwnerName) req = NewRequestWithValues(t, "POST", link, map[string]string{ "_csrf": htmlDoc.GetCSRF(), "uid": fmt.Sprintf("%d", forkOwner.ID), diff --git a/tests/integration/repo_generate_test.go b/tests/integration/repo_generate_test.go index 961255cedfb..ff2aa220d3b 100644 --- a/tests/integration/repo_generate_test.go +++ b/tests/integration/repo_generate_test.go @@ -41,7 +41,7 @@ func testRepoGenerate(t *testing.T, session *TestSession, templateID, templateOw link, exists = htmlDoc.doc.Find("form.ui.form[action^=\"/repo/create\"]").Attr("action") assert.True(t, exists, "The template has changed") _, exists = htmlDoc.doc.Find(fmt.Sprintf(".owner.dropdown .item[data-value=\"%d\"]", generateOwner.ID)).Attr("data-value") - assert.True(t, exists, fmt.Sprintf("Generate owner '%s' is not present in select box", generateOwnerName)) + assert.True(t, exists, "Generate owner '%s' is not present in select box", generateOwnerName) req = NewRequestWithValues(t, "POST", link, map[string]string{ "_csrf": htmlDoc.GetCSRF(), "uid": fmt.Sprintf("%d", generateOwner.ID), diff --git a/tests/integration/repo_test.go b/tests/integration/repo_test.go index 7889dfaf3b7..8c568a12723 100644 --- a/tests/integration/repo_test.go +++ b/tests/integration/repo_test.go @@ -228,7 +228,7 @@ func TestViewRepoDirectory(t *testing.T) { repoSummary := htmlDoc.doc.Find(".repository-summary") repoFilesTable := htmlDoc.doc.Find("#repo-files-table") - assert.NotZero(t, len(repoFilesTable.Nodes)) + assert.NotEmpty(t, repoFilesTable.Nodes) assert.Zero(t, description.Length()) assert.Zero(t, repoTopics.Length()) diff --git a/tests/integration/session_test.go b/tests/integration/session_test.go index d47148efa23..b18a25827de 100644 --- a/tests/integration/session_test.go +++ b/tests/integration/session_test.go @@ -28,10 +28,10 @@ func Test_RegenerateSession(t *testing.T) { sess, err := auth.RegenerateSession(db.DefaultContext, "", key) assert.NoError(t, err) assert.EqualValues(t, key, sess.Key) - assert.Len(t, sess.Data, 0) + assert.Empty(t, sess.Data) sess, err = auth.ReadSession(db.DefaultContext, key2) assert.NoError(t, err) assert.EqualValues(t, key2, sess.Key) - assert.Len(t, sess.Data, 0) + assert.Empty(t, sess.Data) } diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index 99e413c6d95..5b6f28d1ff7 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -255,7 +255,7 @@ func TestListStopWatches(t *testing.T) { assert.EqualValues(t, issue.Title, apiWatches[0].IssueTitle) assert.EqualValues(t, repo.Name, apiWatches[0].RepoName) assert.EqualValues(t, repo.OwnerName, apiWatches[0].RepoOwnerName) - assert.Greater(t, apiWatches[0].Seconds, int64(0)) + assert.Positive(t, apiWatches[0].Seconds) } } From b01b0b99a596ff6f879b460aef9a115a2cd811a5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 15 Dec 2024 19:59:18 +0800 Subject: [PATCH 16/19] Refactor some LDAP code (#32849) --- services/auth/source/ldap/source.go | 5 +- .../auth/source/ldap/source_authenticate.go | 12 +- services/auth/source/ldap/source_search.go | 33 +- services/auth/source/ldap/source_sync.go | 20 +- services/auth/source/ldap/util.go | 6 +- tests/integration/auth_ldap_test.go | 366 +++++++++++------- 6 files changed, 264 insertions(+), 178 deletions(-) diff --git a/services/auth/source/ldap/source.go b/services/auth/source/ldap/source.go index dc4cb2c9403..963cdba7c21 100644 --- a/services/auth/source/ldap/source.go +++ b/services/auth/source/ldap/source.go @@ -56,8 +56,7 @@ type Source struct { UserUID string // User Attribute listed in Group SkipLocalTwoFA bool `json:",omitempty"` // Skip Local 2fa for users authenticated with this source - // reference to the authSource - authSource *auth.Source + authSource *auth.Source // reference to the authSource } // FromDB fills up a LDAPConfig from serialized format. @@ -107,7 +106,7 @@ func (source *Source) UseTLS() bool { // ProvidesSSHKeys returns if this source provides SSH Keys func (source *Source) ProvidesSSHKeys() bool { - return len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0 + return strings.TrimSpace(source.AttributeSSHPublicKey) != "" } // SetAuthSource sets the related AuthSource diff --git a/services/auth/source/ldap/source_authenticate.go b/services/auth/source/ldap/source_authenticate.go index 01cb7437205..020e5784dcf 100644 --- a/services/auth/source/ldap/source_authenticate.go +++ b/services/auth/source/ldap/source_authenticate.go @@ -31,13 +31,13 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u return nil, user_model.ErrUserNotExist{Name: loginName} } // Fallback. - if len(sr.Username) == 0 { + if sr.Username == "" { sr.Username = userName } - if len(sr.Mail) == 0 { + if sr.Mail == "" { sr.Mail = fmt.Sprintf("%s@localhost.local", sr.Username) } - isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0 + isAttributeSSHPublicKeySet := strings.TrimSpace(source.AttributeSSHPublicKey) != "" // Update User admin flag if exist if isExist, err := user_model.IsUserExist(ctx, 0, sr.Username); err != nil { @@ -51,11 +51,11 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u } if user != nil && !user.ProhibitLogin { opts := &user_service.UpdateOptions{} - if len(source.AdminFilter) > 0 && user.IsAdmin != sr.IsAdmin { + if source.AdminFilter != "" && user.IsAdmin != sr.IsAdmin { // Change existing admin flag only if AdminFilter option is set opts.IsAdmin = optional.Some(sr.IsAdmin) } - if !sr.IsAdmin && len(source.RestrictedFilter) > 0 && user.IsRestricted != sr.IsRestricted { + if !sr.IsAdmin && source.RestrictedFilter != "" && user.IsRestricted != sr.IsRestricted { // Change existing restricted flag only if RestrictedFilter option is set opts.IsRestricted = optional.Some(sr.IsRestricted) } @@ -99,7 +99,7 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u return user, err } } - if len(source.AttributeAvatar) > 0 { + if source.AttributeAvatar != "" { if err := user_service.UploadAvatar(ctx, user, sr.Avatar); err != nil { return user, err } diff --git a/services/auth/source/ldap/source_search.go b/services/auth/source/ldap/source_search.go index b20c90e791d..fa2c45ce4ad 100644 --- a/services/auth/source/ldap/source_search.go +++ b/services/auth/source/ldap/source_search.go @@ -147,7 +147,7 @@ func bindUser(l *ldap.Conn, userDN, passwd string) error { } func checkAdmin(l *ldap.Conn, ls *Source, userDN string) bool { - if len(ls.AdminFilter) == 0 { + if ls.AdminFilter == "" { return false } log.Trace("Checking admin with filter %s and base %s", ls.AdminFilter, userDN) @@ -169,7 +169,7 @@ func checkAdmin(l *ldap.Conn, ls *Source, userDN string) bool { } func checkRestricted(l *ldap.Conn, ls *Source, userDN string) bool { - if len(ls.RestrictedFilter) == 0 { + if ls.RestrictedFilter == "" { return false } if ls.RestrictedFilter == "*" { @@ -250,8 +250,17 @@ func (source *Source) getUserAttributeListedInGroup(entry *ldap.Entry) string { // SearchEntry : search an LDAP source if an entry (name, passwd) is valid and in the specific filter func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchResult { + if MockedSearchEntry != nil { + return MockedSearchEntry(source, name, passwd, directBind) + } + return realSearchEntry(source, name, passwd, directBind) +} + +var MockedSearchEntry func(source *Source, name, passwd string, directBind bool) *SearchResult + +func realSearchEntry(source *Source, name, passwd string, directBind bool) *SearchResult { // See https://tools.ietf.org/search/rfc4513#section-5.1.2 - if len(passwd) == 0 { + if passwd == "" { log.Debug("Auth. failed for %s, password cannot be empty", name) return nil } @@ -323,17 +332,17 @@ func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchR return nil } - isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0 - isAtributeAvatarSet := len(strings.TrimSpace(source.AttributeAvatar)) > 0 + isAttributeSSHPublicKeySet := strings.TrimSpace(source.AttributeSSHPublicKey) != "" + isAttributeAvatarSet := strings.TrimSpace(source.AttributeAvatar) != "" attribs := []string{source.AttributeUsername, source.AttributeName, source.AttributeSurname, source.AttributeMail} - if len(strings.TrimSpace(source.UserUID)) > 0 { + if strings.TrimSpace(source.UserUID) != "" { attribs = append(attribs, source.UserUID) } if isAttributeSSHPublicKeySet { attribs = append(attribs, source.AttributeSSHPublicKey) } - if isAtributeAvatarSet { + if isAttributeAvatarSet { attribs = append(attribs, source.AttributeAvatar) } @@ -375,7 +384,7 @@ func (source *Source) SearchEntry(name, passwd string, directBind bool) *SearchR isRestricted = checkRestricted(l, source, userDN) } - if isAtributeAvatarSet { + if isAttributeAvatarSet { Avatar = sr.Entries[0].GetRawAttributeValue(source.AttributeAvatar) } @@ -440,14 +449,14 @@ func (source *Source) SearchEntries() ([]*SearchResult, error) { userFilter := fmt.Sprintf(source.Filter, "*") - isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0 - isAtributeAvatarSet := len(strings.TrimSpace(source.AttributeAvatar)) > 0 + isAttributeSSHPublicKeySet := strings.TrimSpace(source.AttributeSSHPublicKey) != "" + isAttributeAvatarSet := strings.TrimSpace(source.AttributeAvatar) != "" attribs := []string{source.AttributeUsername, source.AttributeName, source.AttributeSurname, source.AttributeMail, source.UserUID} if isAttributeSSHPublicKeySet { attribs = append(attribs, source.AttributeSSHPublicKey) } - if isAtributeAvatarSet { + if isAttributeAvatarSet { attribs = append(attribs, source.AttributeAvatar) } @@ -503,7 +512,7 @@ func (source *Source) SearchEntries() ([]*SearchResult, error) { user.SSHPublicKey = v.GetAttributeValues(source.AttributeSSHPublicKey) } - if isAtributeAvatarSet { + if isAttributeAvatarSet { user.Avatar = v.GetRawAttributeValue(source.AttributeAvatar) } diff --git a/services/auth/source/ldap/source_sync.go b/services/auth/source/ldap/source_sync.go index a6d6d2a0f2f..e817bf1fa93 100644 --- a/services/auth/source/ldap/source_sync.go +++ b/services/auth/source/ldap/source_sync.go @@ -25,7 +25,7 @@ import ( func (source *Source) Sync(ctx context.Context, updateExisting bool) error { log.Trace("Doing: SyncExternalUsers[%s]", source.authSource.Name) - isAttributeSSHPublicKeySet := len(strings.TrimSpace(source.AttributeSSHPublicKey)) > 0 + isAttributeSSHPublicKeySet := strings.TrimSpace(source.AttributeSSHPublicKey) != "" var sshKeysNeedUpdate bool // Find all users with this login type - FIXME: Should this be an iterator? @@ -86,26 +86,26 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { return db.ErrCancelledf("During update of %s before completed update of users", source.authSource.Name) default: } - if len(su.Username) == 0 && len(su.Mail) == 0 { + if su.Username == "" && su.Mail == "" { continue } var usr *user_model.User - if len(su.Username) > 0 { + if su.Username != "" { usr = usernameUsers[su.LowerName] } - if usr == nil && len(su.Mail) > 0 { + if usr == nil && su.Mail != "" { usr = mailUsers[strings.ToLower(su.Mail)] } if usr != nil { keepActiveUsers.Add(usr.ID) - } else if len(su.Username) == 0 { + } else if su.Username == "" { // we cannot create the user if su.Username is empty continue } - if len(su.Mail) == 0 { + if su.Mail == "" { su.Mail = fmt.Sprintf("%s@localhost.local", su.Username) } @@ -141,7 +141,7 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { } } - if err == nil && len(source.AttributeAvatar) > 0 { + if err == nil && source.AttributeAvatar != "" { _ = user_service.UploadAvatar(ctx, usr, su.Avatar) } } else if updateExisting { @@ -151,8 +151,8 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { } // Check if user data has changed - if (len(source.AdminFilter) > 0 && usr.IsAdmin != su.IsAdmin) || - (len(source.RestrictedFilter) > 0 && usr.IsRestricted != su.IsRestricted) || + if (source.AdminFilter != "" && usr.IsAdmin != su.IsAdmin) || + (source.RestrictedFilter != "" && usr.IsRestricted != su.IsRestricted) || !strings.EqualFold(usr.Email, su.Mail) || usr.FullName != fullName || !usr.IsActive { @@ -180,7 +180,7 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { } if usr.IsUploadAvatarChanged(su.Avatar) { - if err == nil && len(source.AttributeAvatar) > 0 { + if err == nil && source.AttributeAvatar != "" { _ = user_service.UploadAvatar(ctx, usr, su.Avatar) } } diff --git a/services/auth/source/ldap/util.go b/services/auth/source/ldap/util.go index bd11e2d1193..05ae32c0fd2 100644 --- a/services/auth/source/ldap/util.go +++ b/services/auth/source/ldap/util.go @@ -6,11 +6,11 @@ package ldap // composeFullName composes a firstname surname or username func composeFullName(firstname, surname, username string) string { switch { - case len(firstname) == 0 && len(surname) == 0: + case firstname == "" && surname == "": return username - case len(firstname) == 0: + case firstname == "": return surname - case len(surname) == 0: + case surname == "": return firstname default: return firstname + " " + surname diff --git a/tests/integration/auth_ldap_test.go b/tests/integration/auth_ldap_test.go index 1837e827956..5d372443310 100644 --- a/tests/integration/auth_ldap_test.go +++ b/tests/integration/auth_ldap_test.go @@ -15,13 +15,17 @@ import ( "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/translation" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/auth/source/ldap" org_service "code.gitea.io/gitea/services/org" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type ldapUser struct { @@ -35,87 +39,97 @@ type ldapUser struct { SSHKeys []string } -var gitLDAPUsers = []ldapUser{ - { - UserName: "professor", - Password: "professor", - FullName: "Hubert Farnsworth", - Email: "professor@planetexpress.com", - OtherEmails: []string{"hubert@planetexpress.com"}, - IsAdmin: true, - }, - { - UserName: "hermes", - Password: "hermes", - FullName: "Conrad Hermes", - Email: "hermes@planetexpress.com", - SSHKeys: []string{ - "SHA256:qLY06smKfHoW/92yXySpnxFR10QFrLdRjf/GNPvwcW8", - "SHA256:QlVTuM5OssDatqidn2ffY+Lc4YA5Fs78U+0KOHI51jQ", - "SHA256:DXdeUKYOJCSSmClZuwrb60hUq7367j4fA+udNC3FdRI", +type ldapTestEnv struct { + gitLDAPUsers []ldapUser + otherLDAPUsers []ldapUser + serverHost string + serverPort string +} + +func prepareLdapTestEnv(t *testing.T) *ldapTestEnv { + if os.Getenv("TEST_LDAP") != "1" { + t.Skip() + return nil + } + + gitLDAPUsers := []ldapUser{ + { + UserName: "professor", + Password: "professor", + FullName: "Hubert Farnsworth", + Email: "professor@planetexpress.com", + OtherEmails: []string{"hubert@planetexpress.com"}, + IsAdmin: true, + }, + { + UserName: "hermes", + Password: "hermes", + FullName: "Conrad Hermes", + Email: "hermes@planetexpress.com", + SSHKeys: []string{ + "SHA256:qLY06smKfHoW/92yXySpnxFR10QFrLdRjf/GNPvwcW8", + "SHA256:QlVTuM5OssDatqidn2ffY+Lc4YA5Fs78U+0KOHI51jQ", + "SHA256:DXdeUKYOJCSSmClZuwrb60hUq7367j4fA+udNC3FdRI", + }, + IsAdmin: true, + }, + { + UserName: "fry", + Password: "fry", + FullName: "Philip Fry", + Email: "fry@planetexpress.com", + }, + { + UserName: "leela", + Password: "leela", + FullName: "Leela Turanga", + Email: "leela@planetexpress.com", + IsRestricted: true, + }, + { + UserName: "bender", + Password: "bender", + FullName: "Bender Rodríguez", + Email: "bender@planetexpress.com", }, - IsAdmin: true, - }, - { - UserName: "fry", - Password: "fry", - FullName: "Philip Fry", - Email: "fry@planetexpress.com", - }, - { - UserName: "leela", - Password: "leela", - FullName: "Leela Turanga", - Email: "leela@planetexpress.com", - IsRestricted: true, - }, - { - UserName: "bender", - Password: "bender", - FullName: "Bender Rodríguez", - Email: "bender@planetexpress.com", - }, -} - -var otherLDAPUsers = []ldapUser{ - { - UserName: "zoidberg", - Password: "zoidberg", - FullName: "John Zoidberg", - Email: "zoidberg@planetexpress.com", - }, - { - UserName: "amy", - Password: "amy", - FullName: "Amy Kroker", - Email: "amy@planetexpress.com", - }, -} - -func skipLDAPTests() bool { - return os.Getenv("TEST_LDAP") != "1" -} - -func getLDAPServerHost() string { - host := os.Getenv("TEST_LDAP_HOST") - if len(host) == 0 { - host = "ldap" } - return host -} -func getLDAPServerPort() string { - port := os.Getenv("TEST_LDAP_PORT") - if len(port) == 0 { - port = "389" + otherLDAPUsers := []ldapUser{ + { + UserName: "zoidberg", + Password: "zoidberg", + FullName: "John Zoidberg", + Email: "zoidberg@planetexpress.com", + }, + { + UserName: "amy", + Password: "amy", + FullName: "Amy Kroker", + Email: "amy@planetexpress.com", + }, + } + + return &ldapTestEnv{ + gitLDAPUsers: gitLDAPUsers, + otherLDAPUsers: otherLDAPUsers, + serverHost: util.IfZero(os.Getenv("TEST_LDAP_HOST"), "ldap"), + serverPort: util.IfZero(os.Getenv("TEST_LDAP_PORT"), "389"), } - return port } -func buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap, groupTeamMapRemoval string) map[string]string { +type ldapAuthOptions struct { + attributeUID optional.Option[string] // defaults to "uid" + attributeSSHPublicKey string + groupFilter string + groupTeamMap string + groupTeamMapRemoval string +} + +func (te *ldapTestEnv) buildAuthSourcePayload(csrf string, opts ...ldapAuthOptions) map[string]string { + opt := util.OptionalArg(opts) // Modify user filter to test group filter explicitly userFilter := "(&(objectClass=inetOrgPerson)(memberOf=cn=git,ou=people,dc=planetexpress,dc=com)(uid=%s))" - if groupFilter != "" { + if opt.groupFilter != "" { userFilter = "(&(objectClass=inetOrgPerson)(uid=%s))" } @@ -123,53 +137,47 @@ func buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap "_csrf": csrf, "type": "2", "name": "ldap", - "host": getLDAPServerHost(), - "port": getLDAPServerPort(), + "host": te.serverHost, + "port": te.serverPort, "bind_dn": "uid=gitea,ou=service,dc=planetexpress,dc=com", "bind_password": "password", "user_base": "ou=people,dc=planetexpress,dc=com", "filter": userFilter, "admin_filter": "(memberOf=cn=admin_staff,ou=people,dc=planetexpress,dc=com)", "restricted_filter": "(uid=leela)", - "attribute_username": "uid", + "attribute_username": util.Iif(opt.attributeUID.Has(), opt.attributeUID.Value(), "uid"), "attribute_name": "givenName", "attribute_surname": "sn", "attribute_mail": "mail", - "attribute_ssh_public_key": sshKeyAttribute, + "attribute_ssh_public_key": opt.attributeSSHPublicKey, "is_sync_enabled": "on", "is_active": "on", "groups_enabled": "on", "group_dn": "ou=people,dc=planetexpress,dc=com", "group_member_uid": "member", - "group_filter": groupFilter, - "group_team_map": groupTeamMap, - "group_team_map_removal": groupTeamMapRemoval, + "group_filter": opt.groupFilter, + "group_team_map": opt.groupTeamMap, + "group_team_map_removal": opt.groupTeamMapRemoval, "user_uid": "DN", } } -func addAuthSourceLDAP(t *testing.T, sshKeyAttribute, groupFilter string, groupMapParams ...string) { - groupTeamMapRemoval := "off" - groupTeamMap := "" - if len(groupMapParams) == 2 { - groupTeamMapRemoval = groupMapParams[0] - groupTeamMap = groupMapParams[1] - } +func (te *ldapTestEnv) addAuthSource(t *testing.T, opts ...ldapAuthOptions) { session := loginUser(t, "user1") csrf := GetUserCSRFToken(t, session) - req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", buildAuthSourceLDAPPayload(csrf, sshKeyAttribute, groupFilter, groupTeamMap, groupTeamMapRemoval)) + req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", te.buildAuthSourcePayload(csrf, opts...)) session.MakeRequest(t, req, http.StatusSeeOther) } func TestLDAPUserSignin(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "") + te.addAuthSource(t) - u := gitLDAPUsers[0] + u := te.gitLDAPUsers[0] session := loginUserWithPassword(t, u.UserName, u.Password) req := NewRequest(t, "GET", "/user/settings") @@ -183,8 +191,13 @@ func TestLDAPUserSignin(t *testing.T) { } func TestLDAPAuthChange(t *testing.T) { + te := prepareLdapTestEnv(t) + if te == nil { + return + } + defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "") + te.addAuthSource(t) session := loginUser(t, "user1") req := NewRequest(t, "GET", "/-/admin/auths") @@ -201,34 +214,35 @@ func TestLDAPAuthChange(t *testing.T) { doc = NewHTMLParser(t, resp.Body) csrf := doc.GetCSRF() host, _ := doc.Find(`input[name="host"]`).Attr("value") - assert.Equal(t, host, getLDAPServerHost()) + assert.Equal(t, te.serverHost, host) binddn, _ := doc.Find(`input[name="bind_dn"]`).Attr("value") assert.Equal(t, "uid=gitea,ou=service,dc=planetexpress,dc=com", binddn) - req = NewRequestWithValues(t, "POST", href, buildAuthSourceLDAPPayload(csrf, "", "", "", "off")) + req = NewRequestWithValues(t, "POST", href, te.buildAuthSourcePayload(csrf, ldapAuthOptions{groupTeamMapRemoval: "off"})) session.MakeRequest(t, req, http.StatusSeeOther) req = NewRequest(t, "GET", href) resp = session.MakeRequest(t, req, http.StatusOK) doc = NewHTMLParser(t, resp.Body) host, _ = doc.Find(`input[name="host"]`).Attr("value") - assert.Equal(t, host, getLDAPServerHost()) + assert.Equal(t, te.serverHost, host) binddn, _ = doc.Find(`input[name="bind_dn"]`).Attr("value") assert.Equal(t, "uid=gitea,ou=service,dc=planetexpress,dc=com", binddn) } func TestLDAPUserSync(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } + defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "") + te.addAuthSource(t) err := auth.SyncExternalUsers(context.Background(), true) assert.NoError(t, err) // Check if users exists - for _, gitLDAPUser := range gitLDAPUsers { + for _, gitLDAPUser := range te.gitLDAPUsers { dbUser, err := user_model.GetUserByName(db.DefaultContext, gitLDAPUser.UserName) assert.NoError(t, err) assert.Equal(t, gitLDAPUser.UserName, dbUser.Name) @@ -238,27 +252,28 @@ func TestLDAPUserSync(t *testing.T) { } // Check if no users exist - for _, otherLDAPUser := range otherLDAPUsers { + for _, otherLDAPUser := range te.otherLDAPUsers { _, err := user_model.GetUserByName(db.DefaultContext, otherLDAPUser.UserName) assert.True(t, user_model.IsErrUserNotExist(err)) } } func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } + defer tests.PrepareTestEnv(t)() session := loginUser(t, "user1") csrf := GetUserCSRFToken(t, session) - payload := buildAuthSourceLDAPPayload(csrf, "", "", "", "") + payload := te.buildAuthSourcePayload(csrf) payload["attribute_username"] = "" req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", payload) session.MakeRequest(t, req, http.StatusSeeOther) - for _, u := range gitLDAPUsers { + for _, u := range te.gitLDAPUsers { req := NewRequest(t, "GET", "/-/admin/users?q="+u.UserName) resp := session.MakeRequest(t, req, http.StatusOK) @@ -268,7 +283,7 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) { assert.Equal(t, 0, tr.Length()) } - for _, u := range gitLDAPUsers { + for _, u := range te.gitLDAPUsers { req := NewRequestWithValues(t, "POST", "/user/login", map[string]string{ "_csrf": csrf, "user_name": u.UserName, @@ -277,7 +292,7 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) { MakeRequest(t, req, http.StatusSeeOther) } - auth.SyncExternalUsers(context.Background(), true) + require.NoError(t, auth.SyncExternalUsers(context.Background(), true)) authSource := unittest.AssertExistsAndLoadBean(t, &auth_model.Source{ Name: payload["name"], @@ -285,9 +300,9 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) { unittest.AssertCount(t, &user_model.User{ LoginType: auth_model.LDAP, LoginSource: authSource.ID, - }, len(gitLDAPUsers)) + }, len(te.gitLDAPUsers)) - for _, u := range gitLDAPUsers { + for _, u := range te.gitLDAPUsers { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ Name: u.UserName, }) @@ -296,12 +311,13 @@ func TestLDAPUserSyncWithEmptyUsernameAttribute(t *testing.T) { } func TestLDAPUserSyncWithGroupFilter(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } + defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "(cn=git)") + te.addAuthSource(t, ldapAuthOptions{groupFilter: "(cn=git)"}) // Assert a user not a member of the LDAP group "cn=git" cannot login // This test may look like TestLDAPUserSigninFailed but it is not. @@ -309,20 +325,20 @@ func TestLDAPUserSyncWithGroupFilter(t *testing.T) { // This test is for the case when LDAP user records may not be linked with // all groups the user is a member of, the user filter is modified accordingly inside // the addAuthSourceLDAP based on the value of the groupFilter - u := otherLDAPUsers[0] + u := te.otherLDAPUsers[0] testLoginFailed(t, u.UserName, u.Password, translation.NewLocale("en-US").TrString("form.username_password_incorrect")) - auth.SyncExternalUsers(context.Background(), true) + require.NoError(t, auth.SyncExternalUsers(context.Background(), true)) // Assert members of LDAP group "cn=git" are added - for _, gitLDAPUser := range gitLDAPUsers { + for _, gitLDAPUser := range te.gitLDAPUsers { unittest.BeanExists(t, &user_model.User{ Name: gitLDAPUser.UserName, }) } // Assert everyone else is not added - for _, gitLDAPUser := range otherLDAPUsers { + for _, gitLDAPUser := range te.otherLDAPUsers { unittest.AssertNotExistsBean(t, &user_model.User{ Name: gitLDAPUser.UserName, }) @@ -333,11 +349,11 @@ func TestLDAPUserSyncWithGroupFilter(t *testing.T) { }) ldapConfig := ldapSource.Cfg.(*ldap.Source) ldapConfig.GroupFilter = "(cn=ship_crew)" - auth_model.UpdateSource(db.DefaultContext, ldapSource) + require.NoError(t, auth_model.UpdateSource(db.DefaultContext, ldapSource)) - auth.SyncExternalUsers(context.Background(), true) + require.NoError(t, auth.SyncExternalUsers(context.Background(), true)) - for _, gitLDAPUser := range gitLDAPUsers { + for _, gitLDAPUser := range te.gitLDAPUsers { if gitLDAPUser.UserName == "fry" || gitLDAPUser.UserName == "leela" || gitLDAPUser.UserName == "bender" { // Assert members of the LDAP group "cn-ship_crew" are still active user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ @@ -355,29 +371,31 @@ func TestLDAPUserSyncWithGroupFilter(t *testing.T) { } func TestLDAPUserSigninFailed(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } - defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "") - u := otherLDAPUsers[0] + defer tests.PrepareTestEnv(t)() + te.addAuthSource(t) + + u := te.otherLDAPUsers[0] testLoginFailed(t, u.UserName, u.Password, translation.NewLocale("en-US").TrString("form.username_password_incorrect")) } func TestLDAPUserSSHKeySync(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } - defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "sshPublicKey", "") - auth.SyncExternalUsers(context.Background(), true) + defer tests.PrepareTestEnv(t)() + te.addAuthSource(t, ldapAuthOptions{attributeSSHPublicKey: "sshPublicKey"}) + + require.NoError(t, auth.SyncExternalUsers(context.Background(), true)) // Check if users has SSH keys synced - for _, u := range gitLDAPUsers { + for _, u := range te.gitLDAPUsers { if len(u.SSHKeys) == 0 { continue } @@ -400,18 +418,22 @@ func TestLDAPUserSSHKeySync(t *testing.T) { } func TestLDAPGroupTeamSyncAddMember(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } + defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "", "on", `{"cn=ship_crew,ou=people,dc=planetexpress,dc=com":{"org26": ["team11"]},"cn=admin_staff,ou=people,dc=planetexpress,dc=com": {"non-existent": ["non-existent"]}}`) + te.addAuthSource(t, ldapAuthOptions{ + groupTeamMap: `{"cn=ship_crew,ou=people,dc=planetexpress,dc=com":{"org26": ["team11"]},"cn=admin_staff,ou=people,dc=planetexpress,dc=com": {"non-existent": ["non-existent"]}}`, + groupTeamMapRemoval: "on", + }) org, err := organization.GetOrgByName(db.DefaultContext, "org26") assert.NoError(t, err) team, err := organization.GetTeam(db.DefaultContext, org.ID, "team11") assert.NoError(t, err) - auth.SyncExternalUsers(context.Background(), true) - for _, gitLDAPUser := range gitLDAPUsers { + require.NoError(t, auth.SyncExternalUsers(context.Background(), true)) + for _, gitLDAPUser := range te.gitLDAPUsers { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ Name: gitLDAPUser.UserName, }) @@ -445,19 +467,22 @@ func TestLDAPGroupTeamSyncAddMember(t *testing.T) { } func TestLDAPGroupTeamSyncRemoveMember(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } defer tests.PrepareTestEnv(t)() - addAuthSourceLDAP(t, "", "", "on", `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`) + te.addAuthSource(t, ldapAuthOptions{ + groupTeamMap: `{"cn=dispatch,ou=people,dc=planetexpress,dc=com": {"org26": ["team11"]}}`, + groupTeamMapRemoval: "on", + }) org, err := organization.GetOrgByName(db.DefaultContext, "org26") assert.NoError(t, err) team, err := organization.GetTeam(db.DefaultContext, org.ID, "team11") assert.NoError(t, err) - loginUserWithPassword(t, gitLDAPUsers[0].UserName, gitLDAPUsers[0].Password) + loginUserWithPassword(t, te.gitLDAPUsers[0].UserName, te.gitLDAPUsers[0].Password) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ - Name: gitLDAPUsers[0].UserName, + Name: te.gitLDAPUsers[0].UserName, }) err = organization.AddOrgUser(db.DefaultContext, org.ID, user.ID) assert.NoError(t, err) @@ -470,7 +495,7 @@ func TestLDAPGroupTeamSyncRemoveMember(t *testing.T) { assert.NoError(t, err) assert.True(t, isMember, "User should be member of this team") // assert team member "professor" gets removed from org26 team11 - loginUserWithPassword(t, gitLDAPUsers[0].UserName, gitLDAPUsers[0].Password) + loginUserWithPassword(t, te.gitLDAPUsers[0].UserName, te.gitLDAPUsers[0].Password) isMember, err = organization.IsOrganizationMember(db.DefaultContext, org.ID, user.ID) assert.NoError(t, err) assert.False(t, isMember, "User membership should have been removed from organization") @@ -480,14 +505,67 @@ func TestLDAPGroupTeamSyncRemoveMember(t *testing.T) { } func TestLDAPPreventInvalidGroupTeamMap(t *testing.T) { - if skipLDAPTests() { - t.Skip() + te := prepareLdapTestEnv(t) + if te == nil { return } defer tests.PrepareTestEnv(t)() session := loginUser(t, "user1") csrf := GetUserCSRFToken(t, session) - req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", buildAuthSourceLDAPPayload(csrf, "", "", `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`, "off")) + payload := te.buildAuthSourcePayload(csrf, ldapAuthOptions{groupTeamMap: `{"NOT_A_VALID_JSON"["MISSING_DOUBLE_POINT"]}`, groupTeamMapRemoval: "off"}) + req := NewRequestWithValues(t, "POST", "/-/admin/auths/new", payload) session.MakeRequest(t, req, http.StatusOK) // StatusOK = failed, StatusSeeOther = ok } + +func TestLDAPEmailSignin(t *testing.T) { + te := ldapTestEnv{ + gitLDAPUsers: []ldapUser{ + { + UserName: "u1", + Password: "xx", + FullName: "user 1", + Email: "u1@gitea.com", + }, + }, + serverHost: "mock-host", + serverPort: "mock-port", + } + defer test.MockVariableValue(&ldap.MockedSearchEntry, func(source *ldap.Source, name, passwd string, directBind bool) *ldap.SearchResult { + var u *ldapUser + for _, user := range te.gitLDAPUsers { + if user.Email == name && user.Password == passwd { + u = &user + break + } + } + if u == nil { + return nil + } + result := &ldap.SearchResult{ + Username: u.UserName, + Mail: u.Email, + LowerName: strings.ToLower(u.UserName), + } + nameFields := strings.Split(u.FullName, " ") + result.Name = nameFields[0] + if len(nameFields) > 1 { + result.Surname = nameFields[1] + } + return result + })() + defer tests.PrepareTestEnv(t)() + te.addAuthSource(t) + + u := te.gitLDAPUsers[0] + + session := loginUserWithPassword(t, u.Email, u.Password) + req := NewRequest(t, "GET", "/user/settings") + resp := session.MakeRequest(t, req, http.StatusOK) + + htmlDoc := NewHTMLParser(t, resp.Body) + + assert.Equal(t, u.UserName, htmlDoc.GetInputValueByName("name")) + assert.Equal(t, u.FullName, htmlDoc.GetInputValueByName("full_name")) + assert.Equal(t, u.Email, htmlDoc.Find("#signed-user-email").Text()) +} From 74b06d4f5cc8dd11140a778768d384c4240ecd66 Mon Sep 17 00:00:00 2001 From: silverwind Date: Sun, 15 Dec 2024 17:56:56 +0100 Subject: [PATCH 17/19] Repo file list enhancements (#32835) 1. restore background color 2. fix border radius on top/bottom and on hover 3. parent link is now full-row again, much easier to click 4. parent link now uses directory icon, matching github 5 changed grid layout to remove auto width on file name column which could get too small. 6. mobile layout now shows more of the filename. --------- Co-authored-by: wxiaoguang --- templates/repo/view_list.tmpl | 6 ++-- web_src/css/repo/home-file-list.css | 44 +++++++++++++++++------- web_src/css/themes/theme-gitea-dark.css | 1 + web_src/css/themes/theme-gitea-light.css | 1 + 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/templates/repo/view_list.tmpl b/templates/repo/view_list.tmpl index 0fdb45e574c..2d555e4c2e9 100644 --- a/templates/repo/view_list.tmpl +++ b/templates/repo/view_list.tmpl @@ -5,9 +5,9 @@
{{if and .LatestCommit .LatestCommit.Committer}}{{DateUtils.TimeSince .LatestCommit.Committer.When}}{{end}}
{{if .HasParentPath}} -
- {{svg "octicon-reply"}} .. -
+ + {{svg "octicon-file-directory-fill"}} .. + {{end}} {{range $item := .Files}}
diff --git a/web_src/css/repo/home-file-list.css b/web_src/css/repo/home-file-list.css index 285b823d57a..19ba1f2bcbe 100644 --- a/web_src/css/repo/home-file-list.css +++ b/web_src/css/repo/home-file-list.css @@ -1,12 +1,23 @@ #repo-files-table { width: 100%; display: grid; - grid-template-columns: auto 1fr auto; - border: 1px solid var(--color-light-border); + grid-template-columns: 2fr 3fr auto; + border: 1px solid var(--color-secondary); + background: var(--color-box-body); border-radius: var(--border-radius); margin: 10px 0; /* match the "clone-panel-popup" margin to avoid "visual double-border" */ } +@media (max-width: 767.98px) { + #repo-files-table { + grid-template-columns: auto 1fr auto; + } +} + +#repo-files-table .repo-file-cell.name .svg { + margin-right: 2px; +} + #repo-files-table .svg.octicon-file-directory-fill, #repo-files-table .svg.octicon-file-submodule { color: var(--color-primary); @@ -22,18 +33,28 @@ display: contents; } -#repo-files-table .repo-file-item:hover > .repo-file-cell { - background: var(--color-hover); +#repo-files-table .repo-file-item:hover > .repo-file-cell, +#repo-files-table .parent-link:hover { + background: var(--color-hover-opaque); } #repo-files-table .repo-file-line, #repo-files-table .repo-file-cell { - border-top: 1px solid var(--color-light-border); + border-top: 1px solid var(--color-secondary); padding: 8px 10px; } #repo-files-table .repo-file-line:first-child { border-top: none; + border-radius: var(--border-radius) var(--border-radius) 0 0; +} + +#repo-files-table .repo-file-item:last-child .repo-file-cell:first-child { + border-bottom-left-radius: calc(var(--border-radius) - 1px); +} + +#repo-files-table .repo-file-item:last-child .repo-file-cell:last-child { + border-bottom-right-radius: calc(var(--border-radius) - 1px); } #repo-files-table .repo-file-line { @@ -48,12 +69,17 @@ } #repo-files-table .repo-file-cell.name { - max-width: 300px; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; } +@media (max-width: 767.98px) { + #repo-files-table .repo-file-cell.name { + max-width: 35vw; + } +} + #repo-files-table .repo-file-cell.message { white-space: nowrap; overflow: hidden; @@ -66,9 +92,3 @@ white-space: nowrap; color: var(--color-text-light-1); } - -@media (max-width: 767.98px) { - #repo-files-table .repo-file-cell.name { - max-width: 150px; - } -} diff --git a/web_src/css/themes/theme-gitea-dark.css b/web_src/css/themes/theme-gitea-dark.css index 85de7e210a0..9bc77476976 100644 --- a/web_src/css/themes/theme-gitea-dark.css +++ b/web_src/css/themes/theme-gitea-dark.css @@ -203,6 +203,7 @@ --color-light-mimic-enabled: rgba(0, 0, 0, calc(40 / 255 * 222 / 255 / var(--opacity-disabled))); --color-light-border: #e8f3ff28; --color-hover: #e8f3ff19; + --color-hover-opaque: #21252a; /* TODO: color-mix(in srgb, var(--color-body), var(--color-hover)); */ --color-active: #e8f3ff24; --color-menu: #171a1e; --color-card: #171a1e; diff --git a/web_src/css/themes/theme-gitea-light.css b/web_src/css/themes/theme-gitea-light.css index 0bdfd076d69..d7f9debf900 100644 --- a/web_src/css/themes/theme-gitea-light.css +++ b/web_src/css/themes/theme-gitea-light.css @@ -203,6 +203,7 @@ --color-light-mimic-enabled: rgba(0, 0, 0, calc(6 / 255 * 222 / 255 / var(--opacity-disabled))); --color-light-border: #0000171d; --color-hover: #00001708; + --color-hover-opaque: #f1f3f5; /* TODO: color-mix(in srgb, var(--color-body), var(--color-hover)); */ --color-active: #00001714; --color-menu: #f8f9fb; --color-card: #f8f9fb; From c8ea41b049c887794e4dd87b690b3031b98458b9 Mon Sep 17 00:00:00 2001 From: silverwind Date: Sun, 15 Dec 2024 22:02:32 +0100 Subject: [PATCH 18/19] Fix remaining typescript issues, enable `tsc` (#32840) Fixes 79 typescript errors. Discovered at least two bugs in `notifications.ts`, and I'm pretty sure this feature was at least partially broken and may still be, I don't really know how to test it. After this, only like ~10 typescript errors remain in the codebase but those are harder to solve. --------- Co-authored-by: wxiaoguang --- Makefile | 8 +-- package-lock.json | 62 +++++++++---------- package.json | 4 +- tsconfig.json | 3 +- web_src/js/features/common-issue-list.ts | 11 ++-- .../js/features/comp/ComboMarkdownEditor.ts | 4 +- .../js/features/comp/EasyMDEToolbarActions.ts | 44 ++++++------- web_src/js/features/comp/ReactionSelector.ts | 2 +- web_src/js/features/comp/WebHookEditor.ts | 2 +- web_src/js/features/dropzone.ts | 13 ++-- web_src/js/features/emoji.ts | 2 +- .../js/features/eventsource.sharedworker.ts | 7 ++- web_src/js/features/heatmap.ts | 4 +- web_src/js/features/install.ts | 36 +++++------ web_src/js/features/notification.ts | 31 +++++----- web_src/js/features/oauth2-settings.ts | 8 ++- web_src/js/features/pull-view-file.ts | 4 +- web_src/js/features/repo-editor.ts | 2 +- web_src/js/features/repo-search.ts | 7 ++- .../features/repo-settings-branches.test.ts | 5 +- web_src/js/features/tribute.ts | 1 + web_src/js/globals.d.ts | 15 ++++- web_src/js/modules/tippy.ts | 1 - web_src/js/utils.ts | 10 +-- 24 files changed, 152 insertions(+), 134 deletions(-) diff --git a/Makefile b/Makefile index d5b779f1e59..4889958c3b6 100644 --- a/Makefile +++ b/Makefile @@ -377,12 +377,12 @@ lint-backend-fix: lint-go-fix lint-go-vet lint-editorconfig .PHONY: lint-js lint-js: node_modules npx eslint --color --max-warnings=0 --ext js,ts,vue $(ESLINT_FILES) -# npx vue-tsc + npx vue-tsc .PHONY: lint-js-fix lint-js-fix: node_modules npx eslint --color --max-warnings=0 --ext js,ts,vue $(ESLINT_FILES) --fix -# npx vue-tsc + npx vue-tsc .PHONY: lint-css lint-css: node_modules @@ -451,10 +451,6 @@ lint-templates: .venv node_modules lint-yaml: .venv @poetry run yamllint . -.PHONY: tsc -tsc: - npx vue-tsc - .PHONY: watch watch: @bash tools/watch.sh diff --git a/package-lock.json b/package-lock.json index 4764282f65e..8755cfe06f7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -67,6 +67,7 @@ "devDependencies": { "@eslint-community/eslint-plugin-eslint-comments": "4.4.1", "@playwright/test": "1.49.0", + "@silverwind/vue-tsc": "2.1.13", "@stoplight/spectral-cli": "6.14.2", "@stylistic/eslint-plugin-js": "2.11.0", "@stylistic/stylelint-plugin": "3.1.1", @@ -111,8 +112,7 @@ "type-fest": "4.30.0", "updates": "16.4.0", "vite-string-plugin": "1.3.4", - "vitest": "2.1.8", - "vue-tsc": "2.1.10" + "vitest": "2.1.8" }, "engines": { "node": ">= 18.0.0" @@ -3833,6 +3833,24 @@ "hasInstallScript": true, "license": "Apache-2.0" }, + "node_modules/@silverwind/vue-tsc": { + "version": "2.1.13", + "resolved": "https://registry.npmjs.org/@silverwind/vue-tsc/-/vue-tsc-2.1.13.tgz", + "integrity": "sha512-ejFxz1KZiUGAESbC+eURnjqt0N95qkU9eZU7W15wgF9zV+v2FEu3ZLduuXTC7D/Sg6lL1R/QjPfUbxbAbBQOsw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@volar/typescript": "~2.4.11", + "@vue/language-core": "2.1.10", + "semver": "^7.5.4" + }, + "bin": { + "vue-tsc": "bin/vue-tsc.js" + }, + "peerDependencies": { + "typescript": ">=5.0.0" + } + }, "node_modules/@silverwind/vue3-calendar-heatmap": { "version": "2.0.6", "resolved": "https://registry.npmjs.org/@silverwind/vue3-calendar-heatmap/-/vue3-calendar-heatmap-2.0.6.tgz", @@ -5335,30 +5353,30 @@ } }, "node_modules/@volar/language-core": { - "version": "2.4.10", - "resolved": "https://registry.npmjs.org/@volar/language-core/-/language-core-2.4.10.tgz", - "integrity": "sha512-hG3Z13+nJmGaT+fnQzAkS0hjJRa2FCeqZt6Bd+oGNhUkQ+mTFsDETg5rqUTxyzIh5pSOGY7FHCWUS8G82AzLCA==", + "version": "2.4.11", + "resolved": "https://registry.npmjs.org/@volar/language-core/-/language-core-2.4.11.tgz", + "integrity": "sha512-lN2C1+ByfW9/JRPpqScuZt/4OrUUse57GLI6TbLgTIqBVemdl1wNcZ1qYGEo2+Gw8coYLgCy7SuKqn6IrQcQgg==", "dev": true, "license": "MIT", "dependencies": { - "@volar/source-map": "2.4.10" + "@volar/source-map": "2.4.11" } }, "node_modules/@volar/source-map": { - "version": "2.4.10", - "resolved": "https://registry.npmjs.org/@volar/source-map/-/source-map-2.4.10.tgz", - "integrity": "sha512-OCV+b5ihV0RF3A7vEvNyHPi4G4kFa6ukPmyVocmqm5QzOd8r5yAtiNvaPEjl8dNvgC/lj4JPryeeHLdXd62rWA==", + "version": "2.4.11", + "resolved": "https://registry.npmjs.org/@volar/source-map/-/source-map-2.4.11.tgz", + "integrity": "sha512-ZQpmafIGvaZMn/8iuvCFGrW3smeqkq/IIh9F1SdSx9aUl0J4Iurzd6/FhmjNO5g2ejF3rT45dKskgXWiofqlZQ==", "dev": true, "license": "MIT" }, "node_modules/@volar/typescript": { - "version": "2.4.10", - "resolved": "https://registry.npmjs.org/@volar/typescript/-/typescript-2.4.10.tgz", - "integrity": "sha512-F8ZtBMhSXyYKuBfGpYwqA5rsONnOwAVvjyE7KPYJ7wgZqo2roASqNWUnianOomJX5u1cxeRooHV59N0PhvEOgw==", + "version": "2.4.11", + "resolved": "https://registry.npmjs.org/@volar/typescript/-/typescript-2.4.11.tgz", + "integrity": "sha512-2DT+Tdh88Spp5PyPbqhyoYavYCPDsqbHLFwcUI9K1NlY1YgUJvujGdrqUp0zWxnW7KWNTr3xSpMuv2WnaTKDAw==", "dev": true, "license": "MIT", "dependencies": { - "@volar/language-core": "2.4.10", + "@volar/language-core": "2.4.11", "path-browserify": "^1.0.1", "vscode-uri": "^3.0.8" } @@ -15780,24 +15798,6 @@ } } }, - "node_modules/vue-tsc": { - "version": "2.1.10", - "resolved": "https://registry.npmjs.org/vue-tsc/-/vue-tsc-2.1.10.tgz", - "integrity": "sha512-RBNSfaaRHcN5uqVqJSZh++Gy/YUzryuv9u1aFWhsammDJXNtUiJMNoJ747lZcQ68wUQFx6E73y4FY3D8E7FGMA==", - "dev": true, - "license": "MIT", - "dependencies": { - "@volar/typescript": "~2.4.8", - "@vue/language-core": "2.1.10", - "semver": "^7.5.4" - }, - "bin": { - "vue-tsc": "bin/vue-tsc.js" - }, - "peerDependencies": { - "typescript": ">=5.0.0" - } - }, "node_modules/watchpack": { "version": "2.4.2", "resolved": "https://registry.npmjs.org/watchpack/-/watchpack-2.4.2.tgz", diff --git a/package.json b/package.json index 275ca898e29..61e65c1f43e 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,7 @@ "devDependencies": { "@eslint-community/eslint-plugin-eslint-comments": "4.4.1", "@playwright/test": "1.49.0", + "@silverwind/vue-tsc": "2.1.13", "@stoplight/spectral-cli": "6.14.2", "@stylistic/eslint-plugin-js": "2.11.0", "@stylistic/stylelint-plugin": "3.1.1", @@ -110,8 +111,7 @@ "type-fest": "4.30.0", "updates": "16.4.0", "vite-string-plugin": "1.3.4", - "vitest": "2.1.8", - "vue-tsc": "2.1.10" + "vitest": "2.1.8" }, "browserslist": [ "defaults" diff --git a/tsconfig.json b/tsconfig.json index e006535c02c..7d0316db299 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -7,7 +7,8 @@ ], "compilerOptions": { "target": "es2020", - "module": "nodenext", + "module": "esnext", + "moduleResolution": "bundler", "lib": ["dom", "dom.iterable", "dom.asynciterable", "esnext"], "allowImportingTsExtensions": true, "allowJs": true, diff --git a/web_src/js/features/common-issue-list.ts b/web_src/js/features/common-issue-list.ts index e8a47eabad6..e2073647940 100644 --- a/web_src/js/features/common-issue-list.ts +++ b/web_src/js/features/common-issue-list.ts @@ -7,7 +7,7 @@ const reIssueSharpIndex = /^#(\d+)$/; // eg: "#123" const reIssueOwnerRepoIndex = /^([-.\w]+)\/([-.\w]+)#(\d+)$/; // eg: "{owner}/{repo}#{index}" // if the searchText can be parsed to an "issue goto link", return the link, otherwise return empty string -export function parseIssueListQuickGotoLink(repoLink, searchText) { +export function parseIssueListQuickGotoLink(repoLink: string, searchText: string) { searchText = searchText.trim(); let targetUrl = ''; if (repoLink) { @@ -15,13 +15,12 @@ export function parseIssueListQuickGotoLink(repoLink, searchText) { if (reIssueIndex.test(searchText)) { targetUrl = `${repoLink}/issues/${searchText}`; } else if (reIssueSharpIndex.test(searchText)) { - targetUrl = `${repoLink}/issues/${searchText.substr(1)}`; + targetUrl = `${repoLink}/issues/${searchText.substring(1)}`; } } else { // try to parse it for a global search (eg: "owner/repo#123") - const matchIssueOwnerRepoIndex = searchText.match(reIssueOwnerRepoIndex); - if (matchIssueOwnerRepoIndex) { - const [_, owner, repo, index] = matchIssueOwnerRepoIndex; + const [_, owner, repo, index] = reIssueOwnerRepoIndex.exec(searchText) || []; + if (owner) { targetUrl = `${appSubUrl}/${owner}/${repo}/issues/${index}`; } } @@ -33,7 +32,7 @@ export function initCommonIssueListQuickGoto() { if (!goto) return; const form = goto.closest('form'); - const input = form.querySelector('input[name=q]'); + const input = form.querySelector('input[name=q]'); const repoLink = goto.getAttribute('data-repo-link'); form.addEventListener('submit', (e) => { diff --git a/web_src/js/features/comp/ComboMarkdownEditor.ts b/web_src/js/features/comp/ComboMarkdownEditor.ts index 80eabaa37ae..bba50a1296f 100644 --- a/web_src/js/features/comp/ComboMarkdownEditor.ts +++ b/web_src/js/features/comp/ComboMarkdownEditor.ts @@ -283,8 +283,8 @@ export class ComboMarkdownEditor { ]; } - parseEasyMDEToolbar(EasyMDE, actions) { - this.easyMDEToolbarActions = this.easyMDEToolbarActions || easyMDEToolbarActions(EasyMDE, this); + parseEasyMDEToolbar(easyMde: typeof EasyMDE, actions) { + this.easyMDEToolbarActions = this.easyMDEToolbarActions || easyMDEToolbarActions(easyMde, this); const processed = []; for (const action of actions) { const actionButton = this.easyMDEToolbarActions[action]; diff --git a/web_src/js/features/comp/EasyMDEToolbarActions.ts b/web_src/js/features/comp/EasyMDEToolbarActions.ts index d91dd23d115..ec5c7304bef 100644 --- a/web_src/js/features/comp/EasyMDEToolbarActions.ts +++ b/web_src/js/features/comp/EasyMDEToolbarActions.ts @@ -1,100 +1,102 @@ import {svg} from '../../svg.ts'; +import type EasyMDE from 'easymde'; +import type {ComboMarkdownEditor} from './ComboMarkdownEditor.ts'; -export function easyMDEToolbarActions(EasyMDE, editor) { - const actions = { +export function easyMDEToolbarActions(easyMde: typeof EasyMDE, editor: ComboMarkdownEditor): Record> { + const actions: Record | string> = { '|': '|', 'heading-1': { - action: EasyMDE.toggleHeading1, + action: easyMde.toggleHeading1, icon: svg('octicon-heading'), title: 'Heading 1', }, 'heading-2': { - action: EasyMDE.toggleHeading2, + action: easyMde.toggleHeading2, icon: svg('octicon-heading'), title: 'Heading 2', }, 'heading-3': { - action: EasyMDE.toggleHeading3, + action: easyMde.toggleHeading3, icon: svg('octicon-heading'), title: 'Heading 3', }, 'heading-smaller': { - action: EasyMDE.toggleHeadingSmaller, + action: easyMde.toggleHeadingSmaller, icon: svg('octicon-heading'), title: 'Decrease Heading', }, 'heading-bigger': { - action: EasyMDE.toggleHeadingBigger, + action: easyMde.toggleHeadingBigger, icon: svg('octicon-heading'), title: 'Increase Heading', }, 'bold': { - action: EasyMDE.toggleBold, + action: easyMde.toggleBold, icon: svg('octicon-bold'), title: 'Bold', }, 'italic': { - action: EasyMDE.toggleItalic, + action: easyMde.toggleItalic, icon: svg('octicon-italic'), title: 'Italic', }, 'strikethrough': { - action: EasyMDE.toggleStrikethrough, + action: easyMde.toggleStrikethrough, icon: svg('octicon-strikethrough'), title: 'Strikethrough', }, 'quote': { - action: EasyMDE.toggleBlockquote, + action: easyMde.toggleBlockquote, icon: svg('octicon-quote'), title: 'Quote', }, 'code': { - action: EasyMDE.toggleCodeBlock, + action: easyMde.toggleCodeBlock, icon: svg('octicon-code'), title: 'Code', }, 'link': { - action: EasyMDE.drawLink, + action: easyMde.drawLink, icon: svg('octicon-link'), title: 'Link', }, 'unordered-list': { - action: EasyMDE.toggleUnorderedList, + action: easyMde.toggleUnorderedList, icon: svg('octicon-list-unordered'), title: 'Unordered List', }, 'ordered-list': { - action: EasyMDE.toggleOrderedList, + action: easyMde.toggleOrderedList, icon: svg('octicon-list-ordered'), title: 'Ordered List', }, 'image': { - action: EasyMDE.drawImage, + action: easyMde.drawImage, icon: svg('octicon-image'), title: 'Image', }, 'table': { - action: EasyMDE.drawTable, + action: easyMde.drawTable, icon: svg('octicon-table'), title: 'Table', }, 'horizontal-rule': { - action: EasyMDE.drawHorizontalRule, + action: easyMde.drawHorizontalRule, icon: svg('octicon-horizontal-rule'), title: 'Horizontal Rule', }, 'preview': { - action: EasyMDE.togglePreview, + action: easyMde.togglePreview, icon: svg('octicon-eye'), title: 'Preview', }, 'fullscreen': { - action: EasyMDE.toggleFullScreen, + action: easyMde.toggleFullScreen, icon: svg('octicon-screen-full'), title: 'Fullscreen', }, 'side-by-side': { - action: EasyMDE.toggleSideBySide, + action: easyMde.toggleSideBySide, icon: svg('octicon-columns'), title: 'Side by Side', }, diff --git a/web_src/js/features/comp/ReactionSelector.ts b/web_src/js/features/comp/ReactionSelector.ts index 1e955c7ab40..671bade3bec 100644 --- a/web_src/js/features/comp/ReactionSelector.ts +++ b/web_src/js/features/comp/ReactionSelector.ts @@ -3,7 +3,7 @@ import {fomanticQuery} from '../../modules/fomantic/base.ts'; export function initCompReactionSelector(parent: ParentNode = document) { for (const container of parent.querySelectorAll('.issue-content, .diff-file-body')) { - container.addEventListener('click', async (e) => { + container.addEventListener('click', async (e: MouseEvent & {target: HTMLElement}) => { // there are 2 places for the "reaction" buttons, one is the top-right reaction menu, one is the bottom of the comment const target = e.target.closest('.comment-reaction-button'); if (!target) return; diff --git a/web_src/js/features/comp/WebHookEditor.ts b/web_src/js/features/comp/WebHookEditor.ts index b13a2ffca3d..203396af80a 100644 --- a/web_src/js/features/comp/WebHookEditor.ts +++ b/web_src/js/features/comp/WebHookEditor.ts @@ -23,7 +23,7 @@ export function initCompWebHookEditor() { } // some webhooks (like Gitea) allow to set the request method (GET/POST), and it would toggle the "Content Type" field - const httpMethodInput = document.querySelector('#http_method'); + const httpMethodInput = document.querySelector('#http_method'); if (httpMethodInput) { const updateContentType = function () { const visible = httpMethodInput.value === 'POST'; diff --git a/web_src/js/features/dropzone.ts b/web_src/js/features/dropzone.ts index c9b0149df58..666c6452304 100644 --- a/web_src/js/features/dropzone.ts +++ b/web_src/js/features/dropzone.ts @@ -6,6 +6,7 @@ import {GET, POST} from '../modules/fetch.ts'; import {showErrorToast} from '../modules/toast.ts'; import {createElementFromHTML, createElementFromAttrs} from '../utils/dom.ts'; import {isImageFile, isVideoFile} from '../utils.ts'; +import type {DropzoneFile} from 'dropzone/index.js'; const {csrfToken, i18n} = window.config; @@ -15,14 +16,14 @@ export const DropzoneCustomEventRemovedFile = 'dropzone-custom-removed-file'; export const DropzoneCustomEventUploadDone = 'dropzone-custom-upload-done'; async function createDropzone(el, opts) { - const [{Dropzone}] = await Promise.all([ + const [{default: Dropzone}] = await Promise.all([ import(/* webpackChunkName: "dropzone" */'dropzone'), import(/* webpackChunkName: "dropzone" */'dropzone/dist/dropzone.css'), ]); return new Dropzone(el, opts); } -export function generateMarkdownLinkForAttachment(file, {width, dppx} = {}) { +export function generateMarkdownLinkForAttachment(file, {width, dppx}: {width?: number, dppx?: number} = {}) { let fileMarkdown = `[${file.name}](/attachments/${file.uuid})`; if (isImageFile(file)) { fileMarkdown = `!${fileMarkdown}`; @@ -60,14 +61,14 @@ function addCopyLink(file) { /** * @param {HTMLElement} dropzoneEl */ -export async function initDropzone(dropzoneEl) { +export async function initDropzone(dropzoneEl: HTMLElement) { const listAttachmentsUrl = dropzoneEl.closest('[data-attachment-url]')?.getAttribute('data-attachment-url'); const removeAttachmentUrl = dropzoneEl.getAttribute('data-remove-url'); const attachmentBaseLinkUrl = dropzoneEl.getAttribute('data-link-url'); let disableRemovedfileEvent = false; // when resetting the dropzone (removeAllFiles), disable the "removedfile" event let fileUuidDict = {}; // to record: if a comment has been saved, then the uploaded files won't be deleted from server when clicking the Remove in the dropzone - const opts = { + const opts: Record = { url: dropzoneEl.getAttribute('data-upload-url'), headers: {'X-Csrf-Token': csrfToken}, acceptedFiles: ['*/*', ''].includes(dropzoneEl.getAttribute('data-accepts')) ? null : dropzoneEl.getAttribute('data-accepts'), @@ -88,7 +89,7 @@ export async function initDropzone(dropzoneEl) { // "http://localhost:3000/owner/repo/issues/[object%20Event]" // the reason is that the preview "callback(dataURL)" is assign to "img.onerror" then "thumbnail" uses the error object as the dataURL and generates '' const dzInst = await createDropzone(dropzoneEl, opts); - dzInst.on('success', (file, resp) => { + dzInst.on('success', (file: DropzoneFile & {uuid: string}, resp: any) => { file.uuid = resp.uuid; fileUuidDict[file.uuid] = {submitted: false}; const input = createElementFromAttrs('input', {name: 'files', type: 'hidden', id: `dropzone-file-${resp.uuid}`, value: resp.uuid}); @@ -97,7 +98,7 @@ export async function initDropzone(dropzoneEl) { dzInst.emit(DropzoneCustomEventUploadDone, {file}); }); - dzInst.on('removedfile', async (file) => { + dzInst.on('removedfile', async (file: DropzoneFile & {uuid: string}) => { if (disableRemovedfileEvent) return; dzInst.emit(DropzoneCustomEventRemovedFile, {fileUuid: file.uuid}); diff --git a/web_src/js/features/emoji.ts b/web_src/js/features/emoji.ts index 032a3efe8aa..933aa951c54 100644 --- a/web_src/js/features/emoji.ts +++ b/web_src/js/features/emoji.ts @@ -1,4 +1,4 @@ -import emojis from '../../../assets/emoji.json'; +import emojis from '../../../assets/emoji.json' with {type: 'json'}; const {assetUrlPrefix, customEmojis} = window.config; diff --git a/web_src/js/features/eventsource.sharedworker.ts b/web_src/js/features/eventsource.sharedworker.ts index 62581cf687d..991c92cc8e8 100644 --- a/web_src/js/features/eventsource.sharedworker.ts +++ b/web_src/js/features/eventsource.sharedworker.ts @@ -2,6 +2,11 @@ const sourcesByUrl = {}; const sourcesByPort = {}; class Source { + url: string; + eventSource: EventSource; + listening: Record; + clients: Array; + constructor(url) { this.url = url; this.eventSource = new EventSource(url); @@ -67,7 +72,7 @@ class Source { } } -self.addEventListener('connect', (e) => { +self.addEventListener('connect', (e: Event & {ports: Array}) => { for (const port of e.ports) { port.addEventListener('message', (event) => { if (!self.EventSource) { diff --git a/web_src/js/features/heatmap.ts b/web_src/js/features/heatmap.ts index 69cd069a94f..53eebc93e55 100644 --- a/web_src/js/features/heatmap.ts +++ b/web_src/js/features/heatmap.ts @@ -21,8 +21,8 @@ export function initHeatmap() { // last heatmap tooltip localization attempt https://github.com/go-gitea/gitea/pull/24131/commits/a83761cbbae3c2e3b4bced71e680f44432073ac8 const locale = { heatMapLocale: { - months: new Array(12).fill().map((_, idx) => translateMonth(idx)), - days: new Array(7).fill().map((_, idx) => translateDay(idx)), + months: new Array(12).fill(undefined).map((_, idx) => translateMonth(idx)), + days: new Array(7).fill(undefined).map((_, idx) => translateDay(idx)), on: ' - ', // no correct locale support for it, because in many languages the sentence is not "something on someday" more: el.getAttribute('data-locale-more'), less: el.getAttribute('data-locale-less'), diff --git a/web_src/js/features/install.ts b/web_src/js/features/install.ts index 3defb7904aa..725dcafab08 100644 --- a/web_src/js/features/install.ts +++ b/web_src/js/features/install.ts @@ -22,9 +22,9 @@ function initPreInstall() { mssql: '127.0.0.1:1433', }; - const dbHost = document.querySelector('#db_host'); - const dbUser = document.querySelector('#db_user'); - const dbName = document.querySelector('#db_name'); + const dbHost = document.querySelector('#db_host'); + const dbUser = document.querySelector('#db_user'); + const dbName = document.querySelector('#db_name'); // Database type change detection. document.querySelector('#db_type').addEventListener('change', function () { @@ -48,12 +48,12 @@ function initPreInstall() { }); document.querySelector('#db_type').dispatchEvent(new Event('change')); - const appUrl = document.querySelector('#app_url'); + const appUrl = document.querySelector('#app_url'); if (appUrl.value.includes('://localhost')) { appUrl.value = window.location.href; } - const domain = document.querySelector('#domain'); + const domain = document.querySelector('#domain'); if (domain.value.trim() === 'localhost') { domain.value = window.location.hostname; } @@ -61,43 +61,43 @@ function initPreInstall() { // TODO: better handling of exclusive relations. document.querySelector('#offline-mode input').addEventListener('change', function () { if (this.checked) { - document.querySelector('#disable-gravatar input').checked = true; - document.querySelector('#federated-avatar-lookup input').checked = false; + document.querySelector('#disable-gravatar input').checked = true; + document.querySelector('#federated-avatar-lookup input').checked = false; } }); document.querySelector('#disable-gravatar input').addEventListener('change', function () { if (this.checked) { - document.querySelector('#federated-avatar-lookup input').checked = false; + document.querySelector('#federated-avatar-lookup input').checked = false; } else { - document.querySelector('#offline-mode input').checked = false; + document.querySelector('#offline-mode input').checked = false; } }); document.querySelector('#federated-avatar-lookup input').addEventListener('change', function () { if (this.checked) { - document.querySelector('#disable-gravatar input').checked = false; - document.querySelector('#offline-mode input').checked = false; + document.querySelector('#disable-gravatar input').checked = false; + document.querySelector('#offline-mode input').checked = false; } }); document.querySelector('#enable-openid-signin input').addEventListener('change', function () { if (this.checked) { - if (!document.querySelector('#disable-registration input').checked) { - document.querySelector('#enable-openid-signup input').checked = true; + if (!document.querySelector('#disable-registration input').checked) { + document.querySelector('#enable-openid-signup input').checked = true; } } else { - document.querySelector('#enable-openid-signup input').checked = false; + document.querySelector('#enable-openid-signup input').checked = false; } }); document.querySelector('#disable-registration input').addEventListener('change', function () { if (this.checked) { - document.querySelector('#enable-captcha input').checked = false; - document.querySelector('#enable-openid-signup input').checked = false; + document.querySelector('#enable-captcha input').checked = false; + document.querySelector('#enable-openid-signup input').checked = false; } else { - document.querySelector('#enable-openid-signup input').checked = true; + document.querySelector('#enable-openid-signup input').checked = true; } }); document.querySelector('#enable-captcha input').addEventListener('change', function () { if (this.checked) { - document.querySelector('#disable-registration input').checked = false; + document.querySelector('#disable-registration input').checked = false; } }); } diff --git a/web_src/js/features/notification.ts b/web_src/js/features/notification.ts index 539f7790561..5cdcd967f06 100644 --- a/web_src/js/features/notification.ts +++ b/web_src/js/features/notification.ts @@ -14,25 +14,25 @@ export function initNotificationsTable() { window.addEventListener('pageshow', (e) => { if (e.persisted) { // page was restored from bfcache const table = document.querySelector('#notification_table'); - const unreadCountEl = document.querySelector('.notifications-unread-count'); + const unreadCountEl = document.querySelector('.notifications-unread-count'); let unreadCount = parseInt(unreadCountEl.textContent); for (const item of table.querySelectorAll('.notifications-item[data-remove="true"]')) { item.remove(); unreadCount -= 1; } - unreadCountEl.textContent = unreadCount; + unreadCountEl.textContent = String(unreadCount); } }); // mark clicked unread links for deletion on bfcache restore for (const link of table.querySelectorAll('.notifications-item[data-status="1"] .notifications-link')) { - link.addEventListener('click', (e) => { + link.addEventListener('click', (e : MouseEvent & {target: HTMLElement}) => { e.target.closest('.notifications-item').setAttribute('data-remove', 'true'); }); } } -async function receiveUpdateCount(event) { +async function receiveUpdateCount(event: MessageEvent) { try { const data = JSON.parse(event.data); @@ -50,7 +50,7 @@ export function initNotificationCount() { if (!document.querySelector('.notification_count')) return; let usingPeriodicPoller = false; - const startPeriodicPoller = (timeout, lastCount) => { + const startPeriodicPoller = (timeout: number, lastCount?: number) => { if (timeout <= 0 || !Number.isFinite(timeout)) return; usingPeriodicPoller = true; lastCount = lastCount ?? getCurrentCount(); @@ -72,13 +72,13 @@ export function initNotificationCount() { type: 'start', url: `${window.location.origin}${appSubUrl}/user/events`, }); - worker.port.addEventListener('message', (event) => { + worker.port.addEventListener('message', (event: MessageEvent) => { if (!event.data || !event.data.type) { console.error('unknown worker message event', event); return; } if (event.data.type === 'notification-count') { - const _promise = receiveUpdateCount(event.data); + receiveUpdateCount(event); // no await } else if (event.data.type === 'no-event-source') { // browser doesn't support EventSource, falling back to periodic poller if (!usingPeriodicPoller) startPeriodicPoller(notificationSettings.MinTimeout); @@ -118,10 +118,10 @@ export function initNotificationCount() { } function getCurrentCount() { - return document.querySelector('.notification_count').textContent; + return Number(document.querySelector('.notification_count').textContent ?? '0'); } -async function updateNotificationCountWithCallback(callback, timeout, lastCount) { +async function updateNotificationCountWithCallback(callback: (timeout: number, newCount: number) => void, timeout: number, lastCount: number) { const currentCount = getCurrentCount(); if (lastCount !== currentCount) { callback(notificationSettings.MinTimeout, currentCount); @@ -149,10 +149,9 @@ async function updateNotificationTable() { if (notificationDiv) { try { const params = new URLSearchParams(window.location.search); - params.set('div-only', true); - params.set('sequence-number', ++notificationSequenceNumber); - const url = `${appSubUrl}/notifications?${params.toString()}`; - const response = await GET(url); + params.set('div-only', String(true)); + params.set('sequence-number', String(++notificationSequenceNumber)); + const response = await GET(`${appSubUrl}/notifications?${params.toString()}`); if (!response.ok) { throw new Error('Failed to fetch notification table'); @@ -169,7 +168,7 @@ async function updateNotificationTable() { } } -async function updateNotificationCount() { +async function updateNotificationCount(): Promise { try { const response = await GET(`${appSubUrl}/notifications/new`); @@ -185,9 +184,9 @@ async function updateNotificationCount() { el.textContent = `${data.new}`; } - return `${data.new}`; + return data.new as number; } catch (error) { console.error(error); - return '0'; + return 0; } } diff --git a/web_src/js/features/oauth2-settings.ts b/web_src/js/features/oauth2-settings.ts index 1e62ca00964..a206bc8912f 100644 --- a/web_src/js/features/oauth2-settings.ts +++ b/web_src/js/features/oauth2-settings.ts @@ -1,5 +1,7 @@ export function initOAuth2SettingsDisableCheckbox() { - for (const e of document.querySelectorAll('.disable-setting')) e.addEventListener('change', ({target}) => { - document.querySelector(e.getAttribute('data-target')).classList.toggle('disabled', target.checked); - }); + for (const el of document.querySelectorAll('.disable-setting')) { + el.addEventListener('change', (e: Event & {target: HTMLInputElement}) => { + document.querySelector(e.target.getAttribute('data-target')).classList.toggle('disabled', e.target.checked); + }); + } } diff --git a/web_src/js/features/pull-view-file.ts b/web_src/js/features/pull-view-file.ts index 9a052207d50..36fe4bc4dfb 100644 --- a/web_src/js/features/pull-view-file.ts +++ b/web_src/js/features/pull-view-file.ts @@ -34,7 +34,7 @@ export function countAndUpdateViewedFiles() { export function initViewedCheckboxListenerFor() { for (const form of document.querySelectorAll(`${viewedCheckboxSelector}:not([data-has-viewed-checkbox-listener="true"])`)) { // To prevent double addition of listeners - form.setAttribute('data-has-viewed-checkbox-listener', true); + form.setAttribute('data-has-viewed-checkbox-listener', String(true)); // The checkbox consists of a div containing the real checkbox with its label and the CSRF token, // hence the actual checkbox first has to be found @@ -67,7 +67,7 @@ export function initViewedCheckboxListenerFor() { // Unfortunately, actual forms cause too many problems, hence another approach is needed const files = {}; files[fileName] = this.checked; - const data = {files}; + const data: Record = {files}; const headCommitSHA = form.getAttribute('data-headcommit'); if (headCommitSHA) data.headCommitSHA = headCommitSHA; POST(form.getAttribute('data-link'), {data}); diff --git a/web_src/js/features/repo-editor.ts b/web_src/js/features/repo-editor.ts index 96b08250fb1..32d0b84f4c9 100644 --- a/web_src/js/features/repo-editor.ts +++ b/web_src/js/features/repo-editor.ts @@ -35,7 +35,7 @@ function initEditPreviewTab(elForm: HTMLFormElement) { } export function initRepoEditor() { - const dropzoneUpload = document.querySelector('.page-content.repository.editor.upload .dropzone'); + const dropzoneUpload = document.querySelector('.page-content.repository.editor.upload .dropzone'); if (dropzoneUpload) initDropzone(dropzoneUpload); const editArea = document.querySelector('.page-content.repository.editor textarea#edit_area'); diff --git a/web_src/js/features/repo-search.ts b/web_src/js/features/repo-search.ts index 9cc2dd42239..7f111dce33f 100644 --- a/web_src/js/features/repo-search.ts +++ b/web_src/js/features/repo-search.ts @@ -5,9 +5,10 @@ export function initRepositorySearch() { repositorySearchForm.addEventListener('change', (e: Event & {target: HTMLFormElement}) => { e.preventDefault(); - const formData = new FormData(repositorySearchForm); - const params = new URLSearchParams(formData); - + const params = new URLSearchParams(); + for (const [key, value] of new FormData(repositorySearchForm).entries()) { + params.set(key, value.toString()); + } if (e.target.name === 'clear-filter') { params.delete('archived'); params.delete('fork'); diff --git a/web_src/js/features/repo-settings-branches.test.ts b/web_src/js/features/repo-settings-branches.test.ts index c4609999bef..32ab54e4c2c 100644 --- a/web_src/js/features/repo-settings-branches.test.ts +++ b/web_src/js/features/repo-settings-branches.test.ts @@ -2,6 +2,7 @@ import {beforeEach, describe, expect, test, vi} from 'vitest'; import {initRepoSettingsBranchesDrag} from './repo-settings-branches.ts'; import {POST} from '../modules/fetch.ts'; import {createSortable} from '../modules/sortable.ts'; +import type {SortableEvent} from 'sortablejs'; vi.mock('../modules/fetch.ts', () => ({ POST: vi.fn(), @@ -54,8 +55,8 @@ describe('Repository Branch Settings', () => { vi.mocked(POST).mockResolvedValue({ok: true} as Response); // Mock createSortable to capture and execute the onEnd callback - vi.mocked(createSortable).mockImplementation((_el, options) => { - options.onEnd(); + vi.mocked(createSortable).mockImplementation(async (_el: Element, options) => { + options.onEnd(new Event('SortableEvent') as SortableEvent); return {destroy: vi.fn()}; }); diff --git a/web_src/js/features/tribute.ts b/web_src/js/features/tribute.ts index 44588c00646..fa65bcbb280 100644 --- a/web_src/js/features/tribute.ts +++ b/web_src/js/features/tribute.ts @@ -51,6 +51,7 @@ function makeCollections({mentions, emoji}) { export async function attachTribute(element, {mentions, emoji}) { const {default: Tribute} = await import(/* webpackChunkName: "tribute" */'tributejs'); const collections = makeCollections({mentions, emoji}); + // @ts-expect-error TS2351: This expression is not constructable (strange, why) const tribute = new Tribute({collection: collections, noMatchTemplate: ''}); tribute.attach(element); return tribute; diff --git a/web_src/js/globals.d.ts b/web_src/js/globals.d.ts index 9780a1cf3cf..a5ec29a83f6 100644 --- a/web_src/js/globals.d.ts +++ b/web_src/js/globals.d.ts @@ -8,6 +8,17 @@ declare module '*.css' { export default value; } +declare module '*.vue' { + import type {DefineComponent} from 'vue'; + const component: DefineComponent; + export default component; + // List of named exports from vue components, used to make `tsc` output clean. + // To actually lint .vue files, `vue-tsc` is used because `tsc` can not parse them. + export function initRepoBranchTagSelector(selector: string): void; + export function initDashboardRepoList(): void; + export function initRepositoryActionView(): void; +} + declare let __webpack_public_path__: string; declare module 'htmx.org/dist/htmx.esm.js' { @@ -16,8 +27,8 @@ declare module 'htmx.org/dist/htmx.esm.js' { } declare module 'uint8-to-base64' { - export function encode(arrayBuffer: ArrayBuffer): string; - export function decode(base64str: string): ArrayBuffer; + export function encode(arrayBuffer: Uint8Array): string; + export function decode(base64str: string): Uint8Array; } declare module 'swagger-ui-dist/swagger-ui-es-bundle.js' { diff --git a/web_src/js/modules/tippy.ts b/web_src/js/modules/tippy.ts index ce0b3cbc398..4e7f1ac0933 100644 --- a/web_src/js/modules/tippy.ts +++ b/web_src/js/modules/tippy.ts @@ -16,7 +16,6 @@ export function createTippy(target: Element, opts: TippyOpts = {}): Instance { // because we should use our own wrapper functions to handle them, do not let the user override them const {onHide, onShow, onDestroy, role, theme, arrow, ...other} = opts; - // @ts-expect-error: wrong type derived by typescript const instance: Instance = tippy(target, { appendTo: document.body, animation: false, diff --git a/web_src/js/utils.ts b/web_src/js/utils.ts index bd872f094ca..997a4d1ff3f 100644 --- a/web_src/js/utils.ts +++ b/web_src/js/utils.ts @@ -134,16 +134,16 @@ export function toAbsoluteUrl(url: string): string { return `${window.location.origin}${url}`; } -// Encode an ArrayBuffer into a URLEncoded base64 string. -export function encodeURLEncodedBase64(arrayBuffer: ArrayBuffer): string { - return encode(arrayBuffer) +// Encode an Uint8Array into a URLEncoded base64 string. +export function encodeURLEncodedBase64(uint8Array: Uint8Array): string { + return encode(uint8Array) .replace(/\+/g, '-') .replace(/\//g, '_') .replace(/=/g, ''); } -// Decode a URLEncoded base64 to an ArrayBuffer. -export function decodeURLEncodedBase64(base64url: string): ArrayBuffer { +// Decode a URLEncoded base64 to an Uint8Array. +export function decodeURLEncodedBase64(base64url: string): Uint8Array { return decode(base64url .replace(/_/g, '/') .replace(/-/g, '+')); From 42090844ed2de5e615abc6ece351c152d3344295 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 15 Dec 2024 13:38:39 -0800 Subject: [PATCH 19/19] Fix bug on action list deleted branch (#32848) Fix https://github.com/go-gitea/gitea/issues/32761#issuecomment-2540946064 --------- Co-authored-by: wxiaoguang --- models/fixtures/action_run.yml | 19 +++++++++++++++++++ models/fixtures/branch.yml | 12 ++++++++++++ routers/web/repo/actions/actions.go | 9 +++++---- routers/web/repo/actions/actions_test.go | 22 ++++++++++++++++++++++ routers/web/repo/actions/main_test.go | 14 ++++++++++++++ 5 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 routers/web/repo/actions/main_test.go diff --git a/models/fixtures/action_run.yml b/models/fixtures/action_run.yml index 0747c46d2f0..1db849352f2 100644 --- a/models/fixtures/action_run.yml +++ b/models/fixtures/action_run.yml @@ -55,3 +55,22 @@ updated: 1683636626 need_approval: 0 approved_by: 0 +- + id: 794 + title: "job output" + repo_id: 4 + owner_id: 1 + workflow_id: "test.yaml" + index: 190 + trigger_user_id: 1 + ref: "refs/heads/test" + commit_sha: "c2d72f548424103f01ee1dc02889c1e2bff816b0" + event: "push" + is_fork_pull_request: 0 + status: 1 + started: 1683636528 + stopped: 1683636626 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index c7bdff77339..17b1869ab63 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -81,3 +81,15 @@ is_deleted: false deleted_by_id: 0 deleted_unix: 0 + +- + id: 15 + repo_id: 4 + name: 'master' + commit_id: 'c7cd3cd144e6d23c9d6f3d07e52b2c1a956e0338' + commit_message: 'add Readme' + commit_time: 1588147171 + pusher_id: 13 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index 7ed37ea26b2..1de18359365 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -5,6 +5,7 @@ package actions import ( "bytes" + stdCtx "context" "fmt" "net/http" "slices" @@ -245,7 +246,7 @@ func List(ctx *context.Context) { return } - if err := loadIsRefDeleted(ctx, runs); err != nil { + if err := loadIsRefDeleted(ctx, ctx.Repo.Repository.ID, runs); err != nil { log.Error("LoadIsRefDeleted", err) } @@ -273,7 +274,7 @@ func List(ctx *context.Context) { // loadIsRefDeleted loads the IsRefDeleted field for each run in the list. // TODO: move this function to models/actions/run_list.go but now it will result in a circular import. -func loadIsRefDeleted(ctx *context.Context, runs actions_model.RunList) error { +func loadIsRefDeleted(ctx stdCtx.Context, repoID int64, runs actions_model.RunList) error { branches := make(container.Set[string], len(runs)) for _, run := range runs { refName := git.RefName(run.Ref) @@ -285,14 +286,14 @@ func loadIsRefDeleted(ctx *context.Context, runs actions_model.RunList) error { return nil } - branchInfos, err := git_model.GetBranches(ctx, ctx.Repo.Repository.ID, branches.Values(), false) + branchInfos, err := git_model.GetBranches(ctx, repoID, branches.Values(), false) if err != nil { return err } branchSet := git_model.BranchesToNamesSet(branchInfos) for _, run := range runs { refName := git.RefName(run.Ref) - if refName.IsBranch() && !branchSet.Contains(run.Ref) { + if refName.IsBranch() && !branchSet.Contains(refName.ShortName()) { run.IsRefDeleted = true } } diff --git a/routers/web/repo/actions/actions_test.go b/routers/web/repo/actions/actions_test.go index 194704d14eb..6a976ed65c0 100644 --- a/routers/web/repo/actions/actions_test.go +++ b/routers/web/repo/actions/actions_test.go @@ -7,6 +7,10 @@ import ( "strings" "testing" + actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/models/db" + unittest "code.gitea.io/gitea/models/unittest" + act_model "github.com/nektos/act/pkg/model" "github.com/stretchr/testify/assert" ) @@ -154,3 +158,21 @@ func TestReadWorkflow_WorkflowDispatchConfig(t *testing.T) { Type: "boolean", }, workflowDispatch.Inputs[2]) } + +func Test_loadIsRefDeleted(t *testing.T) { + unittest.PrepareTestEnv(t) + + runs, total, err := db.FindAndCount[actions_model.ActionRun](db.DefaultContext, + actions_model.FindRunOptions{RepoID: 4, Ref: "refs/heads/test"}) + assert.NoError(t, err) + assert.Len(t, runs, 1) + assert.EqualValues(t, 1, total) + for _, run := range runs { + assert.False(t, run.IsRefDeleted) + } + + assert.NoError(t, loadIsRefDeleted(db.DefaultContext, 4, runs)) + for _, run := range runs { + assert.True(t, run.IsRefDeleted) + } +} diff --git a/routers/web/repo/actions/main_test.go b/routers/web/repo/actions/main_test.go new file mode 100644 index 00000000000..a82f9c6672e --- /dev/null +++ b/routers/web/repo/actions/main_test.go @@ -0,0 +1,14 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +import ( + "testing" + + "code.gitea.io/gitea/models/unittest" +) + +func TestMain(m *testing.M) { + unittest.MainTest(m) +}