Skip to content

Commit 4c18f5c

Browse files
TheFox0x7TheFox0x7
committed
unify error syntax
change tests to depend on length instead of error existance
1 parent 3cdf752 commit 4c18f5c

File tree

6 files changed

+41
-30
lines changed

6 files changed

+41
-30
lines changed

‎modules/setting/git_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,5 +61,5 @@ EXPIRATION = 123
6161
`)
6262
require.NoError(t, err)
6363

64-
require.Error(t, checkForRemovedSettings(cfg))
64+
require.Len(t, checkForRemovedSettings(cfg), 2)
6565
}

‎modules/setting/lfs_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ LFS_CONTENT_PATH = deprecatedpath
4040
`
4141
cfg, err = NewConfigProviderFromData(iniStr)
4242
assert.NoError(t, err)
43-
assert.Error(t, checkForRemovedSettings(cfg))
43+
assert.Len(t, checkForRemovedSettings(cfg), 1)
4444

4545
iniStr = `
4646
[storage.lfs]

‎modules/setting/log_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ ACCESS = file
159159
`)
160160
require.NoError(t, err)
161161

162-
require.Error(t, checkForRemovedSettings(cfg))
162+
require.Len(t, checkForRemovedSettings(cfg), 2)
163163
}
164164

165165
func TestLogConfigLegacyModeDisable(t *testing.T) {
@@ -172,7 +172,7 @@ ENABLE_ACCESS_LOG = false
172172
`)
173173
require.NoError(t, err)
174174

175-
require.Error(t, checkForRemovedSettings(cfg))
175+
require.Len(t, checkForRemovedSettings(cfg), 4)
176176
}
177177

178178
func TestLogConfigNewConfig(t *testing.T) {

‎modules/setting/mailer_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func Test_loadMailerFrom(t *testing.T) {
3232
sec.NewKey("HOST", host)
3333

3434
// Ensure removed settings catches the removed config setting
35-
assert.Error(t, checkForRemovedSettings(cfg))
35+
assert.Len(t, checkForRemovedSettings(cfg), 1)
3636
})
3737
}
3838
}

‎modules/setting/removed.go‎

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,38 +4,37 @@
44
package setting
55

66
import (
7-
"errors"
87
"fmt"
98
)
109

1110
// checkForRemovedSettings checks the entire configuration for removed keys and gathers them fataling if any is present
1211
// Arbitrary permament removal is 3 releases
13-
func checkForRemovedSettings(rootCfg ConfigProvider) error {
12+
func checkForRemovedSettings(rootCfg ConfigProvider) []error {
1413
var errs []error
1514

1615
removedSettings := []struct {
1716
oldSection, oldKey, newSection, newKey, version string
1817
}{
1918
{"service", "EMAIL_DOMAIN_WHITELIST", "service", "EMAIL_DOMAIN_ALLOWLIST", "1.21"},
20-
{"mailer", "MAILER_TYPE", "mailer", "PROTOCOL", "v1.19.0"},
21-
{"mailer", "HOST", "mailer", "SMTP_ADDR", "v1.19.0"},
22-
{"mailer", "IS_TLS_ENABLED", "mailer", "PROTOCOL", "v1.19.0"},
23-
{"mailer", "DISABLE_HELO", "mailer", "ENABLE_HELO", "v1.19.0"},
24-
{"mailer", "SKIP_VERIFY", "mailer", "FORCE_TRUST_SERVER_CERT", "v1.19.0"},
25-
{"mailer", "USE_CERTIFICATE", "mailer", "USE_CLIENT_CERT", "v1.19.0"},
26-
{"mailer", "CERT_FILE", "mailer", "CLIENT_CERT_FILE", "v1.19.0"},
27-
{"mailer", "KEY_FILE", "mailer", "CLIENT_KEY_FILE", "v1.19.0"},
28-
{"task", "QUEUE_TYPE", "queue.task", "TYPE", "v1.19.0"},
29-
{"task", "QUEUE_CONN_STR", "queue.task", "CONN_STR", "v1.19.0"},
30-
{"task", "QUEUE_LENGTH", "queue.task", "LENGTH", "v1.19.0"},
31-
{"server", "ENABLE_LETSENCRYPT", "server", "ENABLE_ACME", "v1.19.0"},
32-
{"server", "LETSENCRYPT_ACCEPTTOS", "server", "ACME_ACCEPTTOS", "v1.19.0"},
33-
{"server", "LETSENCRYPT_DIRECTORY", "server", "ACME_DIRECTORY", "v1.19.0"},
34-
{"server", "LETSENCRYPT_EMAIL", "server", "ACME_EMAIL", "v1.19.0"},
19+
{"mailer", "MAILER_TYPE", "mailer", "PROTOCOL", "v1.19"},
20+
{"mailer", "HOST", "mailer", "SMTP_ADDR", "v1.19"},
21+
{"mailer", "IS_TLS_ENABLED", "mailer", "PROTOCOL", "v1.19"},
22+
{"mailer", "DISABLE_HELO", "mailer", "ENABLE_HELO", "v1.19"},
23+
{"mailer", "SKIP_VERIFY", "mailer", "FORCE_TRUST_SERVER_CERT", "v1.19"},
24+
{"mailer", "USE_CERTIFICATE", "mailer", "USE_CLIENT_CERT", "v1.19"},
25+
{"mailer", "CERT_FILE", "mailer", "CLIENT_CERT_FILE", "v1.19"},
26+
{"mailer", "KEY_FILE", "mailer", "CLIENT_KEY_FILE", "v1.19"},
27+
{"task", "QUEUE_TYPE", "queue.task", "TYPE", "v1.19"},
28+
{"task", "QUEUE_CONN_STR", "queue.task", "CONN_STR", "v1.19"},
29+
{"task", "QUEUE_LENGTH", "queue.task", "LENGTH", "v1.19"},
30+
{"server", "ENABLE_LETSENCRYPT", "server", "ENABLE_ACME", "v1.19"},
31+
{"server", "LETSENCRYPT_ACCEPTTOS", "server", "ACME_ACCEPTTOS", "v1.19"},
32+
{"server", "LETSENCRYPT_DIRECTORY", "server", "ACME_DIRECTORY", "v1.19"},
33+
{"server", "LETSENCRYPT_EMAIL", "server", "ACME_EMAIL", "v1.19"},
3534
{"git.reflog", "ENABLED", "git.config", "core.logAllRefUpdates", "1.21"},
3635
{"git.reflog", "EXPIRATION", "git.config", "core.reflogExpire", "1.21"},
37-
{"repository", "DISABLE_MIRRORS", "mirror", "ENABLED", "v1.19.0"},
38-
{"server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19.0"},
36+
{"repository", "DISABLE_MIRRORS", "mirror", "ENABLED", "v1.19"},
37+
{"server", "LFS_CONTENT_PATH", "lfs", "PATH", "v1.19"},
3938
{"log", "XORM", "log", "logger.xorm.MODE", "1.21"},
4039
{"log", "ENABLE_XORM_LOG", "log", "logger.xorm.MODE", "1.21"},
4140
{"log", "ROUTER", "log", "logger.router.MODE", "1.21"},
@@ -45,19 +44,30 @@ func checkForRemovedSettings(rootCfg ConfigProvider) error {
4544
}
4645

4746
for _, rs := range removedSettings {
48-
errs = append(errs, removedSetting(rootCfg, rs.oldSection, rs.oldKey, rs.newSection, rs.newKey, rs.version))
47+
if err := removedSetting(rootCfg, rs.oldSection, rs.oldKey, rs.newSection, rs.newKey, rs.version); err != nil {
48+
errs = append(errs, err)
49+
}
4950
}
5051

51-
if rootCfg.Section("mailer").Key("PROTOCOL").String() == "smtp+startls" {
52-
errs = append(errs, fmt.Errorf("removed fallback `[mailer]` `PROTOCOL = smtp+startls` present. Use `[mailer]` `PROTOCOL = smtp+starttls`` instead"))
52+
if err := removedOption(rootCfg, "mailer", "PROTOCOL", "smtp+startls", "smtp+starttls", "v1.19"); err != nil {
53+
errs = append(errs, err)
5354
}
5455

55-
return errors.Join(errs...)
56+
return errs
5657
}
5758

