Skip to content

Improve testing init, clean up webhook tests#37412

Merged
lunny merged 34 commits intomainfrom
copilot/remove-text-fixtures-webhook-yml
Apr 25, 2026
Merged

Improve testing init, clean up webhook tests#37412
lunny merged 34 commits intomainfrom
copilot/remove-text-fixtures-webhook-yml

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 25, 2026

Avoid webhook test fixtures affect other tests (be triggered)

Also fixed more testing problems including path init, global config pollution & conflict

Copilot AI and others added 3 commits April 25, 2026 04:18
…t in tests

Agent-Logs-Url: https://github.com/go-gitea/gitea/sessions/52d595da-6991-440e-9cae-a4e070410229

Co-authored-by: wxiaoguang <2114189+wxiaoguang@users.noreply.github.com>
…areWebhookTestData

Agent-Logs-Url: https://github.com/go-gitea/gitea/sessions/d187e3a5-2c68-4115-a4d1-d2bda8f54004

Co-authored-by: wxiaoguang <2114189+wxiaoguang@users.noreply.github.com>
…se HookRepo1Inactive in delete test

Agent-Logs-Url: https://github.com/go-gitea/gitea/sessions/d187e3a5-2c68-4115-a4d1-d2bda8f54004

Co-authored-by: wxiaoguang <2114189+wxiaoguang@users.noreply.github.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 25, 2026
@wxiaoguang wxiaoguang changed the title models/webhook: replace CreateWebhook with db.Insert in tests, consolidate fixture setup into prepareWebhookTestData Clean up webhook tests Apr 25, 2026
@wxiaoguang wxiaoguang marked this pull request as ready for review April 25, 2026 05:50
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 25, 2026
@wxiaoguang wxiaoguang added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Apr 25, 2026
@wxiaoguang wxiaoguang changed the title Clean up webhook tests Improve testing init, clean up webhook tests Apr 25, 2026
@wxiaoguang wxiaoguang requested a review from Copilot April 25, 2026 07:56
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR focuses on making tests more isolated and less fixture-dependent, particularly around webhooks and test environment initialization, and restructures some CLI tests to reduce configuration/global-state conflicts.

Changes:

  • Removes webhook/hook task fixture data and updates webhook-related tests to create their own records.
  • Adjusts unit/integration tests to avoid relying on existing webhook rows/hook-task counts and to use JSON redirect parsing helpers.
  • Refactors CLI test coverage by moving config/path parsing tests into a separate package and exporting a helper used by those tests.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
tests/integration/pull_merge_test.go Adds explicit webhook setup/assertion helpers and improves redirect/JSON decoding usage in PR merge tests.
services/webhook/webhook_test.go Reworks webhook service tests to prepare their own webhooks instead of relying on fixtures; groups related cases under one top-level test.
services/webhook/deliver.go Simplifies default HTTP method handling for webhook delivery.
routers/api/v1/repo/hook_test.go Inserts a webhook within the test and uses the generated ID instead of a fixture ID.
modules/setting/testenv.go Changes test env config initialization to write config content into the test config file.
models/webhook/webhook_test.go Reworks webhook model tests to rely on package setup data rather than fixture IDs.
models/webhook/webhook_system_test.go Updates expectations to use loaded beans instead of fixed IDs and switches some assertions to require.
models/webhook/main_test.go Adds a package-level SetUp hook to populate webhook test data.
models/unittest/testdb.go Changes test init to configure APP_DATA_PATH via generated config content before SetupGiteaTestEnv.
models/fixtures/webhook.yml Replaces webhook fixtures with an empty list and guidance comment.
models/fixtures/hook_task.yml Replaces hook task fixtures with an empty list and guidance comment.
cmd/main_test.go Replaces prior CLI/config tests with a focused default-command validation test.
cmd/main.go Exports the global-flag preparation helper and updates internal usage.
cmd/helper.go Adds shared CLI helper functions (extracted/centralized logic).
cmd/cmdtest/cmd_test.go Adds a separate-package CLI test suite that exercises config/path parsing behaviors.
cmd/cmd_test.go Removes the old default-command test file (moved/merged into cmd/main_test.go).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread models/unittest/testdb.go Outdated
Comment thread cmd/main.go
Comment thread cmd/main_test.go
Comment thread models/webhook/webhook_test.go
Comment thread models/webhook/webhook_test.go
Comment thread cmd/cmdtest/cmd_test.go Outdated
Comment thread modules/setting/testenv.go
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated 13 comments.

