Add Magefile for easy testing #37
Conversation
| fmt run gofmt linter | ||
| lint run golint linter https://github.com/golang/lint | ||
| testCoverHTML generates test coverage report | ||
| testRace run tests with race detector |
There was a problem hiding this comment.
let's name testRace as test
There was a problem hiding this comment.
original had both: https://github.com/gohugoio/hugo/blob/master/magefile.go#L188
If we keep all of this, why not keeping both ?
There was a problem hiding this comment.
would we ever need to run without race?
There was a problem hiding this comment.
let's keep both but default target should call testRace
- Add vet to default target - Add alias for test with race
Magefile.go
Outdated
| mg.Deps(Fmt, Vet) | ||
| // TODO: Enable once errors are fixed | ||
| mg.Deps(TestRace) | ||
| mg.Deps(Build) |
There was a problem hiding this comment.
Why do we have Build in Check? it is counter intuitive 😄 . Rename Check to Build?
There was a problem hiding this comment.
This magefile comes from Hugo (the CMS: https://github.com/gohugoio/hugo/blob/master/magefile.go) which is a great one but still is not totally aligned with our usecase. Here we're working on a library which is not buildable, nor installable.
I believe we have to refine what we expect from this tool.
I would probably expect it to make sure the library code is well formatted / linted, then to run the tests, and at the end, maybe run some example.
I would also be happy if it would start a dev env (running druid, and builder) mostly like does the magefile on its grafana counterpart: https://github.com/grafadruid/druid-grafana/blob/master/Magefile.go
That would also allow to run tests that does interacts with Druid.
WDYT?
Magefile.go
Outdated
| if err := sh.Run("go", "mod", "download"); err != nil { | ||
| return err | ||
| } | ||
| return sh.Run("go", "install", "./...") |
There was a problem hiding this comment.
wondering what it'll build, install and run in our case ? examples ? what one(s) ?
this is a library, only what use it can be installed. We could eventually want to build "examples" but then we would have to make those examples a bit more "unified"
There was a problem hiding this comment.
@jbguerraz I agree, I'm working on issue #36 where I will introduce a Dockerfile and run the examples as a part of integration tests. I can exclude install from this PR. This PR is only aimed at resolving #35.
There was a problem hiding this comment.
let's keep it simple as @jbguerraz suggested and keep the scope of the PR w.r.t what we have now. i.e perhaps just call test and vet, i believe i saw some errors when i ran lint which we can tackle in a different PR.
There was a problem hiding this comment.
Let's keep both testRace and test as @jbguerraz mentioned but default should call testRace https://github.com/gohugoio/hugo/blob/master/magefile.go#L163
|
Let's move ahead with it :) thank you @cosmic-chichu ! |
Resolves #35