59+
// removedOption checks if configuration has an oldValue under key in section and returns error for user if it does
60+
func removedOption(rootCfg ConfigProvider, section, key, oldValue, newValue, version string) error {
61+
if rootCfg.Section(section).Key(key).String() == oldValue {
62+
return fmt.Errorf("Config option `[%s].%s=%s` was removed in %s. Please use `[%s].%s=%s` instead", section, key, oldValue, version, section, key, newValue)
63+
}
64+
return nil
65+
}
66+
67+
// removedSetting checks if oldKey exists in oldSection and returns an error for user if it does
5868
func removedSetting(rootCfg ConfigProvider, oldSection, oldKey, newSection, newKey, version string) error {
5969
if rootCfg.Section(oldSection).HasKey(oldKey) {
60-
return fmt.Errorf("Config option `[%s].%s` was removed in %s. Please use `[%s].%s`", oldSection, oldKey, version, newSection, newKey)
70+
return fmt.Errorf("Config option `[%s].%s` was removed in %s. Please use `[%s].%s` instead", oldSection, oldKey, version, newSection, newKey)
6171
}
6272
return nil
6373
}

‎modules/setting/setting.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package setting
66

77
import (
8+
"errors"
89
"fmt"
910
"os"
1011
"runtime"
@@ -200,7 +201,7 @@ func mustCurrentRunUserMatch(rootCfg ConfigProvider) {
200201
// LoadSettings initializes the settings for normal start up
201202
func LoadSettings() {
202203
initAllLoggers()
203-
if err := checkForRemovedSettings(CfgProvider); err != nil {
204+
if err := errors.Join(checkForRemovedSettings(CfgProvider)...); err != nil {
204205
log.Fatal("Encountered removed settings while loading configuration: %v", err)
205206
}
206207

0 commit comments

Comments
 (0)