From 4b30bc6dd3bcd5e1e03d28755a4a4ef0817c9df3 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 23 Feb 2025 18:56:35 +0100 Subject: [PATCH 1/3] Change test to use named struct fields instead of positional fields This makes the tests a little bit easier to read, the multi-line string literals make this otherwise a little difficult. --- pkg/config/app_config_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/config/app_config_test.go b/pkg/config/app_config_test.go index 044161104..72bc5f7cc 100644 --- a/pkg/config/app_config_test.go +++ b/pkg/config/app_config_test.go @@ -14,24 +14,24 @@ func TestCommitPrefixMigrations(t *testing.T) { expected string }{ { - "Empty String", - "", - "", + name: "Empty String", + input: "", + expected: "", }, { - "Single CommitPrefix Rename", - ` + name: "Single CommitPrefix Rename", + input: ` git: commitPrefix: pattern: "^\\w+-\\w+.*" replace: '[JIRA $0] '`, - ` + expected: ` git: commitPrefix: - pattern: "^\\w+-\\w+.*" replace: '[JIRA $0] '`, }, { - "Complicated CommitPrefixes Rename", - ` + name: "Complicated CommitPrefixes Rename", + input: ` git: commitPrefixes: foo: @@ -40,7 +40,7 @@ git: CrazyName!@#$^*&)_-)[[}{f{[]: pattern: "^foo.bar*" replace: '[FUN $0] '`, - ` + expected: ` git: commitPrefixes: foo: @@ -50,9 +50,9 @@ git: - pattern: "^foo.bar*" replace: '[FUN $0] '`, }, { - "Incomplete Configuration", - "git:", - "git:", + name: "Incomplete Configuration", + input: "git:", + expected: "git:", }, } From 38ab7ebefb444af75f3799fd91cac40d52c9ce50 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 23 Feb 2025 18:51:35 +0100 Subject: [PATCH 2/3] Change TestCommitPrefixMigrations to compare only strings --- pkg/config/app_config_test.go | 47 ++++++++++++++--------------------- 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/pkg/config/app_config_test.go b/pkg/config/app_config_test.go index 72bc5f7cc..4a6ce0ee6 100644 --- a/pkg/config/app_config_test.go +++ b/pkg/config/app_config_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "gopkg.in/yaml.v3" ) func TestCommitPrefixMigrations(t *testing.T) { @@ -19,36 +18,36 @@ func TestCommitPrefixMigrations(t *testing.T) { expected: "", }, { name: "Single CommitPrefix Rename", - input: ` -git: + input: `git: commitPrefix: pattern: "^\\w+-\\w+.*" - replace: '[JIRA $0] '`, - expected: ` -git: + replace: '[JIRA $0] ' +`, + expected: `git: commitPrefix: - pattern: "^\\w+-\\w+.*" - replace: '[JIRA $0] '`, + replace: '[JIRA $0] ' +`, }, { name: "Complicated CommitPrefixes Rename", - input: ` -git: + input: `git: commitPrefixes: foo: pattern: "^\\w+-\\w+.*" replace: '[OTHER $0] ' CrazyName!@#$^*&)_-)[[}{f{[]: pattern: "^foo.bar*" - replace: '[FUN $0] '`, - expected: ` -git: + replace: '[FUN $0] ' +`, + expected: `git: commitPrefixes: - foo: - - pattern: "^\\w+-\\w+.*" - replace: '[OTHER $0] ' - CrazyName!@#$^*&)_-)[[}{f{[]: - - pattern: "^foo.bar*" - replace: '[FUN $0] '`, + foo: + - pattern: "^\\w+-\\w+.*" + replace: '[OTHER $0] ' + CrazyName!@#$^*&)_-)[[}{f{[]: + - pattern: "^foo.bar*" + replace: '[FUN $0] ' +`, }, { name: "Incomplete Configuration", input: "git:", @@ -58,21 +57,11 @@ git: for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { - expectedConfig := GetDefaultConfig() - err := yaml.Unmarshal([]byte(s.expected), expectedConfig) - if err != nil { - t.Error(err) - } actual, err := computeMigratedConfig("path doesn't matter", []byte(s.input)) if err != nil { t.Error(err) } - actualConfig := GetDefaultConfig() - err = yaml.Unmarshal(actual, actualConfig) - if err != nil { - t.Error(err) - } - assert.Equal(t, expectedConfig, actualConfig) + assert.Equal(t, s.expected, string(actual)) }) } } From 72b9e8328de493d2612d17c8fa70aa81b20f6e33 Mon Sep 17 00:00:00 2001 From: Chris McDonnell Date: Sat, 22 Feb 2025 20:35:08 -0500 Subject: [PATCH 3/3] Make commit prefixes migration only return true if it enters if statement --- pkg/config/app_config.go | 6 +++--- pkg/config/app_config_test.go | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index cfdc75e31..e7d884dbb 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -306,6 +306,7 @@ func changeElementToSequence(changedContent []byte, path []string) ([]byte, erro func changeCommitPrefixesMap(changedContent []byte) ([]byte, error) { return yaml_utils.TransformNode(changedContent, []string{"git", "commitPrefixes"}, func(prefixesNode *yaml.Node) (bool, error) { + changedAnyNodes := false if prefixesNode.Kind == yaml.MappingNode { for _, contentNode := range prefixesNode.Content { if contentNode.Kind == yaml.MappingNode { @@ -317,12 +318,11 @@ func changeCommitPrefixesMap(changedContent []byte) ([]byte, error) { Kind: yaml.MappingNode, Content: nodeContentCopy, }} - + changedAnyNodes = true } } - return true, nil } - return false, nil + return changedAnyNodes, nil }) } diff --git a/pkg/config/app_config_test.go b/pkg/config/app_config_test.go index 4a6ce0ee6..cf72f201b 100644 --- a/pkg/config/app_config_test.go +++ b/pkg/config/app_config_test.go @@ -52,6 +52,28 @@ func TestCommitPrefixMigrations(t *testing.T) { name: "Incomplete Configuration", input: "git:", expected: "git:", + }, { + // This test intentionally uses non-standard indentation to test that the migration + // does not change the input. + name: "No changes made when already migrated", + input: ` +git: + commitPrefix: + - pattern: "Hello World" + replace: "Goodbye" + commitPrefixes: + foo: + - pattern: "^\\w+-\\w+.*" + replace: '[JIRA $0] '`, + expected: ` +git: + commitPrefix: + - pattern: "Hello World" + replace: "Goodbye" + commitPrefixes: + foo: + - pattern: "^\\w+-\\w+.*" + replace: '[JIRA $0] '`, }, }