wakatime / semver-action

Auto-generate the next semantic version.
MIT License
11 stars 0 forks source link

Add logic to calculate semver #1

Closed gandarez closed 3 years ago

gandarez commented 3 years ago

This PR creates a new Action which creates a semantic version based on some info extracted from a git repository. Semver is calculated relying on merge message pattern adopted by Github. So it will look for Merge pull request #123 from wakatime/feature/any-feature in the given commit to evaluate which strategy will be used to bump the next version.

More detailed info about scenarios can be seen from here.

gandarez commented 3 years ago

This is quite some code and need some more time here. Will finish the review tomorrow.

No rush

gandarez commented 3 years ago

@dron22 fixes applied. Also I removed tag creation. There are three separate commits grouping them to maybe make the review easier.

gandarez commented 3 years ago

With the added tests, we have a good test coverage now. This is good. Also my intention with my comments also was, to make the structure a little bit simple (e.g. Run() function. Usually thinking about how to efficiently test some code will force you to do a good structuring of the code.

Now the overall size grew to an extend, where it is very hard for me to review it. I trust you that it works, but unfortunately, I cannot give a quality review here.

The review actually can be concentrated at source code and all file under testdata are just git files to simulate repositories.

dron22 commented 3 years ago

We have to clean up the structure. This is mainly what makes it hard to review. One central issue I see, is that Run() is the central piece, but it cannot be covered by unit tests. To illustrate what I mean, in the following an example, for how we can make it unit-testable using an interface:

type gitClient interface {
    CurrentBranch() (string, error)
    IsRepo() bool
    LatestTag() (string, error)
    SourceBranch(commitHash string) (string, error)
}

// Run generates a semantic version using the commit sha.
func Run() (Result, error) {
    params, err := LoadParams()
    if err != nil {
        return Result{}, fmt.Errorf("failed to load parameters: %s", err)
    }

    if params.Debug {
        log.SetLevel(log.DebugLevel)
        log.Debug("debug logs enabled\n")
    }

    log.Debug(params.String())

    gc := git.NewGit(p.RepoDir)

    return run(params, gc)
}

func run(params Params, gc gitClient) (Result, error) {...}

The main thing here is the gitClient interface. This you can mock in the unit tests and write unit tests for the main logic in run() for all scenarios without setting up real git repositories. And these tests will give me a quick and clear overview, of what semver is doing and that everything is working.

The only contexts, where I see a use for the simulated git repositories are:

  1. Having a single end-2-end test for the whole action (Following the testing pyramid, these hard-to-setup end-2-end tests are on the top of the pyramid and should only be used very sparsely and not to test variations)
  2. Testing pkg/git/git.go. These would be integration tests then.
gandarez commented 3 years ago

@dron22 fixes pushed.