search

uber-go / fx

A dependency injection based application framework for Go.
https://uber-go.github.io/fx/
MIT License
5.83k stars 290 forks source link

Faster make test #307

Closed bufdev closed 7 years ago

bufdev commented 7 years ago

I know everyone likes their code coverage, richgo, etc heh, but consider the following:

$ time go test $(go list ./... | grep -v vendor)
...
real    0m5.135s
user    0m21.234s
sys 0m4.598s

$ time make test
..
real    0m58.386s
user    5m24.928s
sys 1m10.965s

I know that the make target is trying to do some smart things, but when I'm developing, most of the time I just want to know if the code builds and the tests pass. I had to turn off neomake (the vim plugin I have setup to run make on every save of a go file) because my computer slowed way down and my computer's fan wouldn't stop running. I actually almost max out every processor.

Can we do something about this for day-to-day development? I also wish I could just specify specific packages with whatever is eventually done.

ascandella commented 7 years ago

Agreed, this is the reason I usually run go test directly instead of our internal build make rules. I think the primary problems are:

  1. Installing glide deps too often
  2. Installing thriftrw and friends every time
bufdev commented 7 years ago

The actual test target alone takes a long time, we should put a "time" call in front of it On Tue, Feb 21, 2017 at 7:38 PM Aiden Scandella notifications@github.com wrote:

Agreed, this is the reason I usually run go test directly instead of our internal build make rules. I think the primary problems are:

  1. Installing glide deps too often
  2. Installing thriftrw and friends every time

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/uber-go/fx/issues/307#issuecomment-281435927, or mute the thread https://github.com/notifications/unsubscribe-auth/AECGvJqVIdD10IgTRr2mMG61sb3strxSks5rey8dgaJpZM4MHiHY .

glibsm commented 7 years ago

Am I am outlier? I know that a clean test takes forever, but this is what I get for just make test right now:

$ time make test
Building examples

Installing thriftrw...
Installing thriftrw-plugin-yarpc...
Installing thriftrw-plugin-thriftsync...
?       go.uber.org/fx/examples/simple  [no test files]
?       go.uber.org/fx/examples/dig     [no test files]
make test  4.82s user 1.45s system 139% cpu 4.491 total
bufdev commented 7 years ago

Ya that's if nothing changed, that's makefile magic, which uh, doesn't always happen well heh. One sec, let me see if I can find a good comparison locally.

bufdev commented 7 years ago 
$ git diff
diff --git a/Makefile b/Makefile
index 25f7375..c07657a 100644
--- a/Makefile
+++ b/Makefile
@@ -53,7 +53,7 @@ $(COV_REPORT): $(PKG_FILES) $(ALL_SRC)
        $(ECHO_V)rm -f $(COV_REPORT)

        @$(call label,Running tests)
-       $(ECHO_V)RICHGO_FORCE_COLOR=1 $(OVERALLS) \
+       $(ECHO_V)RICHGO_FORCE_COLOR=1 time $(OVERALLS) \
                -project=$(PROJECT_ROOT) \
                -go-binary=richgo \
                -ignore "$(OVERALLS_IGNORE)" \
$ make clean test
...
51.11 real       304.89 user        65.37 sys
ascandella commented 7 years ago

Why run clean? On Tue, Feb 21, 2017 at 11:48 AM Peter Edge notifications@github.com wrote:

$ git diff diff --git a/Makefile b/Makefile index 25f7375..c07657a 100644 --- a/Makefile +++ b/Makefile @@ -53,7 +53,7 @@ $(COV_REPORT): $(PKG_FILES) $(ALL_SRC) $(ECHO_V)rm -f $(COV_REPORT)

    @$(call label,Running tests)
  • $(ECHO_V)RICHGO_FORCE_COLOR=1 $(OVERALLS) \
  • $(ECHO_V)RICHGO_FORCE_COLOR=1 time $(OVERALLS) \ -project=$(PROJECT_ROOT) \ -go-binary=richgo \ -ignore "$(OVERALLS_IGNORE)" \

$ make clean test ... 51.11 real 304.89 user 65.37 sys

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/uber-go/fx/issues/307#issuecomment-281459052, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF7P_lROhIidpYfDWCks4TgVyq1w3O2ks5rez-FgaJpZM4MHiHY .

bufdev commented 7 years ago

Correct me if I'm wrong (it's been a long day), but if you have package a with dependent package b, or more to the point a -> b -> c -> d -> e, and you make edits to a, don't you have to re-run all the tests for a, b, c, d, e? Point being, if you're working on say, the service package heh, aren't you effectively going to need to re-do all the tests anyways?

Whatever is happening in overalls/richgo, it's taking approximately 10x longer than a simple go test call, which is a big multiplier.

bufdev commented 7 years ago

Just as an example of running for one package, if you change PROJECT_ROOT to go.uber.org/fx/service, and time the $(OVERALLS) command above, it takes ~7.34s. If you do go test ./service, it takes ~1.23s.

bufdev commented 7 years ago

I'm getting some really weird results by the way, I've run these multiple times (using the above diff where you put time in front of $(OVERALLS)):

go test ./service: ~1.23s go test -race ./service: ~3.68s make clean test RACE=-race: ~3.69s make clean test: ~7.34s.

???

bufdev commented 7 years ago

Now I'm getting even weirder results, I think I'm missing something.

OK, to summarize my thoughts: I feel like make test is cool and whatnot, but really it should be more something like make cov, and I wish that make test was dependent on "core" linters (golint, go vet, errcheck). Basically:

PKGS ?= $(shell glide nv)

test: golint vet errcheck
  go test $(TEST_FLAGS) $(RACE) $(TEST_TIMEOUT) $(TEST_VERBOSITY_FLAG) $(PKGS)
glibsm commented 7 years ago

The most productive thing in this area would probably be to split up make test from make travis.

In travis, we can run with race, coveralls, and building examples. Make test can be a lot more streamlined that just reports test results with coverage.

One days I might get to it :)

But uh... our Makefile is uhh... open sourced wink wink cough cough