From 160ae7276f7796f6a349bc9f77fab9d9a210f9e3 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 25 Apr 2024 09:47:14 +0800 Subject: [PATCH 1/2] fix --- cmd/admin_user_change_password.go | 2 +- cmd/admin_user_create.go | 18 ++++++++----- cmd/admin_user_create_test.go | 44 +++++++++++++++++++++++++++++++ cmd/main.go | 9 +++++-- cmd/main_test.go | 2 +- main.go | 2 +- models/unittest/testdb.go | 12 +++++++-- modules/log/logger_global.go | 4 ++- tests/test_utils.go | 1 - 9 files changed, 79 insertions(+), 15 deletions(-) create mode 100644 cmd/admin_user_create_test.go diff --git a/cmd/admin_user_change_password.go b/cmd/admin_user_change_password.go index bd9063a8e4b8d..f1ed46e70b083 100644 --- a/cmd/admin_user_change_password.go +++ b/cmd/admin_user_change_password.go @@ -35,7 +35,7 @@ var microcmdUserChangePassword = &cli.Command{ }, &cli.BoolFlag{ Name: "must-change-password", - Usage: "User must change password", + Usage: "User must change password (can be disabled by --must-change-password=false)", Value: true, }, }, diff --git a/cmd/admin_user_create.go b/cmd/admin_user_create.go index 403e3ee8d82dc..087a06146d150 100644 --- a/cmd/admin_user_create.go +++ b/cmd/admin_user_create.go @@ -4,6 +4,7 @@ package cmd import ( + "context" "errors" "fmt" @@ -48,7 +49,7 @@ var microcmdUserCreate = &cli.Command{ }, &cli.BoolFlag{ Name: "must-change-password", - Usage: "Set to false to prevent forcing the user to change their password after initial login", + Usage: "User must change password after initial login, defaults to true for all users except the first admin user (can be disabled by --must-change-password=false)", DisableDefaultText: true, }, &cli.IntFlag{ @@ -91,11 +92,16 @@ func runCreateUser(c *cli.Context) error { _, _ = fmt.Fprintf(c.App.ErrWriter, "--name flag is deprecated. Use --username instead.\n") } - ctx, cancel := installSignals() - defer cancel() - - if err := initDB(ctx); err != nil { - return err + ctx := c.Context + if !setting.IsInTesting { + // FIXME: need to refactor the "installSignals/initDB" related code later + // it doesn't make sense to call it in (almost) every command action function + var cancel context.CancelFunc + ctx, cancel = installSignals() + defer cancel() + if err := initDB(ctx); err != nil { + return err + } } var password string diff --git a/cmd/admin_user_create_test.go b/cmd/admin_user_create_test.go new file mode 100644 index 0000000000000..cf172fca5a81f --- /dev/null +++ b/cmd/admin_user_create_test.go @@ -0,0 +1,44 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package cmd + +import ( + "fmt" + "strings" + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + + "github.com/stretchr/testify/assert" +) + +func TestAdminUserCreate(t *testing.T) { + app := NewMainApp(AppVersion{}) + + reset := func() { + assert.NoError(t, db.TruncateBeans(db.DefaultContext, &user_model.User{})) + assert.NoError(t, db.TruncateBeans(db.DefaultContext, &user_model.EmailAddress{})) + } + + type createCheck struct{ IsAdmin, MustChangePassword bool } + createUser := func(name, args string) createCheck { + assert.NoError(t, app.Run(strings.Fields(fmt.Sprintf("./gitea admin user create --username %s --email %s@gitea.local %s --password foobar", name, name, args)))) + u := unittest.AssertExistsAndLoadBean(t, &user_model.User{LowerName: name}) + return createCheck{u.IsAdmin, u.MustChangePassword} + } + reset() + assert.Equal(t, createCheck{IsAdmin: false, MustChangePassword: true}, createUser("u", ""), "first non-admin user must change password") + + reset() + assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: false}, createUser("u", "--admin"), "first admin user doesn't need to change password") + + reset() + assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: true}, createUser("u", "--admin --must-change-password")) + assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: true}, createUser("u2", "--admin")) + assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: false}, createUser("u3", "--admin --must-change-password=false")) + assert.Equal(t, createCheck{IsAdmin: false, MustChangePassword: true}, createUser("u4", "")) + assert.Equal(t, createCheck{IsAdmin: false, MustChangePassword: false}, createUser("u5", "--must-change-password=false")) +} diff --git a/cmd/main.go b/cmd/main.go index 02dd660e9eec4..fd648946efa39 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -112,13 +112,18 @@ func prepareWorkPathAndCustomConf(action cli.ActionFunc) func(ctx *cli.Context) } } -func NewMainApp(version, versionExtra string) *cli.App { +type AppVersion struct { + Version string + Extra string +} + +func NewMainApp(appVer AppVersion) *cli.App { app := cli.NewApp() app.Name = "Gitea" app.HelpName = "gitea" app.Usage = "A painless self-hosted Git service" app.Description = `Gitea program contains "web" and other subcommands. If no subcommand is given, it starts the web server by default. Use "web" subcommand for more web server arguments, use other subcommands for other purposes.` - app.Version = version + versionExtra + app.Version = appVer.Version + appVer.Extra app.EnableBashCompletion = true // these sub-commands need to use config file diff --git a/cmd/main_test.go b/cmd/main_test.go index a916c61f853d2..c182b440199d2 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -28,7 +28,7 @@ func makePathOutput(workPath, customPath, customConf string) string { } func newTestApp(testCmdAction func(ctx *cli.Context) error) *cli.App { - app := NewMainApp("version", "version-extra") + app := NewMainApp(AppVersion{}) testCmd := &cli.Command{Name: "test-cmd", Action: testCmdAction} prepareSubcommandWithConfig(testCmd, appGlobalFlags()) app.Commands = append(app.Commands, testCmd) diff --git a/main.go b/main.go index 775c729c569ea..756c3e0f9ba12 100644 --- a/main.go +++ b/main.go @@ -42,7 +42,7 @@ func main() { log.GetManager().Close() os.Exit(code) } - app := cmd.NewMainApp(Version, formatBuiltWith()) + app := cmd.NewMainApp(cmd.AppVersion{Version: Version, Extra: formatBuiltWith()}) _ = cmd.RunMainApp(app, os.Args...) // all errors should have been handled by the RunMainApp log.GetManager().Close() } diff --git a/models/unittest/testdb.go b/models/unittest/testdb.go index cb90c12f2b716..57b9ba355a0e4 100644 --- a/models/unittest/testdb.go +++ b/models/unittest/testdb.go @@ -6,7 +6,6 @@ package unittest import ( "context" "fmt" - "log" "os" "path/filepath" "strings" @@ -17,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/auth/password/hash" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting/config" "code.gitea.io/gitea/modules/storage" @@ -45,6 +45,14 @@ func fatalTestError(fmtStr string, args ...any) { // InitSettings initializes config provider and load common settings for tests func InitSettings() { + setting.IsInTesting = true + log.OsExiter = func(code int) { + if code != 0 { + // non-zero exit code (log.Fatal) shouldn't occur during testing, if it happens, show a full stacktrace for more details + panic(fmt.Errorf("non-zero exit code during testing: %d", code)) + } + os.Exit(0) + } if setting.CustomConf == "" { setting.CustomConf = filepath.Join(setting.CustomPath, "conf/app-unittest-tmp.ini") _ = os.Remove(setting.CustomConf) @@ -53,7 +61,7 @@ func InitSettings() { setting.LoadCommonSettings() if err := setting.PrepareAppDataPath(); err != nil { - log.Fatalf("Can not prepare APP_DATA_PATH: %v", err) + log.Fatal("Can not prepare APP_DATA_PATH: %v", err) } // register the dummy hash algorithm function used in the test fixtures _ = hash.Register("dummy", hash.NewDummyHasher) diff --git a/modules/log/logger_global.go b/modules/log/logger_global.go index 994acfedbb3a1..6ce8b70fed620 100644 --- a/modules/log/logger_global.go +++ b/modules/log/logger_global.go @@ -57,11 +57,13 @@ func Critical(format string, v ...any) { Log(1, ERROR, format, v...) } +var OsExiter = os.Exit + // Fatal records fatal log and exit process func Fatal(format string, v ...any) { Log(1, FATAL, format, v...) GetManager().Close() - os.Exit(1) + OsExiter(1) } func GetLogger(name string) Logger { diff --git a/tests/test_utils.go b/tests/test_utils.go index 50049e73f01c9..66a287ecad262 100644 --- a/tests/test_utils.go +++ b/tests/test_utils.go @@ -46,7 +46,6 @@ func InitTest(requireGitea bool) { // TODO: Speedup tests that rely on the event source ticker, confirm whether there is any bug or failure. // setting.UI.Notification.EventSourceUpdateTime = time.Second - setting.IsInTesting = true setting.AppWorkPath = giteaRoot setting.CustomPath = filepath.Join(setting.AppWorkPath, "custom") if requireGitea { From 7d69cde0dc2be1d8ffba484409f8761f5aeddb27 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 27 Apr 2024 19:03:34 +0800 Subject: [PATCH 2/2] revert to old behavior --- cmd/admin_user_create.go | 6 +++--- cmd/admin_user_create_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/admin_user_create.go b/cmd/admin_user_create.go index 087a06146d150..f328b753d86da 100644 --- a/cmd/admin_user_create.go +++ b/cmd/admin_user_create.go @@ -49,7 +49,7 @@ var microcmdUserCreate = &cli.Command{ }, &cli.BoolFlag{ Name: "must-change-password", - Usage: "User must change password after initial login, defaults to true for all users except the first admin user (can be disabled by --must-change-password=false)", + Usage: "User must change password after initial login, defaults to true for all users except the first one (can be disabled by --must-change-password=false)", DisableDefaultText: true, }, &cli.IntFlag{ @@ -129,8 +129,8 @@ func runCreateUser(c *cli.Context) error { if err != nil { return fmt.Errorf("IsTableNotEmpty: %w", err) } - if !hasUserRecord && isAdmin { - // if this is the first admin being created, don't force to change password (keep the old behavior) + if !hasUserRecord { + // if this is the first one being created, don't force to change password (keep the old behavior) mustChangePassword = false } } diff --git a/cmd/admin_user_create_test.go b/cmd/admin_user_create_test.go index cf172fca5a81f..83754e97b10a4 100644 --- a/cmd/admin_user_create_test.go +++ b/cmd/admin_user_create_test.go @@ -30,7 +30,7 @@ func TestAdminUserCreate(t *testing.T) { return createCheck{u.IsAdmin, u.MustChangePassword} } reset() - assert.Equal(t, createCheck{IsAdmin: false, MustChangePassword: true}, createUser("u", ""), "first non-admin user must change password") + assert.Equal(t, createCheck{IsAdmin: false, MustChangePassword: false}, createUser("u", ""), "first non-admin user doesn't need to change password") reset() assert.Equal(t, createCheck{IsAdmin: true, MustChangePassword: false}, createUser("u", "--admin"), "first admin user doesn't need to change password")