diff --git a/Makefile b/Makefile index 25773ed36..dd4711aac 100644 --- a/Makefile +++ b/Makefile @@ -4,14 +4,16 @@ GOTOOLS = \ github.com/Masterminds/glide \ github.com/alecthomas/gometalinter +PACKAGES=$(shell go list ./... | grep -v '/vendor/') REPO:=github.com/tendermint/tmlibs all: test -NOVENDOR = go list github.com/tendermint/tmlibs/... | grep -v /vendor/ - test: - go test `glide novendor` + @echo "--> Running linter" + @make metalinter_test + @echo "--> Running go test" + @go test $(PACKAGES) get_vendor_deps: ensure_tools @rm -rf vendor/ @@ -20,16 +22,14 @@ get_vendor_deps: ensure_tools ensure_tools: go get $(GOTOOLS) - -metalinter: ensure_tools @gometalinter --install + +metalinter: gometalinter --vendor --deadline=600s --enable-all --disable=lll ./... -metalinter_test: ensure_tools - @gometalinter --install +metalinter_test: gometalinter --vendor --deadline=600s --disable-all \ --enable=deadcode \ - --enable=gas \ --enable=goconst \ --enable=gosimple \ --enable=ineffassign \ @@ -46,6 +46,7 @@ metalinter_test: ensure_tools --enable=vet \ ./... + #--enable=gas \ #--enable=aligncheck \ #--enable=dupl \ #--enable=errcheck \ diff --git a/circle.yml b/circle.yml index 3dba976be..104cfa6f3 100644 --- a/circle.yml +++ b/circle.yml @@ -15,7 +15,7 @@ dependencies: test: override: - - cd $PROJECT_PATH && make get_vendor_deps && make metalinter_test && bash ./test.sh + - cd $PROJECT_PATH && make get_vendor_deps && bash ./test.sh post: - cd "$PROJECT_PATH" && bash <(curl -s https://codecov.io/bash) -f coverage.txt - cd "$PROJECT_PATH" && mv coverage.txt "${CIRCLE_ARTIFACTS}" diff --git a/common/service.go b/common/service.go index 8d4de30a8..d70d16a80 100644 --- a/common/service.go +++ b/common/service.go @@ -1,23 +1,41 @@ package common import ( + "errors" + "fmt" "sync/atomic" "github.com/tendermint/tmlibs/log" ) +var ( + ErrAlreadyStarted = errors.New("already started") + ErrAlreadyStopped = errors.New("already stopped") +) + +// Service defines a service that can be started, stopped, and reset. type Service interface { - Start() (bool, error) + // Start the service. + // If it's already started or stopped, will return an error. + // If OnStart() returns an error, it's returned by Start() + Start() error OnStart() error - Stop() bool + // Stop the service. + // If it's already stopped, will return an error. + // OnStop must never error. + Stop() error OnStop() - Reset() (bool, error) + // Reset the service. + // Panics by default - must be overwritten to enable reset. + Reset() error OnReset() error + // Return true if the service is running IsRunning() bool + // String representation of the service String() string SetLogger(log.Logger) @@ -94,11 +112,11 @@ func (bs *BaseService) SetLogger(l log.Logger) { } // Implements Servce -func (bs *BaseService) Start() (bool, error) { +func (bs *BaseService) Start() error { if atomic.CompareAndSwapUint32(&bs.started, 0, 1) { if atomic.LoadUint32(&bs.stopped) == 1 { bs.Logger.Error(Fmt("Not starting %v -- already stopped", bs.name), "impl", bs.impl) - return false, nil + return ErrAlreadyStopped } else { bs.Logger.Info(Fmt("Starting %v", bs.name), "impl", bs.impl) } @@ -106,12 +124,12 @@ func (bs *BaseService) Start() (bool, error) { if err != nil { // revert flag atomic.StoreUint32(&bs.started, 0) - return false, err + return err } - return true, err + return nil } else { bs.Logger.Debug(Fmt("Not starting %v -- already started", bs.name), "impl", bs.impl) - return false, nil + return ErrAlreadyStarted } } @@ -121,15 +139,15 @@ func (bs *BaseService) Start() (bool, error) { func (bs *BaseService) OnStart() error { return nil } // Implements Service -func (bs *BaseService) Stop() bool { +func (bs *BaseService) Stop() error { if atomic.CompareAndSwapUint32(&bs.stopped, 0, 1) { bs.Logger.Info(Fmt("Stopping %v", bs.name), "impl", bs.impl) bs.impl.OnStop() close(bs.Quit) - return true + return nil } else { bs.Logger.Debug(Fmt("Stopping %v (ignoring: already stopped)", bs.name), "impl", bs.impl) - return false + return ErrAlreadyStopped } } @@ -139,17 +157,17 @@ func (bs *BaseService) Stop() bool { func (bs *BaseService) OnStop() {} // Implements Service -func (bs *BaseService) Reset() (bool, error) { +func (bs *BaseService) Reset() error { if !atomic.CompareAndSwapUint32(&bs.stopped, 1, 0) { bs.Logger.Debug(Fmt("Can't reset %v. Not stopped", bs.name), "impl", bs.impl) - return false, nil + return fmt.Errorf("can't reset running %s", bs.name) } // whether or not we've started, we can reset atomic.CompareAndSwapUint32(&bs.started, 1, 0) bs.Quit = make(chan struct{}) - return true, bs.impl.OnReset() + return bs.impl.OnReset() } // Implements Service diff --git a/common/service_test.go b/common/service_test.go index 6e24dad6a..ef360a648 100644 --- a/common/service_test.go +++ b/common/service_test.go @@ -2,23 +2,53 @@ package common import ( "testing" + "time" + + "github.com/stretchr/testify/require" ) -func TestBaseServiceWait(t *testing.T) { +type testService struct { + BaseService +} - type TestService struct { - BaseService - } - ts := &TestService{} +func (testService) OnReset() error { + return nil +} + +func TestBaseServiceWait(t *testing.T) { + ts := &testService{} ts.BaseService = *NewBaseService(nil, "TestService", ts) ts.Start() + waitFinished := make(chan struct{}) go func() { - ts.Stop() + ts.Wait() + waitFinished <- struct{}{} }() - for i := 0; i < 10; i++ { - ts.Wait() + go ts.Stop() + + select { + case <-waitFinished: + // all good + case <-time.After(100 * time.Millisecond): + t.Fatal("expected Wait() to finish within 100 ms.") } +} + +func TestBaseServiceReset(t *testing.T) { + ts := &testService{} + ts.BaseService = *NewBaseService(nil, "TestService", ts) + ts.Start() + + err := ts.Reset() + require.Error(t, err, "expected cant reset service error") + + ts.Stop() + + err = ts.Reset() + require.NoError(t, err) + err = ts.Start() + require.NoError(t, err) } diff --git a/events/events_test.go b/events/events_test.go index dee50e5bd..87db2a304 100644 --- a/events/events_test.go +++ b/events/events_test.go @@ -13,8 +13,8 @@ import ( // listener to an event, and sends a string "data". func TestAddListenerForEventFireOnce(t *testing.T) { evsw := NewEventSwitch() - started, err := evsw.Start() - if !started || err != nil { + err := evsw.Start() + if err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } messages := make(chan EventData) @@ -33,8 +33,8 @@ func TestAddListenerForEventFireOnce(t *testing.T) { // listener to an event, and sends a thousand integers. func TestAddListenerForEventFireMany(t *testing.T) { evsw := NewEventSwitch() - started, err := evsw.Start() - if !started || err != nil { + err := evsw.Start() + if err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum := make(chan uint64) @@ -62,8 +62,8 @@ func TestAddListenerForEventFireMany(t *testing.T) { // of the three events. func TestAddListenerForDifferentEvents(t *testing.T) { evsw := NewEventSwitch() - started, err := evsw.Start() - if !started || err != nil { + err := evsw.Start() + if err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum := make(chan uint64) @@ -107,8 +107,8 @@ func TestAddListenerForDifferentEvents(t *testing.T) { // for each of the three events. func TestAddDifferentListenerForDifferentEvents(t *testing.T) { evsw := NewEventSwitch() - started, err := evsw.Start() - if !started || err != nil { + err := evsw.Start() + if err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum1 := make(chan uint64) @@ -167,8 +167,8 @@ func TestAddDifferentListenerForDifferentEvents(t *testing.T) { // the listener and fires a thousand integers for the second event. func TestAddAndRemoveListener(t *testing.T) { evsw := NewEventSwitch() - started, err := evsw.Start() - if !started || err != nil { + err := evsw.Start() + if err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum1 := make(chan uint64) @@ -212,8 +212,8 @@ func TestAddAndRemoveListener(t *testing.T) { // TestRemoveListener does basic tests on adding and removing func TestRemoveListener(t *testing.T) { evsw := NewEventSwitch() - started, err := evsw.Start() - if !started || err != nil { + err := evsw.Start() + if err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } count := 10 @@ -265,8 +265,8 @@ func TestRemoveListener(t *testing.T) { // `go test -race`, to examine for possible race conditions. func TestRemoveListenersAsync(t *testing.T) { evsw := NewEventSwitch() - started, err := evsw.Start() - if !started || err != nil { + err := evsw.Start() + if err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum1 := make(chan uint64) diff --git a/test.sh b/test.sh index 012162b07..02bdaae86 100755 --- a/test.sh +++ b/test.sh @@ -1,8 +1,11 @@ #!/usr/bin/env bash - set -e -echo "" > coverage.txt +# run the linter +make metalinter_test + +# run the unit tests with coverage +echo "" > coverage.txt for d in $(go list ./... | grep -v vendor); do go test -race -coverprofile=profile.out -covermode=atomic "$d" if [ -f profile.out ]; then