Skip to content

Remove external service dependencies in migration tests#36866

Merged
silverwind merged 2 commits intogo-gitea:mainfrom
silverwind:nodep
Apr 23, 2026
Merged

Remove external service dependencies in migration tests#36866
silverwind merged 2 commits intogo-gitea:mainfrom
silverwind:nodep

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Mar 8, 2026 •
edited
Loading

Fix #36859

Replace live third-party API calls in migration tests with a fixture-based HTTP mock server. Fixtures are committed so tests run offline by default; live recording is gated per service on an API-token env var.

This PR was written with the help of Claude Opus 4.7

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 8, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Mar 8, 2026

But you shouldn't use it to test itself.

What if the API breaks? The tests succeed, but it causes problems when released.

@wxiaoguang wxiaoguang marked this pull request as draft March 8, 2026 14:30
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Mar 8, 2026 •
edited
Loading

So just write a HTTP-level mock that replicates what gitea.com currently does? I'll try that.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Mar 8, 2026

I think yes. Maybe the work can start without waiting for gitea.com, indeed there are other tests needing such feature, e.g.: gitlab, github

@TheFox0x7
Copy link
Copy Markdown
Contributor

TheFox0x7 commented Mar 8, 2026

There was a tool that could record the traffic and mock the replies from the server... I'm trying to find it since it be interesting for this.

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Mar 8, 2026

I don't think we need such tools. Just a proper HTTP mock in go should do and AI can determine exactly what's needed for this test to pass.

@TheFox0x7
Copy link
Copy Markdown
Contributor

TheFox0x7 commented Mar 8, 2026

Found it. Not sure if it'll be useful but linking just in case https://keploy.io/docs/keploy-explained/why-keploy/

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Mar 8, 2026

Here's the repo: https://github.com/keploy/keploy. Seems interesting but definitely out of scope for gitea right now.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Mar 8, 2026 •
edited
Loading

Can be done like this https://codeberg.org/forgejo/forgejo/commits/branch/forgejo/models/unittest/mock_http.go

https://codeberg.org/forgejo/forgejo/src/branch/forgejo/models/unittest/mock_http.go

The license is still MIT, so can just copy

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Mar 8, 2026 •
edited
Loading

Can be done like this

That server has some sort of "live mode" toggle based on the presence of an auth token where it rewrites test fixtures. Essentially it's similar to snapshot testing (but more manual), which you had rejected earlier.

Current approach now has the minimal JSON responses inline using httptest.Server, if think it's sufficient. But if you really want we can also implement this "live mode" stuff.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Mar 8, 2026

Essentially it's similar to snapshot testing (but more manual), which you had rejected earlier.

Really? Which comment? I think there are some misunderstandings.

@silverwind silverwind marked this pull request as ready for review March 8, 2026 15:26
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Mar 8, 2026

Essentially it's similar to snapshot testing (but more manual), which you had rejected earlier.

Really? Which comment? I think there are some misunderstandings.

#36238 (review)

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Mar 8, 2026

Essentially it's similar to snapshot testing (but more manual), which you had rejected earlier.

Really? Which comment? I think there are some misunderstandings.

#36238 (review)

I think I have explained clearly:

Because many Gitea maintainers are too lazy to write correct tests, not that experienced as typescript-go developers.

If you use "go-snaps", then the test data causing failure will just be blindly updated to make tests pass.

But here, if you have the mock server with live mode, you first test against the live servers, then capture the test data. Nobody can blindly update the test data.

They are totally different.

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Mar 8, 2026

Not that different to me, but let's see what Claude comes up if I implement that pattern.

@wxiaoguang
Copy link
Copy Markdown
Contributor

wxiaoguang commented Mar 8, 2026 •
edited
Loading

The difference is whether you have a trust source which can provide the facts!!

Mocked data with live server: the live server is the trust source.

Your "go-snaps": you can blindly use the untrusted test output to update the test data without verifying them in many cases.

I hope you can understand the difference.

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Mar 8, 2026 •
edited
Loading

Rewrite still in progress. Once tidbit that was noticed is the Forgejo test does not actually test the cloning of the repo so it's incomplete.

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Mar 8, 2026

Implemented that mechansim, now we can run

GITEA_TOKEN=<token> go test -run Test_MigrateFromGiteaToGitea ./tests/integration/ -tags='bindata sqlite sqlite_unlock_notify'

To record and update the test fixtures.

