diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml new file mode 100644 index 0000000..b68fcc8 --- /dev/null +++ b/.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 diff --git a/docs/LICENSE b/LICENSE similarity index 100% rename from docs/LICENSE rename to LICENSE 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/cmd/commit/main.go b/cmd/commit/main.go index f32af1d..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" @@ -53,13 +52,19 @@ func main() { // Read branch name or HEAD if headRef.Name().IsBranch() { - cfg := config.ReadCommitConfig(configFilePath) - branchName := headRef.Name().Short() - matches := findIssueMatchesInBranch(cfg.IssueRegex, branchName) + 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) + } - 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) @@ -119,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/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. diff --git a/internal/config/config.go b/internal/config/config.go index bf85f4d..e145896 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,14 +2,22 @@ package config import ( "encoding/json" - "fmt" - "os" "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"` @@ -17,48 +25,62 @@ type CommitConfig struct { } // Reads config at the file path and unmarshals it into commitConfig struct -func ReadCommitConfig(configFilePath string) CommitConfig { - - var cfg CommitConfig +func ReadCommitConfig(fileReader FileReading, configFilePath string) (CommitConfig, error) { + var cfgDto commitConfigDTO - _, 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 CommitConfig{}, err } - err = json.Unmarshal(file, &cfg) + err = json.Unmarshal(file, &cfgDto) if err != nil { - fmt.Fprintf(os.Stderr, helpers.Red("Error unmarshalling %s file: %v\n"), err, configFilePath) - os.Exit(1) + 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 diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000..a651fdb --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,180 @@ +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 := config.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", makeJSON(cfg)) + } +} + +func Test_ReadCommitConfig_WhenFilledWithValidSettings_LoadsAllValuesFromConfig(t *testing.T) { + // Arrange + var mock *mocks.FileReadingMock = &mocks.FileReadingMock{} + + expectedConfig := config.CommitConfig{ + IssueRegex: "XY[0-9]+", + OutputIssuePrefix: "(", + OutputIssueSuffix: ")", + OutputStringPrefix: "[ ", + OutputStringSuffix: " ]", + } + configJson := makeJSON(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`", + makeJSON(expectedConfig), + makeJSON(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 := config.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'", + makeJSON(expectedConfig), + makeJSON(cfg), + ) + } +} + +func Test_MakeDefaultConfig_CreatesConfigWithDefaultValues(t *testing.T) { + // Arrange + expectedConfig := config.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, + } + + // 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 makeJSON(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/file_reader.go b/internal/config/file_reader.go new file mode 100644 index 0000000..3c9bc10 --- /dev/null +++ b/internal/config/file_reader.go @@ -0,0 +1,22 @@ +package config + +import ( + "io/fs" + "os" +) + +type FileReading interface { + ReadFile(filename string) ([]byte, error) + Stat(filename string) (fs.FileInfo, error) +} + +// Simple facade around the os file reading functions +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/file_reader_mock.go b/internal/config/mocks/file_reader_mock.go new file mode 100644 index 0000000..b5c8e25 --- /dev/null +++ b/internal/config/mocks/file_reader_mock.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 +} 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" diff --git a/internal/message_generator/message_generator.go b/internal/message_generator/message_generator.go new file mode 100644 index 0000000..2ddeba8 --- /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..e21a273 --- /dev/null +++ b/internal/message_generator/message_generator_test.go @@ -0,0 +1,111 @@ +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 + expectedMessage := "Test commit message" + + sut := message_generator.MessageGenerator{ + BranchName: "main", + UserMessage: expectedMessage, + Config: config.CommitConfig{ + IssueRegex: "XY[0-9]+", + OutputIssuePrefix: "(", + OutputIssueSuffix: ")", + OutputStringPrefix: "[ ", + 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 + 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: "(", + OutputIssueSuffix: ")", + OutputStringPrefix: "", + 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 + 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: "#", + OutputIssueSuffix: "", + OutputStringPrefix: "[", + 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 + 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: "", + OutputIssueSuffix: "", + OutputStringPrefix: "", + OutputStringSuffix: "", + }, + } + + // Act + message := sut.GenerateMessage() + + // Assert + if message != expectedMessage { + t.Errorf("Expected message '%s', got '%s'", expectedMessage, message) + } +}