Comments suppressed due to low confidence (1)

modules/setting/testenv.go:69

  • initGiteaConf builds CustomConf using the current AppWorkPath, but AppWorkPath is still the test-binary directory at this point (and you no longer override it to the source root). When GITEA_TEST_CONF is a relative path like tests/sqlite.ini, this will resolve to a non-existent file and tests may run with an empty config. Consider resolving relative GITEA_TEST_CONF against *giteaTestSourceRoot (or set the test work path before computing CustomConf).
	initGiteaConf := func() string {
		// giteaConf (GITEA_CONF) must be relative because it is used in the git hooks as "$GITEA_ROOT/$GITEA_CONF"
		giteaConf := os.Getenv("GITEA_TEST_CONF")
		if giteaConf == "" {
			// if no GITEA_TEST_CONF, then it is in unit test, use a temp (non-existing / empty) config file
			// do not really use such config file, the test can run concurrently, using the same config file will cause data-race between tests
			giteaConf = "custom/conf/app-test-tmp.ini"
			customConfBuiltin = filepath.Join(AppWorkPath, giteaConf)
			CustomConf = customConfBuiltin
			_ = os.Remove(CustomConf)
		} else {
			// CustomConf must be absolute path to make tests pass,
			CustomConf = filepath.Join(AppWorkPath, giteaConf)
		}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread models/webhook/webhook_system_test.go
Comment thread models/webhook/webhook_test.go
Comment thread modules/git/git.go Outdated
Comment thread cmd/main.go
Comment thread tests/integration/repo_webhook_test.go
Comment thread tests/integration/pull_merge_test.go
Comment thread services/webhook/webhook_test.go
Comment thread modules/queue/manager_test.go
Comment thread routers/api/v1/repo/hook_test.go
Comment thread models/webhook/webhook_test.go
@wxiaoguang wxiaoguang marked this pull request as ready for review April 25, 2026 12:42
@wxiaoguang wxiaoguang marked this pull request as draft April 25, 2026 13:18
@wxiaoguang wxiaoguang marked this pull request as ready for review April 25, 2026 14:08
Comment thread cmd/cmdtest/cmd_test.go Outdated
Signed-off-by: silverwind <me@silverwind.io>
Comment thread cmd/main_test.go Outdated
Signed-off-by: silverwind <me@silverwind.io>
Comment thread cmd/cmdtest/cmd_test.go
Signed-off-by: silverwind <me@silverwind.io>
Comment thread cmd/cmdtest/cmd_test.go Outdated
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 25, 2026
@lunny lunny enabled auto-merge (squash) April 25, 2026 18:47
@lunny lunny merged commit 9b9fb95 into main Apr 25, 2026
30 checks passed
@lunny lunny deleted the copilot/remove-text-fixtures-webhook-yml branch April 25, 2026 18:55
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 25, 2026
@silverwind silverwind added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Apr 26, 2026
silverwind added a commit to McMichalK/gitea that referenced this pull request Apr 26, 2026
* origin/main: (176 commits)
  Refactor pull request view (3) (go-gitea#37439)
  Update 1.26.1 changelog in main (go-gitea#37442)
  Make GetPossibleUserByID can handle deleted user (go-gitea#37430)
  Fix fetch action redirect (go-gitea#37437)
  Refactor integration test DecodeJSON calls to use generic return value (go-gitea#37432)
  Integrate renovate bot for all dependency updates (go-gitea#37050)
  Refactor pull request view (2) (go-gitea#37428)
  Use MarkLongPolling instead of hard-coded route path (go-gitea#37427)
  Optimize CI caches (go-gitea#37387)
  Update AGENTS.md (go-gitea#37420)
  Update Nix flake (go-gitea#37425)
  [skip ci] Updated translations via Crowdin
  remove excessive quote from terraform instructions (go-gitea#37424)
  Improve testing init, clean up webhook tests (go-gitea#37412)
  Fix color regressions, add `priority` color (go-gitea#37417)
  [skip ci] Updated translations via Crowdin
  Stabilize e2e logout propagation test (go-gitea#37403)
  refactor: serve site manifest via `/assets/site-manifest.json` endpoint (go-gitea#37405)
  feat(security): set X-Content-Type-Options: nosniff by default (go-gitea#37354)
  Refactor pull request view (1) (go-gitea#37380)
  ...

# Conflicts:
#	templates/repo/diff/box.tmpl
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 27, 2026
* main: (33 commits)
  refactor: use named `Permission` field in `Repository` struct instead of anonymous embedding (go-gitea#37441)
  Refactor pull request view (3) (go-gitea#37439)
  Update 1.26.1 changelog in main (go-gitea#37442)
  Make GetPossibleUserByID can handle deleted user (go-gitea#37430)
  Fix fetch action redirect (go-gitea#37437)
  Refactor integration test DecodeJSON calls to use generic return value (go-gitea#37432)
  Integrate renovate bot for all dependency updates (go-gitea#37050)
  Refactor pull request view (2) (go-gitea#37428)
  Use MarkLongPolling instead of hard-coded route path (go-gitea#37427)
  Optimize CI caches (go-gitea#37387)
  Update AGENTS.md (go-gitea#37420)
  Update Nix flake (go-gitea#37425)
  [skip ci] Updated translations via Crowdin
  remove excessive quote from terraform instructions (go-gitea#37424)
  Improve testing init, clean up webhook tests (go-gitea#37412)
  Fix color regressions, add `priority` color (go-gitea#37417)
  [skip ci] Updated translations via Crowdin
  Stabilize e2e logout propagation test (go-gitea#37403)
  refactor: serve site manifest via `/assets/site-manifest.json` endpoint (go-gitea#37405)
  feat(security): set X-Content-Type-Options: nosniff by default (go-gitea#37354)
  ...
silverwind added a commit to hanism01/gitea that referenced this pull request Apr 27, 2026
…-review-feedback

* origin/main: (144 commits)
  Add API endpoint to reply to pull request review comments (go-gitea#36683)
  Add CurrentURL template variable back (go-gitea#37444)
  refactor: use named `Permission` field in `Repository` struct instead of anonymous embedding (go-gitea#37441)
  Refactor pull request view (3) (go-gitea#37439)
  Update 1.26.1 changelog in main (go-gitea#37442)
  Make GetPossibleUserByID can handle deleted user (go-gitea#37430)
  Fix fetch action redirect (go-gitea#37437)
  Refactor integration test DecodeJSON calls to use generic return value (go-gitea#37432)
  Integrate renovate bot for all dependency updates (go-gitea#37050)
  Refactor pull request view (2) (go-gitea#37428)
  Use MarkLongPolling instead of hard-coded route path (go-gitea#37427)
  Optimize CI caches (go-gitea#37387)
  Update AGENTS.md (go-gitea#37420)
  Update Nix flake (go-gitea#37425)
  [skip ci] Updated translations via Crowdin
  remove excessive quote from terraform instructions (go-gitea#37424)
  Improve testing init, clean up webhook tests (go-gitea#37412)
  Fix color regressions, add `priority` color (go-gitea#37417)
  [skip ci] Updated translations via Crowdin
  Stabilize e2e logout propagation test (go-gitea#37403)
  ...

# Conflicts:
#	models/project/column.go
#	routers/web/repo/issue_page_meta.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/refactoring Existing code has been cleaned up. There should be no new functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants