diff --git a/.forgejo/workflows/testing.yml b/.forgejo/workflows/testing.yml index 06ef5eb131..d3d4282b9c 100644 --- a/.forgejo/workflows/testing.yml +++ b/.forgejo/workflows/testing.yml @@ -92,7 +92,6 @@ jobs: run: | apt-get -q install -qq -y jq ./release-notes-assistant.sh test_main - - uses: ./.forgejo/workflows-composite/build-backend - run: | su forgejo -c 'make test-backend test-check' timeout-minutes: 120 @@ -123,7 +122,6 @@ jobs: if: steps.cache-frontend.outputs.cache-hit != 'true' run: | su forgejo -c 'make deps-frontend frontend' - - uses: ./.forgejo/workflows-composite/build-backend - name: Decide to run all tests id: run-all if: contains(github.event.pull_request.labels.*.name, 'run-all-playwright-tests') || contains(github.event.pull_request.title, 'playwright') @@ -181,7 +179,6 @@ jobs: steps: - uses: https://data.forgejo.org/actions/checkout@v6 - uses: ./.forgejo/workflows-composite/setup-env - - uses: ./.forgejo/workflows-composite/build-backend - run: | su forgejo -c 'make test-remote-cacher test-check' timeout-minutes: 120 @@ -214,7 +211,6 @@ jobs: run: apt-get update -qq && apt-get -q install -qq -y git-lfs env: DEBIAN_FRONTEND: noninteractive - - uses: ./.forgejo/workflows-composite/build-backend - run: | su forgejo -c 'make test-mysql-migration test-mysql' timeout-minutes: 120 @@ -252,7 +248,6 @@ jobs: run: apt-get update -qq && apt-get -q install -qq -y git-lfs env: DEBIAN_FRONTEND: noninteractive - - uses: ./.forgejo/workflows-composite/build-backend - run: | su forgejo -c 'make test-pgsql-migration test-pgsql' timeout-minutes: 120 @@ -274,7 +269,6 @@ jobs: run: apt-get update -qq && apt-get -q install -qq -y git-lfs env: DEBIAN_FRONTEND: noninteractive - - uses: ./.forgejo/workflows-composite/build-backend - run: | su forgejo -c 'make test-sqlite-migration test-sqlite' timeout-minutes: 120 diff --git a/Makefile b/Makefile index 5a5c8c1e54..a61b1a4759 100644 --- a/Makefile +++ b/Makefile @@ -75,6 +75,10 @@ MAKE_EVIDENCE_DIR := .make_evidence ifeq ($(RACE_ENABLED),true) GOFLAGS += -race GOTESTFLAGS += -race + # The test binary calls itself on each git hook + # When the race detector is enabled, don't wait 1s before exiting. + # https://go.dev/doc/articles/race_detector + GOTESTCOMPILEDRUNPREFIX += GORACE="atexit_sleep_ms=0" endif STORED_VERSION_FILE := VERSION diff --git a/models/gitea_migrations/test/tests.go b/models/gitea_migrations/test/tests.go index 41b3cec728..6308e486ff 100644 --- a/models/gitea_migrations/test/tests.go +++ b/models/gitea_migrations/test/tests.go @@ -124,12 +124,7 @@ func MainTest(m *testing.M) { fmt.Println("Environment variable $GITEA_ROOT not set") os.Exit(1) } - giteaBinary := "gitea" - setting.AppPath = path.Join(giteaRoot, giteaBinary) - if _, err := os.Stat(setting.AppPath); err != nil { - fmt.Printf("Could not find gitea binary at %s\n", setting.AppPath) - os.Exit(1) - } + setting.AppPath = path.Join(giteaRoot, "gitea_migrations-should-not-need-a-binary") // use WrapMainAppPath if a binary is needed giteaConf := os.Getenv("GITEA_CONF") if giteaConf == "" { diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index d2959fa43c..3fa9058dd6 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -55,7 +55,6 @@ func InitCustomSettings(confFileName string) { if root == "" { fatalTestError("Environment variable $GITEA_ROOT not set") } - setting.AppPath = filepath.Join(root, "gitea") if setting.CustomConf == "" { templateFile := confFileName + ".tmpl" content, err := os.ReadFile(filepath.Join(root, "tests", templateFile)) @@ -100,6 +99,15 @@ type TestOptions struct { // MainTest a reusable TestMain(..) function for unit tests that need to use a // test database. Creates the test database, and sets necessary settings. func MainTest(m *testing.M, testOpts ...*TestOptions) { + if _, ok := os.LookupEnv("GIT_DIR"); ok { + // The wiki tests require perform git operations. + // It worked before dropping the need for the gitea binary because in case of wiki push, + // the git hooks do not perform http requests (access permission is checked before git invocation). + log.Println("Fake git hook which accepts everything (GIT_DIR is set).") + log.Println("Forgejo with proper http hooks is available in integration tests.") + os.Exit(0) + } + searchDir, _ := os.Getwd() for searchDir != "" { if _, err := os.Stat(filepath.Join(searchDir, "go.mod")); err == nil { diff --git a/modules/base/tool.go b/modules/base/tool.go index 1cc053ba58..059d5aed21 100644 --- a/modules/base/tool.go +++ b/modules/base/tool.go @@ -130,6 +130,7 @@ func IsCitationFile(entry *git.TreeEntry) bool { } // SetupGiteaRoot Sets GITEA_ROOT if it is not already set and returns the value +// TODO: move to a test folder (e.g. models/unittest/) since this isn't called outside tests func SetupGiteaRoot() string { giteaRoot := os.Getenv("GITEA_ROOT") if giteaRoot == "" { @@ -142,7 +143,7 @@ func SetupGiteaRoot() string { giteaRoot = wd } } - if _, err := os.Stat(filepath.Join(giteaRoot, "gitea")); os.IsNotExist(err) { + if _, err := os.Stat(filepath.Join(giteaRoot, "go.mod")); os.IsNotExist(err) { giteaRoot = "" } else if err := os.Setenv("GITEA_ROOT", giteaRoot); err != nil { giteaRoot = "" diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 5ce6029f07..e28b02b5c1 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -31,13 +31,15 @@ import ( var testE2eWebRoutes *web.Route func TestMain(m *testing.M) { + tests.DelegateToMainApp() + defer log.GetManager().Close() managerCtx, cancel := context.WithCancel(context.Background()) graceful.InitManager(managerCtx) defer cancel() - tests.InitTest(true) + tests.InitTest() initChangedFiles() testE2eWebRoutes = routers.NormalRoutes() diff --git a/tests/integration/cmd_forgejo_actions_test.go b/tests/integration/cmd_forgejo_actions_test.go index 088cb5860b..937ce7fcc0 100644 --- a/tests/integration/cmd_forgejo_actions_test.go +++ b/tests/integration/cmd_forgejo_actions_test.go @@ -108,7 +108,7 @@ func TestActions_CmdForgejo_Actions(t *testing.T) { }, } { t.Run(testCase.testName, func(t *testing.T) { - uuid, err := runMainAppWithStdin(testCase.stdin, "forgejo-cli", "actions", "register", testCase.secretOption(), "--scope=org26") + uuid, err := tests.RunMainAppWithStdin(testCase.stdin, "forgejo-cli", "actions", "register", testCase.secretOption(), "--scope=org26") require.NoError(t, err) assert.Equal(t, expecteduuid, uuid) }) diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index e1f08509df..ad1e608adf 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -182,12 +182,11 @@ func doGitPushTestRepository(dstPath string, args ...string) func(*testing.T) { } } -func doGitPushTestRepositoryFail(dstPath string, args ...string) func(*testing.T) { - return func(t *testing.T) { - t.Helper() - _, _, err := git.NewCommand(git.DefaultContext, "push").AddArguments(git.ToTrustedCmdArgs(args)...).RunStdString(&git.RunOpts{Dir: dstPath}) - require.Error(t, err) - } +func doGitPushTestRepositoryFail(t *testing.T, dstPath string, args ...string) (stderr string) { + t.Helper() + _, stderr, err := git.NewCommand(git.DefaultContext, "push").AddArguments(git.ToTrustedCmdArgs(args)...).RunStdString(&git.RunOpts{Dir: dstPath}) + require.Error(t, err) + return stderr } func doGitAddSomeCommits(dstPath, branch string) func(*testing.T) { diff --git a/tests/integration/git_push_test.go b/tests/integration/git_push_test.go index 3a8c653efb..0c693974d6 100644 --- a/tests/integration/git_push_test.go +++ b/tests/integration/git_push_test.go @@ -7,7 +7,6 @@ import ( "fmt" "net/url" "testing" - "time" "forgejo.org/models/db" git_model "forgejo.org/models/git" @@ -15,9 +14,7 @@ import ( "forgejo.org/models/unittest" user_model "forgejo.org/models/user" "forgejo.org/modules/git" - "forgejo.org/modules/log" repo_module "forgejo.org/modules/repository" - "forgejo.org/modules/test" repo_service "forgejo.org/services/repository" "forgejo.org/tests" @@ -267,18 +264,15 @@ func testOptionsGitPush(t *testing.T, u *url.URL) { doGitAddRemote(gitPath, "collaborator", u)(t) t.Run("User without write access is not allowed to push", func(t *testing.T) { - pushLogChecker, cleanup := test.NewLogChecker("ssh", log.ERROR) - pushLogChecker.Filter("User 'user5' is not allowed to push to branch 'branch3' in 'user2/repo-to-push'.") - pushLogChecker.Filter("If you instead wanted to create a pull request to the branch 'branch3', please use:") - pushLogChecker.Filter("git push origin HEAD:refs/for/branch3/choose-a-descriptor") - pushLogChecker.Filter("You might want to replace 'origin' with the name of your Git remote if it is different from origin. You can freely choose the descriptor to set it to a topic.") - pushLogChecker.Filter("You can learn about creating pull requests with AGit in the docs: https://forgejo.org/docs/latest/user/agit-support/") - defer cleanup() branchName := "branch3" doGitCreateBranch(gitPath, branchName)(t) - doGitPushTestRepositoryFail(gitPath, "collaborator", branchName)(t) - pushLogFiltered, _ := pushLogChecker.Check(5 * time.Second) - assert.True(t, pushLogFiltered[0]) + stderr := doGitPushTestRepositoryFail(t, gitPath, "collaborator", branchName) + + assert.Contains(t, stderr, `remote: Forgejo: User 'user5' is not allowed to push to branch 'branch3' in 'user2/repo-to-push'.`) + assert.Contains(t, stderr, `remote: If you instead wanted to create a pull request to the branch 'branch3', please use:`) + assert.Contains(t, stderr, `remote: git push origin HEAD:refs/for/branch3/choose-a-descriptor`) + assert.Contains(t, stderr, `remote: You might want to replace 'origin' with the name of your Git remote if it is different from origin. You can freely choose the descriptor to set it to a topic.`) + assert.Contains(t, stderr, `remote: You can learn about creating pull requests with AGit in the docs: https://forgejo.org/docs/latest/user/agit-support/`) }) // give write access to the collaborator @@ -291,23 +285,15 @@ func testOptionsGitPush(t *testing.T, u *url.URL) { }) t.Run("Collaborator with write access fails to change private & template via push options", func(t *testing.T) { - logChecker, cleanup := test.NewLogChecker(log.DEFAULT, log.TRACE) - logChecker.StopMark("Git push options validation") - defer cleanup() - sshLogChecker, cleanup := test.NewLogChecker("ssh", log.ERROR) - sshLogChecker.Filter("permission denied for changing repo settings") - defer cleanup() branchName := "branch5" doGitCreateBranch(gitPath, branchName)(t) - doGitPushTestRepositoryFail(gitPath, "collaborator", branchName, "-o", "repo.private=true", "-o", "repo.template=true")(t) + stderr := doGitPushTestRepositoryFail(t, gitPath, "collaborator", branchName, "-o", "repo.private=true", "-o", "repo.template=true") + assert.Contains(t, stderr, "Forgejo: options validation failed: permission denied for changing repo settings") + repo, err = repo_model.GetRepositoryByOwnerAndName(db.DefaultContext, user.Name, "repo-to-push") require.NoError(t, err) require.False(t, repo.IsPrivate) require.False(t, repo.IsTemplate) - _, logStopped := logChecker.Check(5 * time.Second) - logFiltered, _ := sshLogChecker.Check(5 * time.Second) - assert.True(t, logStopped) - assert.True(t, logFiltered[0]) }) require.NoError(t, repo_service.DeleteRepositoryDirectly(db.DefaultContext, repo.ID, repo_service.DeleteRepositoryOpts{})) diff --git a/tests/integration/git_test.go b/tests/integration/git_test.go index 3c03630688..e36833f624 100644 --- a/tests/integration/git_test.go +++ b/tests/integration/git_test.go @@ -409,7 +409,7 @@ func doBranchProtect(baseCtx *APITestContext, dstPath string) func(t *testing.T) "apply_to_admins": "on", })) - doGitPushTestRepositoryFail(dstPath, "origin", "HEAD:before-create-2")(t) + doGitPushTestRepositoryFail(t, dstPath, "origin", "HEAD:before-create-2") }) t.Run("FailToPushToProtectedBranch", func(t *testing.T) { @@ -420,7 +420,7 @@ func doBranchProtect(baseCtx *APITestContext, dstPath string) func(t *testing.T) generateCommitWithNewData(t, littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-") }) - doGitPushTestRepositoryFail(dstPath, "origin", "modified-protected-branch:protected")(t) + doGitPushTestRepositoryFail(t, dstPath, "origin", "modified-protected-branch:protected") }) t.Run("PushToUnprotectedBranch", doGitPushTestRepository(dstPath, "origin", "modified-protected-branch:unprotected")) @@ -433,7 +433,7 @@ func doBranchProtect(baseCtx *APITestContext, dstPath string) func(t *testing.T) }) t.Run("ProtectedFilePathsApplyToAdmins", doProtectBranch(ctx, "protected")) - doGitPushTestRepositoryFail(dstPath, "origin", "modified-protected-file-protected-branch:protected")(t) + doGitPushTestRepositoryFail(t, dstPath, "origin", "modified-protected-file-protected-branch:protected") doGitCheckoutBranch(dstPath, "protected")(t) doGitPull(dstPath, "origin", "protected")(t) @@ -467,7 +467,7 @@ func doBranchProtect(baseCtx *APITestContext, dstPath string) func(t *testing.T) t.Run("GenerateCommit", func(t *testing.T) { generateCommitWithNewData(t, littleSize, dstPath, "user2@example.com", "User Two", "branch-data-file-") }) - doGitPushTestRepositoryFail(dstPath, "-f", "origin", "toforce:protected")(t) + doGitPushTestRepositoryFail(t, dstPath, "-f", "origin", "toforce:protected") }) t.Run("WhitelistedUserPushToProtectedBranch", func(t *testing.T) { @@ -660,7 +660,9 @@ func doPushCreate(ctx APITestContext, u *url.URL, objectFormat git.ObjectFormat) // Disable "Push To Create" and attempt to push setting.Repository.EnablePushCreateUser = false - t.Run("FailToPushAndCreateTestRepository", doGitPushTestRepositoryFail(tmpDir, "origin", "master")) + t.Run("FailToPushAndCreateTestRepository", func(t *testing.T) { + doGitPushTestRepositoryFail(t, tmpDir, "origin", "master") + }) // Enable "Push To Create" setting.Repository.EnablePushCreateUser = true @@ -684,7 +686,9 @@ func doPushCreate(ctx APITestContext, u *url.URL, objectFormat git.ObjectFormat) t.Run("AddInvalidRemote", doGitAddRemote(tmpDir, "invalid", u)) // Fail to "Push To Create" the invalid - t.Run("FailToPushAndCreateInvalidTestRepository", doGitPushTestRepositoryFail(tmpDir, "invalid", "master")) + t.Run("FailToPushAndCreateInvalidTestRepository", func(t *testing.T) { + doGitPushTestRepositoryFail(t, tmpDir, "invalid", "master") + }) } } diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index ae594cf9f8..4b32a3ad44 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -17,7 +17,6 @@ import ( "net/http/httptest" "net/url" "os" - "os/exec" "path/filepath" "slices" "strconv" @@ -26,7 +25,6 @@ import ( "testing" "time" - "forgejo.org/cmd" "forgejo.org/models/auth" "forgejo.org/models/db" "forgejo.org/models/unittest" @@ -103,43 +101,11 @@ func NewNilResponseHashSumRecorder() *NilResponseHashSumRecorder { // runMainApp runs the subcommand and returns its standard output. Any returned error will usually be of type *ExitError. If c.Stderr was nil, Output populates ExitError.Stderr. func runMainApp(subcommand string, args ...string) (string, error) { - return runMainAppWithStdin(nil, subcommand, args...) -} - -// runMainAppWithStdin runs the subcommand and returns its standard output. Any returned error will usually be of type *ExitError. If c.Stderr was nil, Output populates ExitError.Stderr. -func runMainAppWithStdin(stdin io.Reader, subcommand string, args ...string) (string, error) { - // running the main app directly will very likely mess with the testing setup (logger & co.) - // hence we run it as a subprocess and capture its output - args = append([]string{subcommand}, args...) - cmd := exec.Command(os.Args[0], args...) - cmd.Env = append(os.Environ(), - "GITEA_TEST_CLI=true", - "GITEA_CONF="+setting.CustomConf, - "GITEA_WORK_DIR="+setting.AppWorkPath) - cmd.Stdin = stdin - out, err := cmd.Output() - if ee, ok := err.(*exec.ExitError); ok { - log.Error("%s %v exit on error %s", os.Args[0], args, ee.Stderr) - } - return string(out), err + return tests.RunMainAppWithStdin(nil, subcommand, args...) } func TestMain(m *testing.M) { - // GITEA_TEST_CLI is set by runMainAppWithStdin - // inspired by https://abhinavg.net/2022/05/15/hijack-testmain/ - if testCLI := os.Getenv("GITEA_TEST_CLI"); testCLI == "true" { - app := cmd.NewMainApp("test-version", "integration-test") - args := append([]string{ - "executable-name", // unused, but expected at position 1 - "--config", os.Getenv("GITEA_CONF"), - }, - os.Args[1:]..., // skip the executable name - ) - if err := cmd.RunMainApp(app, args...); err != nil { - panic(err) // should never happen since RunMainApp exits on error - } - return - } + tests.DelegateToMainApp() defer log.GetManager().Close() @@ -147,7 +113,7 @@ func TestMain(m *testing.M) { graceful.InitManager(managerCtx) defer cancel() - tests.InitTest(true) + tests.InitTest() testWebRoutes = routers.NormalRoutes() // integration test settings... diff --git a/tests/integration/migration-test/migration_test.go b/tests/integration/migration-test/migration_test.go index 98887c3493..dec9619b00 100644 --- a/tests/integration/migration-test/migration_test.go +++ b/tests/integration/migration-test/migration_test.go @@ -51,11 +51,7 @@ func initMigrationTest(t *testing.T) func() { deferFn := tests.PrintCurrentTest(t, 2) root := getRoot(t) - setting.AppPath = path.Join(root, "gitea") - if _, err := os.Stat(setting.AppPath); err != nil { - tests.Printf("Could not find gitea binary at %s\n", setting.AppPath) - os.Exit(1) - } + setting.AppPath = path.Join(root, "migration-test-should-not-need-a-binary") // use WrapMainAppPath if a binary is needed giteaConf := os.Getenv("GITEA_CONF") if giteaConf == "" { diff --git a/tests/integration/ssh_key_test.go b/tests/integration/ssh_key_test.go index 747b6c4eb0..126d027b17 100644 --- a/tests/integration/ssh_key_test.go +++ b/tests/integration/ssh_key_test.go @@ -154,7 +154,9 @@ func testKeyOnlyOneType(t *testing.T, u *url.URL) { t.Run("AddChanges", doAddChangesToCheckout(dstPath, "CHANGES2.md")) - t.Run("FailToPush", doGitPushTestRepositoryFail(dstPath, "origin", "master")) + t.Run("FailToPush", func(t *testing.T) { + doGitPushTestRepositoryFail(t, dstPath, "origin", "master") + }) otherSSHURL := createSSHUrl(otherCtx.GitPath(), u) dstOtherPath := t.TempDir() diff --git a/tests/test_utils.go b/tests/test_utils.go index 57c9c9bcd6..d93bfb3223 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -12,6 +12,7 @@ import ( "io" "mime/multipart" "os" + "os/exec" "path" "path/filepath" "runtime" @@ -21,6 +22,7 @@ import ( "testing/fstest" "time" + "forgejo.org/cmd" "forgejo.org/models/db" packages_model "forgejo.org/models/packages" repo_model "forgejo.org/models/repo" @@ -56,7 +58,41 @@ func exitf(format string, args ...any) { var preparedDir string -func InitTest(requireGitea bool) { +// DelegateToMainApp must be the first call in TestMain. +// If the call must be delegated to the main, it will exit upon completion. +func DelegateToMainApp() { + // inspired by https://abhinavg.net/2022/05/15/hijack-testmain/ + + _, isGitHook := os.LookupEnv("GIT_DIR") // set when called from a hook or as subprocess + _, isSSHServ := os.LookupEnv("GIT_PROTOCOL") // set when called from ssh (for key lookup) + + if isGitHook || isSSHServ { + app := cmd.NewMainApp("test-version", "integration-test") + if err := cmd.RunMainApp(app, os.Args...); err != nil { + panic(err) // should never happen since RunMainApp exits on error + } + os.Exit(0) + } +} + +// RunMainAppWithStdin runs the subcommand and returns its standard output. Any returned error will usually be of type *ExitError. If c.Stderr was nil, Output populates ExitError.Stderr. +func RunMainAppWithStdin(stdin io.Reader, subcommand string, args ...string) (string, error) { + // running the main app directly will very likely mess with the testing setup (logger & co.) + // hence we run it as a subprocess and capture its output + args = append([]string{"--config", setting.CustomConf, subcommand}, args...) + cmd := exec.Command(os.Args[0], args...) + cmd.Env = append(os.Environ(), + "GIT_DIR=", // signal DelegateToMainApp that we want to run main + ) + cmd.Stdin = stdin + out, err := cmd.Output() + if ee, ok := err.(*exec.ExitError); ok { + log.Error("%s %v exit on error %s", os.Args[0], args, ee.Stderr) + } + return string(out), err +} + +func InitTest() { log.RegisterEventWriter("test", testlogger.NewTestLoggerWriter) giteaRoot := base.SetupGiteaRoot() @@ -70,13 +106,6 @@ func InitTest(requireGitea bool) { setting.IsInTesting = true setting.AppWorkPath = giteaRoot setting.CustomPath = filepath.Join(setting.AppWorkPath, "custom") - if requireGitea { - giteaBinary := "gitea" - setting.AppPath = path.Join(giteaRoot, giteaBinary) - if _, err := os.Stat(setting.AppPath); err != nil { - exitf("Could not find gitea binary at %s", setting.AppPath) - } - } giteaConf := os.Getenv("GITEA_CONF") if giteaConf == "" { // By default, use sqlite.ini for testing, then IDE like GoLand can start the test process with debugger. @@ -95,6 +124,12 @@ func InitTest(requireGitea bool) { setting.CustomConf = giteaConf } + executablePath, err := filepath.Abs(os.Args[0]) + if err != nil { + exitf("could not determine absolute path: %w", err) + } + setting.AppPath = executablePath + unittest.InitSettings() setting.Repository.DefaultBranch = "master" // many test code still assume that default branch is called "master" _ = util.RemoveAll(repo_module.LocalCopyPath())