wxiaoguang
wxiaoguang reviewed Mar 8, 2026
View reviewed changes
Comment thread tests/integration/migrate_test.go Outdated
@silverwind silverwind changed the title Remove external dependency in Test_MigrateFromGiteaToGitea Remove external dependency in Test_MigrateFromGiteaToGitea Mar 8, 2026
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Mar 8, 2026 •
edited
Loading

I also checked other tests for external dependencies and there are a few:

Test File External Service Gate
TestGitHubDownloadRepo github_test.go github.com GITHUB_READ_TOKEN
TestGitlabDownloadRepo gitlab_test.go gitlab.com GITLAB_READ_TOKEN
TestGiteaDownloadRepo gitea_downloader_test.go gitea.com GITEA_TEST_OFFICIAL_SITE_TOKEN
TestCodebaseDownloadRepo codebase_test.go codebasehq.com CODEBASE_CLONE_USER + 3 more
TestGogsDownloadRepo gogs_test.go try.gogs.io GOGS_READ_TOKEN
TestOneDevDownloadRepo onedev_test.go code.onedev.io HTTP reachability check

Maybe we should also use GITEA_TEST_OFFICIAL_SITE_TOKEN in this test instead of GITEA_TOKEN (too generic name).

@silverwind silverwind marked this pull request as draft March 8, 2026 19:45
@silverwind silverwind changed the title Remove external dependency in Test_MigrateFromGiteaToGitea Replace external dependencies with fixture-based mocks in migration tests Mar 9, 2026
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Mar 9, 2026 •
edited
Loading

All migration tests now follow this scheme. If a token is provided they go into live mode, otherwise they use the recorded fixtures.

  • The Gogs, OneDev and Codebase fixtures were fabricated (either repo was deleted or no access)
  • Gitea, GitLab and GitHub were recorded.

None of the remaining integration tests have dependencies on external services now (only local services like database etc).

@silverwind silverwind marked this pull request as ready for review March 9, 2026 20:43
@silverwind silverwind changed the title Replace external dependencies with fixture-based mocks in migration tests Remve external service dependencies in migration tests Mar 10, 2026
@silverwind silverwind changed the title Remve external service dependencies in migration tests Remove external service dependencies in migration tests Mar 10, 2026
@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 Mar 11, 2026
TheFox0x7
TheFox0x7 reviewed Mar 16, 2026
View reviewed changes
Comment thread services/migrations/gitlab_test.go Outdated
@bircni
Copy link
Copy Markdown
Member

bircni commented Mar 24, 2026

Updates on that?

TheFox0x7
TheFox0x7 requested changes Mar 28, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@TheFox0x7 TheFox0x7 left a comment

Choose a reason for hiding this comment

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

@silverwind Can you rerun/fix the gitlab test so the asserts aren't removed?
Blocking until then so it won't get merged with missing asserts compared to main

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 28, 2026
@lunny lunny removed the modifies/go label Apr 19, 2026
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 19, 2026 •
edited
Loading

@silverwind Can you rerun/fix the gitlab test so the asserts aren't removed? Blocking until then so it won't get merged with missing asserts compared to main

Done in 9c7c1d0. Ready for review.

@silverwind silverwind requested a review from TheFox0x7 April 21, 2026 00:16
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 21, 2026

@TheFox0x7 please re-check.

TheFox0x7
TheFox0x7 reviewed Apr 21, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@TheFox0x7 TheFox0x7 left a comment

Choose a reason for hiding this comment

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

We get PR 6 but test reviews on PR 1

I know you can't exactly control which gets pulled from gitlab but the previous one was more comprehensive and should be kept IMO - so maybe or fetch two and compare the closed and unmerged and the approved and merged one?

There's also the date change in reviews but I have no idea why it happened (both seem wrong to me) so it's probably fine.

@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 23, 2026 •
edited
Loading

Done in cfbe630 and re-reviewed all other fixtures, all successful.

TheFox0x7
TheFox0x7 approved these changes Apr 23, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@TheFox0x7 TheFox0x7 left a comment

Choose a reason for hiding this comment

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

Not exactly what I had in mind but on reading the raw gitlab test code instead of a diff I think issue assert covers what I wanted so it's fine as is IMO.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Apr 23, 2026
@silverwind @claude
Remove external service dependencies in migration tests
dd41452 
Replace live calls to gitea.com, github.com, gitlab.com, codebase,
gogs, and onedev with a fixture-based HTTP mock server under
models/unittest. Fixtures are committed so the default run is offline;
live/record mode is gated on an API-token env var per service.

Test_MigrateFromGiteaToGitea additionally serves its git data via an
in-process git-http-backend backed by a deterministic fast-import
stream, and simulates the PR fork repository so the head branch
resolves without a second upstream dependency.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind enabled auto-merge (squash) April 23, 2026 14:24
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 23, 2026
@silverwind silverwind merged commit 7947851 into go-gitea:main Apr 23, 2026
26 checks passed
@silverwind silverwind deleted the nodep branch April 23, 2026 15:18
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 23, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 23, 2026
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 24, 2026
@silverwind
Merge remote-tracking branch 'origin/main' into acticons
e9bb781 
* origin/main:
  Allow fast-forward-only merge when signed commits are required (go-gitea#37335)
  Introduce `ActionRunAttempt` to represent each execution of a run (go-gitea#37119)
  Move review request functions to a standalone file (go-gitea#37358)
  Fix repo init README EOL (go-gitea#37388)
  Fix org team assignee/reviewer lookups for team member permissions (go-gitea#37365)
  Remove external service dependencies in migration tests (go-gitea#36866)
  Extend issue context popup beyond markdown content (go-gitea#36908)

# Conflicts:
#	routers/api/v1/repo/action.go
#	web_src/js/components/RepoActionView.vue
silverwind added a commit to mohammad-rj/gitea that referenced this pull request Apr 24, 2026
@silverwind
Merge remote-tracking branch 'origin/main' into feat/websocket-migration
ec54754 
* origin/main: (127 commits)
  Refactor pull request view (1) (go-gitea#37380)
  Improve AGENTS.md (go-gitea#37382)
  Remove dead CSS (go-gitea#37376)
  Add pr-review e2e test and speed up e2e tests (go-gitea#37345)
  Drop Fomantic tab, checkbox and form patches (go-gitea#37377)
  fix: dump with default zip type produces uncompressed zip (go-gitea#37401)
  Allow fast-forward-only merge when signed commits are required (go-gitea#37335)
  Introduce `ActionRunAttempt` to represent each execution of a run (go-gitea#37119)
  Move review request functions to a standalone file (go-gitea#37358)
  Fix repo init README EOL (go-gitea#37388)
  Fix org team assignee/reviewer lookups for team member permissions (go-gitea#37365)
  Remove external service dependencies in migration tests (go-gitea#36866)
  Extend issue context popup beyond markdown content (go-gitea#36908)
  fix: commit status reporting (go-gitea#37372)
  Support for Custom URI Schemes in OAuth2 Redirect URIs (go-gitea#37356)
  Fix cmd tests by mocking builtin paths (go-gitea#37369)
  chore: upgrade Go version in devcontainer image to 1.26 (go-gitea#37374)
  Fix button layout shift when collapsing file tree in editor (go-gitea#37363)
  Update `Block a user` form (go-gitea#37359)
  Remove IsValidExternalURL/IsAPIURL and use IsValidURL at call sites (go-gitea#37364)
  ...

# Conflicts:
#	modules/eventsource/event.go
#	tests/e2e/events.test.ts
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 25, 2026
@silverwind
Merge remote-tracking branch 'origin/main' into reply
aec2f56 
* origin/main: (51 commits)
  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)
  Improve AGENTS.md (go-gitea#37382)
  Remove dead CSS (go-gitea#37376)
  Add pr-review e2e test and speed up e2e tests (go-gitea#37345)
  Drop Fomantic tab, checkbox and form patches (go-gitea#37377)
  fix: dump with default zip type produces uncompressed zip (go-gitea#37401)
  Allow fast-forward-only merge when signed commits are required (go-gitea#37335)
  Introduce `ActionRunAttempt` to represent each execution of a run (go-gitea#37119)
  Move review request functions to a standalone file (go-gitea#37358)
  Fix repo init README EOL (go-gitea#37388)
  Fix org team assignee/reviewer lookups for team member permissions (go-gitea#37365)
  Remove external service dependencies in migration tests (go-gitea#36866)
  Extend issue context popup beyond markdown content (go-gitea#36908)
  fix: commit status reporting (go-gitea#37372)
  Support for Custom URI Schemes in OAuth2 Redirect URIs (go-gitea#37356)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wxiaoguang wxiaoguang wxiaoguang left review comments

@lunny lunny lunny approved these changes

@TheFox0x7 TheFox0x7 TheFox0x7 approved these changes

Assignees

No one assigned

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/testing

Projects

None yet

Milestone

1.27.0

Development

Successfully merging this pull request may close these issues.

Test_MigrateFromGiteaToGitea fails when gitea.com is unavailable

6 participants

@silverwind @wxiaoguang @TheFox0x7 @bircni @lunny @GiteaBot