From efbbbd5dd0bf98305907adf5b07d93d458ec0ddc Mon Sep 17 00:00:00 2001 From: Artem Yelizarov <52959979+artem-y@users.noreply.github.com> Date: Sun, 28 Apr 2024 01:02:28 +0300 Subject: [PATCH 1/7] Add simple unit testing workflow --- tests/.github/workflows/run-tests.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 tests/.github/workflows/run-tests.yml diff --git a/tests/.github/workflows/run-tests.yml b/tests/.github/workflows/run-tests.yml new file mode 100644 index 0000000..b68fcc8 --- /dev/null +++ b/tests/.github/workflows/run-tests.yml @@ -0,0 +1,22 @@ +name: All Tests + +on: + push: + branches: [ "main" ] + pull_request: + branches: [ "main" ] + +jobs: + run-tests: + if: github.event.pull_request.draft == false + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v4 + with: + go-version: '1.22' + + - name: Test + run: make test From 48e863c7e66dfe1f9a6fbd46a77bd03a842a36e2 Mon Sep 17 00:00:00 2001 From: Artem Yelizarov <52959979+artem-y@users.noreply.github.com> Date: Sun, 28 Apr 2024 06:53:28 +0300 Subject: [PATCH 2/7] Add tests for config package --- cmd/commit/main.go | 7 +- internal/config/config.go | 17 +- internal/config/config_test.go | 196 ++++++++++++++++++++++++ internal/config/filereader.go | 21 +++ internal/config/mocks/filereadermock.go | 39 +++++ 5 files changed, 268 insertions(+), 12 deletions(-) create mode 100644 internal/config/config_test.go create mode 100644 internal/config/filereader.go create mode 100644 internal/config/mocks/filereadermock.go diff --git a/cmd/commit/main.go b/cmd/commit/main.go index f32af1d..607fe4c 100644 --- a/cmd/commit/main.go +++ b/cmd/commit/main.go @@ -53,7 +53,12 @@ func main() { // Read branch name or HEAD if headRef.Name().IsBranch() { - cfg := config.ReadCommitConfig(configFilePath) + fileReader := config.FileReader{} + cfg, err := config.ReadCommitConfig(fileReader, configFilePath) + if err != nil { + fmt.Fprintf(os.Stderr, helpers.Red("Failed to read config: %v\n"), err) + os.Exit(1) + } branchName := headRef.Name().Short() matches := findIssueMatchesInBranch(cfg.IssueRegex, branchName) diff --git a/internal/config/config.go b/internal/config/config.go index bf85f4d..6e62270 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,8 +2,6 @@ package config import ( "encoding/json" - "fmt" - "os" "github.com/artem-y/commit/internal/helpers" ) @@ -17,23 +15,20 @@ type CommitConfig struct { } // Reads config at the file path and unmarshals it into commitConfig struct -func ReadCommitConfig(configFilePath string) CommitConfig { - +func ReadCommitConfig(fileReader FileReading, configFilePath string) (CommitConfig, error) { var cfg CommitConfig - _, err := os.Stat(configFilePath) + _, err := fileReader.Stat(configFilePath) if err == nil { - file, err := os.ReadFile(configFilePath) + file, err := fileReader.ReadFile(configFilePath) if err != nil { - fmt.Fprintf(os.Stderr, helpers.Red("Error reading %s file: %v\n"), err, configFilePath) - os.Exit(1) + return cfg, err } err = json.Unmarshal(file, &cfg) if err != nil { - fmt.Fprintf(os.Stderr, helpers.Red("Error unmarshalling %s file: %v\n"), err, configFilePath) - os.Exit(1) + return cfg, err } } @@ -61,5 +56,5 @@ func ReadCommitConfig(configFilePath string) CommitConfig { cfg.OutputStringSuffix = &defaultStringSuffix } - return cfg + return cfg, nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000..c9c39c5 --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,196 @@ +package config_test + +import ( + "errors" + "fmt" + "reflect" + "testing" + + "github.com/artem-y/commit/internal/config" + "github.com/artem-y/commit/internal/config/mocks" + "github.com/artem-y/commit/internal/helpers" +) + +func Test_ReadCommitConfig_WhenFileDoesNotExist_ReturnsDefaultConfig(t *testing.T) { + // Arrange + var mock *mocks.FileReadingMock = &mocks.FileReadingMock{} + err := errors.New("file does not exist") + mock.Results.Stat.Error = err + mock.Results.ReadFile.Error = err + + defaultConfig := makeDefaultConfig() + + // Act + cfg, err := config.ReadCommitConfig(mock, "some/path") + + // Assert + for _, invocation := range mock.Invocations { + if invocation == mocks.InvocationReadFile { + t.Error("Didn't expect trying to read file when the file does not exist") + } + } + + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + if !reflect.DeepEqual(cfg, defaultConfig) { + t.Errorf("Expected default config, got %s", describe(cfg)) + } +} + +func Test_ReadCommitConfig_WhenFilledWithValidSettings_LoadsAllValuesFromConfig(t *testing.T) { + // Arrange + var mock *mocks.FileReadingMock = &mocks.FileReadingMock{} + issueRegex := "XY[0-9]+" + outputIssuePrefix := "(" + outputIssueSuffix := ")" + outputStringPrefix := "[ " + outputStringSuffix := " ]" + + expectedConfig := config.CommitConfig{ + IssueRegex: issueRegex, + OutputIssuePrefix: &outputIssuePrefix, + OutputIssueSuffix: &outputIssueSuffix, + OutputStringPrefix: &outputStringPrefix, + OutputStringSuffix: &outputStringSuffix, + } + configJson := makeJSONFromConfig(expectedConfig) + + mock.Results.ReadFile.Success = []byte(configJson) + + // Act + cfg, err := config.ReadCommitConfig(mock, "some/path") + + // Assert + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + if !reflect.DeepEqual(cfg, expectedConfig) { + t.Errorf( + "Expected `%s`, got `%s`", + describe(expectedConfig), + describe(cfg), + ) + } +} + +func Test_ReadCommitConfig_WhenInvalidJson_ReturnsError(t *testing.T) { + // Arrange + var mock *mocks.FileReadingMock = &mocks.FileReadingMock{} + mock.Results.ReadFile.Success = []byte("{invalid json}") + + // Act + _, err := config.ReadCommitConfig(mock, "some/path") + + // Assert + if err == nil { + t.Error("Expected an error, got `nil`") + } +} + +func Test_ReadCommitConfig_WhenFailedToReadFile_ReturnsError(t *testing.T) { + // Arrange + var mock *mocks.FileReadingMock = &mocks.FileReadingMock{} + mock.Results.ReadFile.Error = errors.New("failed to read file") + + // Act + _, err := config.ReadCommitConfig(mock, "some/path") + + // Assert + if err == nil { + t.Errorf("Expected error 'failed to read file', got '%v'", err) + return + } + if err.Error() != "failed to read file" { + t.Errorf("Expected error 'failed to read file', got '%v'", err) + } +} + +func Test_ReadCommitConfig_WhenOnlyRegexInConfix_ReturnsConfigWithRegex(t *testing.T) { + // Arrange + var mock *mocks.FileReadingMock = &mocks.FileReadingMock{} + expectedRegex := "ABC-[0-9]+" + configJson := fmt.Sprintf("{\"issueRegex\":\"%s\"}", expectedRegex) + mock.Results.ReadFile.Success = []byte(configJson) + + expectedConfig := makeDefaultConfig() + expectedConfig.IssueRegex = expectedRegex + + // Act + cfg, err := config.ReadCommitConfig(mock, "some/path") + + // Assert + if err != nil { + t.Errorf("Expected no error, got %v", err) + } + + if cfg.IssueRegex != expectedRegex { + t.Errorf( + "Expected regex ('%s') in config, got '%s'", + expectedRegex, + cfg.IssueRegex, + ) + } + + if !reflect.DeepEqual(cfg, expectedConfig) { + t.Errorf( + "Expected config:\n'%s'\nActual config:\n'%s'", + describe(expectedConfig), + describe(cfg), + ) + } +} + +// Helper function to create a default config +func makeDefaultConfig() config.CommitConfig { + outputIssuePrefix := helpers.DEFAULT_OUTPUT_ISSUE_PREFIX + outputIssueSuffix := helpers.DEFAULT_OUTPUT_ISSUE_SUFFIX + outputStringPrefix := helpers.DEFAULT_OUTPUT_STRING_PREFIX + outputStringSuffix := helpers.DEFAULT_OUTPUT_STRING_SUFFIX + + return config.CommitConfig{ + IssueRegex: helpers.DEFAULT_ISSUE_REGEX, + OutputIssuePrefix: &outputIssuePrefix, + OutputIssueSuffix: &outputIssueSuffix, + OutputStringPrefix: &outputStringPrefix, + OutputStringSuffix: &outputStringSuffix, + } +} + +// Helper function to make a human-readable description for a config +func describe(cfg config.CommitConfig) string { + return fmt.Sprintf( + `{ + IssueRegex: '%s', + OutputIssuePrefix: '%s', + OutputIssueSuffix: '%s', + OutputStringPrefix: '%s', + OutputStringSuffix: '%s' + }`, + cfg.IssueRegex, + *cfg.OutputIssuePrefix, + *cfg.OutputIssueSuffix, + *cfg.OutputStringPrefix, + *cfg.OutputStringSuffix, + ) +} + +// Helper function to create a JSON string from a config +func makeJSONFromConfig(cfg config.CommitConfig) string { + return fmt.Sprintf( + `{ + "issueRegex": "%s", + "outputIssuePrefix": "%s", + "outputIssueSuffix": "%s", + "outputStringPrefix": "%s", + "outputStringSuffix": "%s" + }`, + cfg.IssueRegex, + *cfg.OutputIssuePrefix, + *cfg.OutputIssueSuffix, + *cfg.OutputStringPrefix, + *cfg.OutputStringSuffix, + ) +} diff --git a/internal/config/filereader.go b/internal/config/filereader.go new file mode 100644 index 0000000..77dc9a7 --- /dev/null +++ b/internal/config/filereader.go @@ -0,0 +1,21 @@ +package config + +import ( + "io/fs" + "os" +) + +type FileReading interface { + ReadFile(filename string) ([]byte, error) + Stat(filename string) (fs.FileInfo, error) +} + +type FileReader struct{} + +func (FileReader) ReadFile(filename string) ([]byte, error) { + return os.ReadFile(filename) +} + +func (FileReader) Stat(filename string) (fs.FileInfo, error) { + return os.Stat(filename) +} diff --git a/internal/config/mocks/filereadermock.go b/internal/config/mocks/filereadermock.go new file mode 100644 index 0000000..b5c8e25 --- /dev/null +++ b/internal/config/mocks/filereadermock.go @@ -0,0 +1,39 @@ +package mocks + +import ( + "io/fs" +) + +type FileReadingMock struct { + Invocations []string + + Results struct { + ReadFile struct { + Success []byte + Error error + } + Stat struct { + Success fs.FileInfo + Error error + } + } +} + +const ( + InvocationReadFile = "ReadFile" + InvocationStat = "Stat" +) + +func (mock *FileReadingMock) Reset() { + mock.Invocations = nil +} + +func (mock *FileReadingMock) ReadFile(filename string) ([]byte, error) { + mock.Invocations = append(mock.Invocations, InvocationReadFile) + return mock.Results.ReadFile.Success, mock.Results.ReadFile.Error +} + +func (mock *FileReadingMock) Stat(filename string) (fs.FileInfo, error) { + mock.Invocations = append(mock.Invocations, InvocationStat) + return mock.Results.Stat.Success, mock.Results.Stat.Error +} From c68e219ed3a8ba8dce377d76c332459d3279d04d Mon Sep 17 00:00:00 2001 From: Artem Yelizarov <52959979+artem-y@users.noreply.github.com> Date: Sun, 28 Apr 2024 06:54:53 +0300 Subject: [PATCH 3/7] Fix invocation of all tests --- {tests/.github => .github}/workflows/run-tests.yml | 0 Makefile | 2 +- {tests => internal/helpers}/helpers_test.go | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename {tests/.github => .github}/workflows/run-tests.yml (100%) rename {tests => internal/helpers}/helpers_test.go (94%) diff --git a/tests/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml similarity index 100% rename from tests/.github/workflows/run-tests.yml rename to .github/workflows/run-tests.yml diff --git a/Makefile b/Makefile index d43ce8c..ac51767 100644 --- a/Makefile +++ b/Makefile @@ -52,7 +52,7 @@ run: # Run the tests .PHONY: test test: - @go test -v ./tests/ + @go test -v ./... %: @: diff --git a/tests/helpers_test.go b/internal/helpers/helpers_test.go similarity index 94% rename from tests/helpers_test.go rename to internal/helpers/helpers_test.go index 1d3daa3..bc1d4a4 100644 --- a/tests/helpers_test.go +++ b/internal/helpers/helpers_test.go @@ -1,4 +1,4 @@ -package tests +package helpers_test import ( "fmt" From 76cc4e064b15284d184c9b8f8c2fe2ff7633278c Mon Sep 17 00:00:00 2001 From: Artem Yelizarov <52959979+artem-y@users.noreply.github.com> Date: Sun, 28 Apr 2024 07:11:55 +0300 Subject: [PATCH 4/7] Rename file reader files --- internal/config/{filereader.go => file_reader.go} | 0 internal/config/mocks/{filereadermock.go => file_reader_mock.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename internal/config/{filereader.go => file_reader.go} (100%) rename internal/config/mocks/{filereadermock.go => file_reader_mock.go} (100%) diff --git a/internal/config/filereader.go b/internal/config/file_reader.go similarity index 100% rename from internal/config/filereader.go rename to internal/config/file_reader.go diff --git a/internal/config/mocks/filereadermock.go b/internal/config/mocks/file_reader_mock.go similarity index 100% rename from internal/config/mocks/filereadermock.go rename to internal/config/mocks/file_reader_mock.go From f2b62c8dd84c3adc3e36e37fccc23c1fdf4cdfea Mon Sep 17 00:00:00 2001 From: Artem Yelizarov <52959979+artem-y@users.noreply.github.com> Date: Sun, 28 Apr 2024 17:58:26 +0300 Subject: [PATCH 5/7] Separate message generating logic, add unit tests --- cmd/commit/main.go | 50 +------ .../message_generator/message_generator.go | 68 +++++++++ .../message_generator_test.go | 131 ++++++++++++++++++ 3 files changed, 205 insertions(+), 44 deletions(-) create mode 100644 internal/message_generator/message_generator.go create mode 100644 internal/message_generator/message_generator_test.go diff --git a/cmd/commit/main.go b/cmd/commit/main.go index 607fe4c..854a9ce 100644 --- a/cmd/commit/main.go +++ b/cmd/commit/main.go @@ -5,12 +5,11 @@ import ( "fmt" "os" "path/filepath" - "regexp" - "strings" "time" "github.com/artem-y/commit/internal/config" "github.com/artem-y/commit/internal/helpers" + "github.com/artem-y/commit/internal/message_generator" "github.com/artem-y/commit/internal/user" "github.com/go-git/go-git/v5" @@ -59,12 +58,13 @@ func main() { fmt.Fprintf(os.Stderr, helpers.Red("Failed to read config: %v\n"), err) os.Exit(1) } - branchName := headRef.Name().Short() - matches := findIssueMatchesInBranch(cfg.IssueRegex, branchName) - if len(matches) > 0 { - commitMessage = generateCommitMessageWithMatches(matches, cfg, commitMessage) + messageGenerator := message_generator.MessageGenerator{ + BranchName: headRef.Name().Short(), + UserMessage: commitMessage, + Config: cfg, } + commitMessage = messageGenerator.GenerateMessage() if !dryRun { commitChanges(repo, worktree, commitMessage) @@ -124,44 +124,6 @@ func openWorktree(repo *git.Repository) *git.Worktree { return worktree } -// Searches the branch name for issue numbers matching the given regex -func findIssueMatchesInBranch(rgxRaw string, branchName string) []string { - rgx := regexp.MustCompile(rgxRaw) - - allSubmatches := rgx.FindAllStringSubmatch(branchName, -1) - - matches := []string{} - for _, submatches := range allSubmatches { - matches = append(matches, submatches[0]) - } - - return matches -} - -// Generates a commit message with the issue number matches and config settings -func generateCommitMessageWithMatches(matches []string, cfg config.CommitConfig, commitMessage string) string { - mappedMatches := make([]string, len(matches)) - - for index, match := range matches { - wrappedIssueNumber := fmt.Sprintf( - "%s%s%s", - *cfg.OutputIssuePrefix, - match, - *cfg.OutputIssueSuffix, - ) - mappedMatches[index] = wrappedIssueNumber - } - - joinedIssues := strings.Join(mappedMatches, ", ") - return fmt.Sprintf( - "%s%s%s%s", - *cfg.OutputStringPrefix, - joinedIssues, - *cfg.OutputStringSuffix, - commitMessage, - ) -} - // Creates commit options with the author information func makeCommitOptions(usr user.User) git.CommitOptions { return git.CommitOptions{ diff --git a/internal/message_generator/message_generator.go b/internal/message_generator/message_generator.go new file mode 100644 index 0000000..3b370da --- /dev/null +++ b/internal/message_generator/message_generator.go @@ -0,0 +1,68 @@ +package message_generator + +import ( + "fmt" + "regexp" + "strings" + + "github.com/artem-y/commit/internal/config" +) + +type MessageGenerator struct { + BranchName string + UserMessage string + Config config.CommitConfig +} + +// Generates a commit message based on the branch name and user message +func (generator *MessageGenerator) GenerateMessage() string { + cfg := generator.Config + branchName := generator.BranchName + commitMessage := generator.UserMessage + + matches := findIssueMatchesInBranch(cfg.IssueRegex, branchName) + + if len(matches) > 0 { + commitMessage = generateCommitMessageWithMatches(matches, cfg, commitMessage) + } + + return commitMessage +} + +// Searches the branch name for issue numbers matching the given regex +func findIssueMatchesInBranch(rgxRaw string, branchName string) []string { + rgx := regexp.MustCompile(rgxRaw) + + allSubmatches := rgx.FindAllStringSubmatch(branchName, -1) + + matches := []string{} + for _, submatches := range allSubmatches { + matches = append(matches, submatches[0]) + } + + return matches +} + +// Generates a commit message with the issue number matches and config settings +func generateCommitMessageWithMatches(matches []string, cfg config.CommitConfig, commitMessage string) string { + mappedMatches := make([]string, len(matches)) + + for index, match := range matches { + wrappedIssueNumber := fmt.Sprintf( + "%s%s%s", + *cfg.OutputIssuePrefix, + match, + *cfg.OutputIssueSuffix, + ) + mappedMatches[index] = wrappedIssueNumber + } + + joinedIssues := strings.Join(mappedMatches, ", ") + return fmt.Sprintf( + "%s%s%s%s", + *cfg.OutputStringPrefix, + joinedIssues, + *cfg.OutputStringSuffix, + commitMessage, + ) +} diff --git a/internal/message_generator/message_generator_test.go b/internal/message_generator/message_generator_test.go new file mode 100644 index 0000000..a462a4a --- /dev/null +++ b/internal/message_generator/message_generator_test.go @@ -0,0 +1,131 @@ +package message_generator_test + +import ( + "testing" + + "github.com/artem-y/commit/internal/config" + "github.com/artem-y/commit/internal/message_generator" +) + +func Test_Generate_WhenNoMatchesInBranchName_ReturnsUserCommitMessageUnchanged(t *testing.T) { + // Arrange + outputIssuePrefix := "(" + outputIssueSuffix := ")" + outputStringPrefix := "[ " + outputStringSuffix := " ]" + + expectedMessage := "Test commit message" + + sut := message_generator.MessageGenerator{ + BranchName: "main", + UserMessage: expectedMessage, + Config: config.CommitConfig{ + IssueRegex: "XY[0-9]+", + OutputIssuePrefix: &outputIssuePrefix, + OutputIssueSuffix: &outputIssueSuffix, + OutputStringPrefix: &outputStringPrefix, + OutputStringSuffix: &outputStringSuffix, + }, + } + + // Act + message := sut.GenerateMessage() + + // Assert + if message != expectedMessage { + t.Errorf("Expected message '%s', got '%s'", expectedMessage, message) + } +} + +func Test_Generate_WhenSingleMatchFound_AddsIssueToTheMessage(t *testing.T) { + // Arrange + outputIssuePrefix := "(" + outputIssueSuffix := ")" + outputStringPrefix := "" + outputStringSuffix := " " + + userMessage := "Add validation service" + expectedMessage := "(CD-13) Add validation service" + + sut := message_generator.MessageGenerator{ + BranchName: "feature/CD-13-implement-login-screen-validation", + UserMessage: userMessage, + Config: config.CommitConfig{ + IssueRegex: "CD-[0-9]+", + OutputIssuePrefix: &outputIssuePrefix, + OutputIssueSuffix: &outputIssueSuffix, + OutputStringPrefix: &outputStringPrefix, + OutputStringSuffix: &outputStringSuffix, + }, + } + + // Act + message := sut.GenerateMessage() + + // Assert + if message != expectedMessage { + t.Errorf("Expected message '%s', got '%s'", expectedMessage, message) + } +} + +func Test_Generate_WhenFoundMultipleMatches_AddsCommaSeparatedIssuesToTheMessage(t *testing.T) { + // Arrange + outputIssuePrefix := "#" + outputIssueSuffix := "" + outputStringPrefix := "[" + outputStringSuffix := "]: " + + userMessage := "Prepare mocks for core unit tests" + expectedMessage := "[#27, #30]: Prepare mocks for core unit tests" + + sut := message_generator.MessageGenerator{ + BranchName: "add-unit-tests-for-issues-27-and-30", + UserMessage: userMessage, + Config: config.CommitConfig{ + IssueRegex: "[0-9]+", + OutputIssuePrefix: &outputIssuePrefix, + OutputIssueSuffix: &outputIssueSuffix, + OutputStringPrefix: &outputStringPrefix, + OutputStringSuffix: &outputStringSuffix, + }, + } + + // Act + message := sut.GenerateMessage() + + // Assert + if message != expectedMessage { + t.Errorf("Expected message '%s', got '%s'", expectedMessage, message) + } +} + +func Test_Generate_WhenAllPrefixesAndSuffixesEmpty_AddsIssueWithoutWrapping(t *testing.T) { + // Arrange + outputIssuePrefix := "" + outputIssueSuffix := "" + outputStringPrefix := "" + outputStringSuffix := "" + + userMessage := "chore: regenerate localisation files" + expectedMessage := "#210chore: regenerate localisation files" + + sut := message_generator.MessageGenerator{ + BranchName: "#210-implement-login-screen-validation", + UserMessage: userMessage, + Config: config.CommitConfig{ + IssueRegex: "(#)?[0-9]+", + OutputIssuePrefix: &outputIssuePrefix, + OutputIssueSuffix: &outputIssueSuffix, + OutputStringPrefix: &outputStringPrefix, + OutputStringSuffix: &outputStringSuffix, + }, + } + + // Act + message := sut.GenerateMessage() + + // Assert + if message != expectedMessage { + t.Errorf("Expected message '%s', got '%s'", expectedMessage, message) + } +} From 189bedb420738e89f26dbee4181df2ad81182d7f Mon Sep 17 00:00:00 2001 From: Artem Yelizarov <52959979+artem-y@users.noreply.github.com> Date: Sun, 28 Apr 2024 18:45:11 +0300 Subject: [PATCH 6/7] Use safe config struct, minor refactoring and cleanup --- internal/config/config.go | 67 +++++++++----- internal/config/config_test.go | 88 ++++++++----------- internal/config/file_reader.go | 1 + .../message_generator/message_generator.go | 8 +- .../message_generator_test.go | 52 ++++------- 5 files changed, 104 insertions(+), 112 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 6e62270..e145896 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,8 +6,18 @@ import ( "github.com/artem-y/commit/internal/helpers" ) +// Representation of settings that can be set from the config file type CommitConfig struct { - IssueRegex string `json:"issueRegex"` + IssueRegex string // Regex for issue numbers in branch name + OutputIssuePrefix string // Prefix before each issue number in the commit message + OutputIssueSuffix string // Suffix after each issue number in the commit message + OutputStringPrefix string // Prefix before the list of issues in the commit message + OutputStringSuffix string // Suffix after the list of issues and before the user's message +} + +// DTO for unmarshalling JSON config and safe-guarding against nil values +type commitConfigDTO struct { + IssueRegex *string `json:"issueRegex"` OutputIssuePrefix *string `json:"outputIssuePrefix"` OutputIssueSuffix *string `json:"outputIssueSuffix"` OutputStringPrefix *string `json:"outputStringPrefix"` @@ -16,45 +26,62 @@ type CommitConfig struct { // Reads config at the file path and unmarshals it into commitConfig struct func ReadCommitConfig(fileReader FileReading, configFilePath string) (CommitConfig, error) { - var cfg CommitConfig + var cfgDto commitConfigDTO _, err := fileReader.Stat(configFilePath) if err == nil { file, err := fileReader.ReadFile(configFilePath) if err != nil { - return cfg, err + return CommitConfig{}, err } - err = json.Unmarshal(file, &cfg) + err = json.Unmarshal(file, &cfgDto) if err != nil { - return cfg, err + return CommitConfig{}, err } } - if cfg.IssueRegex == "" { - cfg.IssueRegex = helpers.DEFAULT_ISSUE_REGEX + cfg := makeConfig(cfgDto) + + return cfg, nil +} + +// Helper function to create a default config +func MakeDefaultConfig() CommitConfig { + return CommitConfig{ + IssueRegex: helpers.DEFAULT_ISSUE_REGEX, + OutputIssuePrefix: helpers.DEFAULT_OUTPUT_ISSUE_PREFIX, + OutputIssueSuffix: helpers.DEFAULT_OUTPUT_ISSUE_SUFFIX, + OutputStringPrefix: helpers.DEFAULT_OUTPUT_STRING_PREFIX, + OutputStringSuffix: helpers.DEFAULT_OUTPUT_STRING_SUFFIX, + } +} + +// MARK: - Private + +func makeConfig(cfgDto commitConfigDTO) CommitConfig { + cfg := MakeDefaultConfig() + + if cfgDto.IssueRegex != nil { + cfg.IssueRegex = *cfgDto.IssueRegex } - if cfg.OutputIssuePrefix == nil { - defaultIssuePrefix := helpers.DEFAULT_OUTPUT_ISSUE_PREFIX - cfg.OutputIssuePrefix = &defaultIssuePrefix + if cfgDto.OutputIssuePrefix != nil { + cfg.OutputIssuePrefix = *cfgDto.OutputIssuePrefix } - if cfg.OutputIssueSuffix == nil { - defaultIssueSuffix := helpers.DEFAULT_OUTPUT_ISSUE_SUFFIX - cfg.OutputIssueSuffix = &defaultIssueSuffix + if cfgDto.OutputIssueSuffix != nil { + cfg.OutputIssueSuffix = *cfgDto.OutputIssueSuffix } - if cfg.OutputStringPrefix == nil { - defaultStringPrefix := helpers.DEFAULT_OUTPUT_STRING_PREFIX - cfg.OutputStringPrefix = &defaultStringPrefix + if cfgDto.OutputStringPrefix != nil { + cfg.OutputStringPrefix = *cfgDto.OutputStringPrefix } - if cfg.OutputStringSuffix == nil { - defaultStringSuffix := helpers.DEFAULT_OUTPUT_STRING_SUFFIX - cfg.OutputStringSuffix = &defaultStringSuffix + if cfgDto.OutputStringSuffix != nil { + cfg.OutputStringSuffix = *cfgDto.OutputStringSuffix } - return cfg, nil + return cfg } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c9c39c5..a651fdb 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -18,7 +18,7 @@ func Test_ReadCommitConfig_WhenFileDoesNotExist_ReturnsDefaultConfig(t *testing. mock.Results.Stat.Error = err mock.Results.ReadFile.Error = err - defaultConfig := makeDefaultConfig() + defaultConfig := config.MakeDefaultConfig() // Act cfg, err := config.ReadCommitConfig(mock, "some/path") @@ -35,27 +35,22 @@ func Test_ReadCommitConfig_WhenFileDoesNotExist_ReturnsDefaultConfig(t *testing. } if !reflect.DeepEqual(cfg, defaultConfig) { - t.Errorf("Expected default config, got %s", describe(cfg)) + t.Errorf("Expected default config, got %s", makeJSON(cfg)) } } func Test_ReadCommitConfig_WhenFilledWithValidSettings_LoadsAllValuesFromConfig(t *testing.T) { // Arrange var mock *mocks.FileReadingMock = &mocks.FileReadingMock{} - issueRegex := "XY[0-9]+" - outputIssuePrefix := "(" - outputIssueSuffix := ")" - outputStringPrefix := "[ " - outputStringSuffix := " ]" expectedConfig := config.CommitConfig{ - IssueRegex: issueRegex, - OutputIssuePrefix: &outputIssuePrefix, - OutputIssueSuffix: &outputIssueSuffix, - OutputStringPrefix: &outputStringPrefix, - OutputStringSuffix: &outputStringSuffix, + IssueRegex: "XY[0-9]+", + OutputIssuePrefix: "(", + OutputIssueSuffix: ")", + OutputStringPrefix: "[ ", + OutputStringSuffix: " ]", } - configJson := makeJSONFromConfig(expectedConfig) + configJson := makeJSON(expectedConfig) mock.Results.ReadFile.Success = []byte(configJson) @@ -70,8 +65,8 @@ func Test_ReadCommitConfig_WhenFilledWithValidSettings_LoadsAllValuesFromConfig( if !reflect.DeepEqual(cfg, expectedConfig) { t.Errorf( "Expected `%s`, got `%s`", - describe(expectedConfig), - describe(cfg), + makeJSON(expectedConfig), + makeJSON(cfg), ) } } @@ -115,7 +110,7 @@ func Test_ReadCommitConfig_WhenOnlyRegexInConfix_ReturnsConfigWithRegex(t *testi configJson := fmt.Sprintf("{\"issueRegex\":\"%s\"}", expectedRegex) mock.Results.ReadFile.Success = []byte(configJson) - expectedConfig := makeDefaultConfig() + expectedConfig := config.MakeDefaultConfig() expectedConfig.IssueRegex = expectedRegex // Act @@ -137,48 +132,37 @@ func Test_ReadCommitConfig_WhenOnlyRegexInConfix_ReturnsConfigWithRegex(t *testi if !reflect.DeepEqual(cfg, expectedConfig) { t.Errorf( "Expected config:\n'%s'\nActual config:\n'%s'", - describe(expectedConfig), - describe(cfg), + makeJSON(expectedConfig), + makeJSON(cfg), ) } } -// Helper function to create a default config -func makeDefaultConfig() config.CommitConfig { - outputIssuePrefix := helpers.DEFAULT_OUTPUT_ISSUE_PREFIX - outputIssueSuffix := helpers.DEFAULT_OUTPUT_ISSUE_SUFFIX - outputStringPrefix := helpers.DEFAULT_OUTPUT_STRING_PREFIX - outputStringSuffix := helpers.DEFAULT_OUTPUT_STRING_SUFFIX - - return config.CommitConfig{ +func Test_MakeDefaultConfig_CreatesConfigWithDefaultValues(t *testing.T) { + // Arrange + expectedConfig := config.CommitConfig{ IssueRegex: helpers.DEFAULT_ISSUE_REGEX, - OutputIssuePrefix: &outputIssuePrefix, - OutputIssueSuffix: &outputIssueSuffix, - OutputStringPrefix: &outputStringPrefix, - OutputStringSuffix: &outputStringSuffix, + OutputIssuePrefix: helpers.DEFAULT_OUTPUT_ISSUE_PREFIX, + OutputIssueSuffix: helpers.DEFAULT_OUTPUT_ISSUE_SUFFIX, + OutputStringPrefix: helpers.DEFAULT_OUTPUT_STRING_PREFIX, + OutputStringSuffix: helpers.DEFAULT_OUTPUT_STRING_SUFFIX, } -} -// Helper function to make a human-readable description for a config -func describe(cfg config.CommitConfig) string { - return fmt.Sprintf( - `{ - IssueRegex: '%s', - OutputIssuePrefix: '%s', - OutputIssueSuffix: '%s', - OutputStringPrefix: '%s', - OutputStringSuffix: '%s' - }`, - cfg.IssueRegex, - *cfg.OutputIssuePrefix, - *cfg.OutputIssueSuffix, - *cfg.OutputStringPrefix, - *cfg.OutputStringSuffix, - ) + // Act + cfg := config.MakeDefaultConfig() + + // Assert + if !reflect.DeepEqual(cfg, expectedConfig) { + t.Errorf( + "Expected default config ('%s'), got '%s'", + makeJSON(expectedConfig), + makeJSON(cfg), + ) + } } // Helper function to create a JSON string from a config -func makeJSONFromConfig(cfg config.CommitConfig) string { +func makeJSON(cfg config.CommitConfig) string { return fmt.Sprintf( `{ "issueRegex": "%s", @@ -188,9 +172,9 @@ func makeJSONFromConfig(cfg config.CommitConfig) string { "outputStringSuffix": "%s" }`, cfg.IssueRegex, - *cfg.OutputIssuePrefix, - *cfg.OutputIssueSuffix, - *cfg.OutputStringPrefix, - *cfg.OutputStringSuffix, + cfg.OutputIssuePrefix, + cfg.OutputIssueSuffix, + cfg.OutputStringPrefix, + cfg.OutputStringSuffix, ) } diff --git a/internal/config/file_reader.go b/internal/config/file_reader.go index 77dc9a7..3c9bc10 100644 --- a/internal/config/file_reader.go +++ b/internal/config/file_reader.go @@ -10,6 +10,7 @@ type FileReading interface { Stat(filename string) (fs.FileInfo, error) } +// Simple facade around the os file reading functions type FileReader struct{} func (FileReader) ReadFile(filename string) ([]byte, error) { diff --git a/internal/message_generator/message_generator.go b/internal/message_generator/message_generator.go index 3b370da..2ddeba8 100644 --- a/internal/message_generator/message_generator.go +++ b/internal/message_generator/message_generator.go @@ -50,9 +50,9 @@ func generateCommitMessageWithMatches(matches []string, cfg config.CommitConfig, for index, match := range matches { wrappedIssueNumber := fmt.Sprintf( "%s%s%s", - *cfg.OutputIssuePrefix, + cfg.OutputIssuePrefix, match, - *cfg.OutputIssueSuffix, + cfg.OutputIssueSuffix, ) mappedMatches[index] = wrappedIssueNumber } @@ -60,9 +60,9 @@ func generateCommitMessageWithMatches(matches []string, cfg config.CommitConfig, joinedIssues := strings.Join(mappedMatches, ", ") return fmt.Sprintf( "%s%s%s%s", - *cfg.OutputStringPrefix, + cfg.OutputStringPrefix, joinedIssues, - *cfg.OutputStringSuffix, + cfg.OutputStringSuffix, commitMessage, ) } diff --git a/internal/message_generator/message_generator_test.go b/internal/message_generator/message_generator_test.go index a462a4a..e21a273 100644 --- a/internal/message_generator/message_generator_test.go +++ b/internal/message_generator/message_generator_test.go @@ -9,11 +9,6 @@ import ( func Test_Generate_WhenNoMatchesInBranchName_ReturnsUserCommitMessageUnchanged(t *testing.T) { // Arrange - outputIssuePrefix := "(" - outputIssueSuffix := ")" - outputStringPrefix := "[ " - outputStringSuffix := " ]" - expectedMessage := "Test commit message" sut := message_generator.MessageGenerator{ @@ -21,10 +16,10 @@ func Test_Generate_WhenNoMatchesInBranchName_ReturnsUserCommitMessageUnchanged(t UserMessage: expectedMessage, Config: config.CommitConfig{ IssueRegex: "XY[0-9]+", - OutputIssuePrefix: &outputIssuePrefix, - OutputIssueSuffix: &outputIssueSuffix, - OutputStringPrefix: &outputStringPrefix, - OutputStringSuffix: &outputStringSuffix, + OutputIssuePrefix: "(", + OutputIssueSuffix: ")", + OutputStringPrefix: "[ ", + OutputStringSuffix: " ]", }, } @@ -39,11 +34,6 @@ func Test_Generate_WhenNoMatchesInBranchName_ReturnsUserCommitMessageUnchanged(t func Test_Generate_WhenSingleMatchFound_AddsIssueToTheMessage(t *testing.T) { // Arrange - outputIssuePrefix := "(" - outputIssueSuffix := ")" - outputStringPrefix := "" - outputStringSuffix := " " - userMessage := "Add validation service" expectedMessage := "(CD-13) Add validation service" @@ -52,10 +42,10 @@ func Test_Generate_WhenSingleMatchFound_AddsIssueToTheMessage(t *testing.T) { UserMessage: userMessage, Config: config.CommitConfig{ IssueRegex: "CD-[0-9]+", - OutputIssuePrefix: &outputIssuePrefix, - OutputIssueSuffix: &outputIssueSuffix, - OutputStringPrefix: &outputStringPrefix, - OutputStringSuffix: &outputStringSuffix, + OutputIssuePrefix: "(", + OutputIssueSuffix: ")", + OutputStringPrefix: "", + OutputStringSuffix: " ", }, } @@ -70,11 +60,6 @@ func Test_Generate_WhenSingleMatchFound_AddsIssueToTheMessage(t *testing.T) { func Test_Generate_WhenFoundMultipleMatches_AddsCommaSeparatedIssuesToTheMessage(t *testing.T) { // Arrange - outputIssuePrefix := "#" - outputIssueSuffix := "" - outputStringPrefix := "[" - outputStringSuffix := "]: " - userMessage := "Prepare mocks for core unit tests" expectedMessage := "[#27, #30]: Prepare mocks for core unit tests" @@ -83,10 +68,10 @@ func Test_Generate_WhenFoundMultipleMatches_AddsCommaSeparatedIssuesToTheMessage UserMessage: userMessage, Config: config.CommitConfig{ IssueRegex: "[0-9]+", - OutputIssuePrefix: &outputIssuePrefix, - OutputIssueSuffix: &outputIssueSuffix, - OutputStringPrefix: &outputStringPrefix, - OutputStringSuffix: &outputStringSuffix, + OutputIssuePrefix: "#", + OutputIssueSuffix: "", + OutputStringPrefix: "[", + OutputStringSuffix: "]: ", }, } @@ -101,11 +86,6 @@ func Test_Generate_WhenFoundMultipleMatches_AddsCommaSeparatedIssuesToTheMessage func Test_Generate_WhenAllPrefixesAndSuffixesEmpty_AddsIssueWithoutWrapping(t *testing.T) { // Arrange - outputIssuePrefix := "" - outputIssueSuffix := "" - outputStringPrefix := "" - outputStringSuffix := "" - userMessage := "chore: regenerate localisation files" expectedMessage := "#210chore: regenerate localisation files" @@ -114,10 +94,10 @@ func Test_Generate_WhenAllPrefixesAndSuffixesEmpty_AddsIssueWithoutWrapping(t *t UserMessage: userMessage, Config: config.CommitConfig{ IssueRegex: "(#)?[0-9]+", - OutputIssuePrefix: &outputIssuePrefix, - OutputIssueSuffix: &outputIssueSuffix, - OutputStringPrefix: &outputStringPrefix, - OutputStringSuffix: &outputStringSuffix, + OutputIssuePrefix: "", + OutputIssueSuffix: "", + OutputStringPrefix: "", + OutputStringSuffix: "", }, } From a0b3ef14e9876b07881e3db29df0c054310b4c8c Mon Sep 17 00:00:00 2001 From: Artem Yelizarov <52959979+artem-y@users.noreply.github.com> Date: Sun, 28 Apr 2024 18:53:25 +0300 Subject: [PATCH 7/7] Move LICENSE up to the root of the repo, update README --- docs/LICENSE => LICENSE | 0 docs/README.md | 6 +++--- 2 files changed, 3 insertions(+), 3 deletions(-) rename docs/LICENSE => LICENSE (100%) diff --git a/docs/LICENSE b/LICENSE similarity index 100% rename from docs/LICENSE rename to LICENSE diff --git a/docs/README.md b/docs/README.md index da73ea6..4d1eef7 100644 --- a/docs/README.md +++ b/docs/README.md @@ -75,8 +75,8 @@ If the branch has multiple issues in its name, the tool will include them all, c For example, the branch named `add-tests-for-CR-127-and-CR-131-features`, the issue regex set to `[A-Z]{2}-[0-9]+`, and the "outputIssuePrefix" and "outputIssueSuffix" settings for the output set to `[` and `]:`, the generated commit message would start with the following: > [CR-127, CR-131]: ## Testing -So far I haven't added a lot of unit tests for this project, but I will be doing it in the future. -To run the tests, use the `make test` or `go test -v ./tests` commands. +So far only the core parts of the logic are covered with tests, but I will be adding more in the future. +To run the tests, use the `make test` or `go test -v ./...` commands. ### Debug To check what commit message will be generated without making the actual commit, there is a `-dry-run` flag that can be passed to the command: ```shell @@ -87,5 +87,5 @@ Originally I wrote this tool for myself in shell and Swift and used a lot on Mac If you find it useful and see that something's wrong or missing, feel free to raise issues and contribute to the project. When doing so, please follow the ususal code of conduct and contribution guidelines from GitHub, and just common sense. ## License -This repository is distributed under the MIT license (see [LICENSE.md](/docs/LICENSE)). +This repository is distributed under the MIT license (see [LICENSE](/LICENSE)). My tool uses [go-git](https://github.com/go-git/go-git) as a 3rd party dependency to work with git directly, you might want to check it out too.