Fix #11 by separating the responsibility to store and run hooks (#27)

* Fix #11 by separating the responsibility to store and run hooks

* Cleanup

Remove unused fields from hooks.DummyRunner
Fix type name in DummyRunner description

Fix some warnings during testing:
```
warn: Parser warning in file './apiary.apib': (warning code 10) message-body asset is expected to be a pre-formatted code block, separate it by a newline and indent every of its line by 8 spaces or 2 tabs on lines 3-4
warn: Parser warning in file './apiary.apib': (warning code 10) message-body asset is expected to be a pre-formatted code block, separate it by a newline and indent every of its line by 8 spaces or 2 tabs on lines 5-6
```

* Fix RPC service name

* Fix cucumber tests

* Get HooksRunner struct name via reflection to avoid a hardcoded string

* Remove vendored goodman packages

.travis.yml

@@ -9,6 +9,7 @@ go:
   - 1.5
   - 1.6
   - 1.7
+  - 1.8
 env:
   - GO15VENDOREXPERIMENT=1
 

README.md

@@ -44,7 +44,7 @@ import (
 
 func main() {
       h := hooks.NewHooks()
-      server := hooks.NewServer(h)
+      server := hooks.NewServer(hooks.NewHooksRunner(h))
       h.Before("/message > GET", func(t *trans.Transaction) {
           fmt.Println("before modification")
       })

cmd/goodman/main.go

@@ -7,10 +7,12 @@ import (
 	"os"
 	"os/exec"
 	"os/signal"
+	"reflect"
 	"syscall"
 	"time"
 
 	"github.com/snikch/goodman"
+	"github.com/snikch/goodman/hooks"
 )
 
 var (
@@ -50,8 +52,9 @@ func main() {
 					fmt.Println("Hooks client failed with " + err.Error())
 				}
 			}()
+			rpcService := reflect.TypeOf((*hooks.HooksRunner)(nil)).Elem().Name()
 			for retries := 5; retries > 0; retries-- {
-				runner, err := goodman.NewRunner("Hooks", hookServerInitalPort)
+				runner, err := goodman.NewRunner(rpcService, hookServerInitalPort)
 				if err == nil {
 					runners = append(runners, runner)
 					break

example/hooks/hooks.go

@@ -16,7 +16,7 @@ var (
 
 func main() {
 	h := hooks.NewHooks()
-	server := hooks.NewServer(h)
+	server := hooks.NewServer(hooks.NewHooksRunner(h))
 
 	db, err := sql.Open("sqlite3", "./foo.db")
 	if err != nil {

example/vendor/vendor.json

@@ -7,18 +7,6 @@
 			"path": "github.com/mattn/go-sqlite3",
 			"revision": "38ee283dabf11c9cbdb968eebd79b1fa7acbabe6",
 			"revisionTime": "2016-05-14T12:23:48Z"
-		},
-		{
-			"checksumSHA1": "Mb+NfGb/UUnxP9KrMFWJjuVULVs=",
-			"path": "github.com/snikch/goodman/hooks",
-			"revision": "7213c8a4b0cefc6ee046f3820256aaa197dcd26c",
-			"revisionTime": "2016-06-03T13:03:46Z"
-		},
-		{
-			"checksumSHA1": "ktzdIvtDXOMIEShtXl09//4j0JM=",
-			"path": "github.com/snikch/goodman/transaction",
-			"revision": "7213c8a4b0cefc6ee046f3820256aaa197dcd26c",
-			"revisionTime": "2016-06-03T13:03:46Z"
 		}
 	],
 	"rootPath": "github.com/snikch/goodman/example"

features/execution_order.feature

@@ -8,7 +8,7 @@ Feature: Execution order
       """
       require 'sinatra'
       get '/message' do
-        "Hello World!\n\n"
+        "Hello World!\n"
       end
       """
 
@@ -17,9 +17,12 @@ Feature: Execution order
       # My Api
       ## GET /message
       + Request (text)
-          This prevents a dredd bug
+
+              This prevents a dredd bug
+
       + Response 200 (text/html;charset=utf-8)
-          Hello World!
+
+              Hello World!
       """
 
   @announce
@@ -34,7 +37,7 @@ Feature: Execution order
 
     func main() {
         h := hooks.NewHooks()
-        server := hooks.NewServer(h)
+        server := hooks.NewServer(hooks.NewHooksRunner(h))
         h.BeforeAll(func(t []*trans.Transaction) {
             if t[0].TestOrder == nil {
                 t[0].TestOrder = []string{"before all modification"}

features/failing_transaction.feature

@@ -8,7 +8,7 @@ Feature: Failing a transaction
       """
       require 'sinatra'
       get '/message' do
-        "Hello World!\n\n"
+        "Hello World!\n"
       end
       """
 
@@ -17,9 +17,12 @@ Feature: Failing a transaction
       # My Api
       ## GET /message
       + Request (text)
-        This prevents dredd bug
+
+              This prevents dredd bug
+
       + Response 200 (text/html;charset=utf-8)
-          Hello World!
+
+              Hello World!
       """
 
   @debug
@@ -34,7 +37,7 @@ Feature: Failing a transaction
 
       func main() {
           h := hooks.NewHooks()
-          server := hooks.NewServer(h)
+          server := hooks.NewServer(hooks.NewHooksRunner(h))
           h.Before("/message > GET", func(t *trans.Transaction) {
               t.Fail = "Yay! Failed!"
           })

features/hook_handlers.feature

@@ -8,7 +8,7 @@ Feature: Hook handlers
       """
       require 'sinatra'
       get '/message' do
-        "Hello World!\n\n"
+        "Hello World!\n"
       end
       """
 
@@ -17,9 +17,12 @@ Feature: Hook handlers
       # My Api
       ## GET /message
       + Request (text)
-          test this
+
+              test this
+
       + Response 200 (text/html;charset=utf-8)
-          Hello World!
+
+              Hello World!
       """
 
   @debug
@@ -36,7 +39,7 @@ Feature: Hook handlers
 
       func main() {
           h := hooks.NewHooks()
-          server := hooks.NewServer(h)
+          server := hooks.NewServer(hooks.NewHooksRunner(h))
           h.BeforeAll(func(t []*trans.Transaction) {
             fmt.Println("before all hook handled")
           })

features/multiple_hookfiles.feature

@@ -8,7 +8,7 @@ Feature: Multiple hook files with a glob
       """
       require 'sinatra'
       get '/message' do
-        "Hello World!\n\n"
+        "Hello World!\n"
       end
       """
 
@@ -17,9 +17,12 @@ Feature: Multiple hook files with a glob
       # My Api
       ## GET /message
       + Request (text)
-          This prevents a dredd bug
+
+              This prevents a dredd bug
+
       + Response 200 (text/html;charset=utf-8)
-          Hello World!
+
+              Hello World!
       """
 
   @debug
@@ -36,7 +39,7 @@ Feature: Multiple hook files with a glob
 
       func main() {
           h := hooks.NewHooks()
-          server := hooks.NewServer(h)
+          server := hooks.NewServer(hooks.NewHooksRunner(h))
           h.Before("/message > GET", func(t *trans.Transaction) {
             fmt.Println("It's me, File1")
           })
@@ -58,7 +61,7 @@ Feature: Multiple hook files with a glob
 
       func main() {
           h := hooks.NewHooks()
-          server := hooks.NewServer(h)
+          server := hooks.NewServer(hooks.NewHooksRunner(h))
           h.Before("/message > GET", func(t *trans.Transaction) {
             fmt.Println("It's me, File2")
           })
@@ -80,7 +83,7 @@ Feature: Multiple hook files with a glob
 
       func main() {
           h := hooks.NewHooks()
-          server := hooks.NewServer(h)
+          server := hooks.NewServer(hooks.NewHooksRunner(h))
           h.Before("/message > GET", func(t *trans.Transaction) {
             fmt.Println("It's me, File3")
           })

hooks/hooks.go

@@ -9,7 +9,7 @@ type (
 	AllCallback func([]*trans.Transaction)
 )
 
-// Hooks is responsible for storing and running lifecycle callbacks.
+// Hooks is responsible for storing lifecycle callbacks.
 type Hooks struct {
 	beforeAll            []AllCallback
 	beforeEach           []Callback
@@ -21,7 +21,7 @@ type Hooks struct {
 	afterAll             []AllCallback
 }
 
-// NewHooks returns a new Hooks instance will all callback fields initialized.
+// NewHooks returns a new Hooks instance with all callback fields initialized.
 func NewHooks() *Hooks {
 	return &Hooks{
 		beforeAll:            []AllCallback{},
@@ -35,71 +35,6 @@ func NewHooks() *Hooks {
 	}
 }
 
-func (h *Hooks) RunBeforeAll(args []*trans.Transaction, reply *[]*trans.Transaction) error {
-	*reply = args
-	for _, cb := range h.beforeAll {
-		cb(args)
-	}
-	return nil
-}
-
-func (h *Hooks) RunBeforeEach(args trans.Transaction, reply *trans.Transaction) error {
-	*reply = args
-	for _, cb := range h.beforeEach {
-		cb(reply)
-	}
-	return nil
-}
-func (h *Hooks) RunBefore(args trans.Transaction, reply *trans.Transaction) error {
-	name := args.Name
-	*reply = args
-	for _, cb := range h.before[name] {
-		cb(reply)
-	}
-	return nil
-}
-
-func (h *Hooks) RunBeforeEachValidation(args trans.Transaction, reply *trans.Transaction) error {
-	*reply = args
-	for _, cb := range h.beforeEachValidation {
-		cb(reply)
-	}
-	return nil
-}
-func (h *Hooks) RunBeforeValidation(args trans.Transaction, reply *trans.Transaction) error {
-	name := args.Name
-	*reply = args
-	for _, cb := range h.beforeValidation[name] {
-		cb(reply)
-	}
-	return nil
-}
-
-func (h *Hooks) RunAfter(args trans.Transaction, reply *trans.Transaction) error {
-	name := args.Name
-	*reply = args
-	for _, cb := range h.after[name] {
-		cb(reply)
-	}
-	return nil
-}
-
-func (h *Hooks) RunAfterEach(args trans.Transaction, reply *trans.Transaction) error {
-	*reply = args
-	for _, cb := range h.afterEach {
-		cb(reply)
-	}
-	return nil
-}
-
-func (h *Hooks) RunAfterAll(args []*trans.Transaction, reply *[]*trans.Transaction) error {
-	*reply = args
-	for _, cb := range h.afterAll {
-		cb(args)
-	}
-	return nil
-}
-
 // BeforeAll adds a callback function to be called before the entire test suite.
 func (h *Hooks) BeforeAll(fn AllCallback) {
 	h.beforeAll = append(h.beforeAll, fn)
@@ -148,3 +83,80 @@ func (h *Hooks) AfterEach(fn Callback) {
 func (h *Hooks) AfterAll(fn AllCallback) {
 	h.afterAll = append(h.afterAll, fn)
 }
+
+// Hooks is responsible for running lifecycle callbacks.
+type HooksRunner struct {
+	hooks *Hooks
+}
+
+// NewHooksRunner returns a new HooksRunner instance with a given hooks structure.
+func NewHooksRunner(h *Hooks) *HooksRunner {
+	return &HooksRunner{
+		hooks: h,
+	}
+}
+
+func (h *HooksRunner) RunBeforeAll(args []*trans.Transaction, reply *[]*trans.Transaction) error {
+	*reply = args
+	for _, cb := range h.hooks.beforeAll {
+		cb(args)
+	}
+	return nil
+}
+
+func (h *HooksRunner) RunBeforeEach(args trans.Transaction, reply *trans.Transaction) error {
+	*reply = args
+	for _, cb := range h.hooks.beforeEach {
+		cb(reply)
+	}
+	return nil
+}
+func (h *HooksRunner) RunBefore(args trans.Transaction, reply *trans.Transaction) error {
+	name := args.Name
+	*reply = args
+	for _, cb := range h.hooks.before[name] {
+		cb(reply)
+	}
+	return nil
+}
+
+func (h *HooksRunner) RunBeforeEachValidation(args trans.Transaction, reply *trans.Transaction) error {
+	*reply = args
+	for _, cb := range h.hooks.beforeEachValidation {
+		cb(reply)
+	}
+	return nil
+}
+func (h *HooksRunner) RunBeforeValidation(args trans.Transaction, reply *trans.Transaction) error {
+	name := args.Name
+	*reply = args
+	for _, cb := range h.hooks.beforeValidation[name] {
+		cb(reply)
+	}
+	return nil
+}
+
+func (h *HooksRunner) RunAfter(args trans.Transaction, reply *trans.Transaction) error {
+	name := args.Name
+	*reply = args
+	for _, cb := range h.hooks.after[name] {
+		cb(reply)
+	}
+	return nil
+}
+
+func (h *HooksRunner) RunAfterEach(args trans.Transaction, reply *trans.Transaction) error {
+	*reply = args
+	for _, cb := range h.hooks.afterEach {
+		cb(reply)
+	}
+	return nil
+}
+
+func (h *HooksRunner) RunAfterAll(args []*trans.Transaction, reply *[]*trans.Transaction) error {
+	*reply = args
+	for _, cb := range h.hooks.afterAll {
+		cb(args)
+	}
+	return nil
+}