Refactor, improve test blocking logic, support providing a test Trans…

…port (#2)

* Refactor, make tests verify blocking behavior, make Init() not required

* Support providing a test Transport

* Timeout+err if expecting (and fails to complete) a blocking read

doc.go

@@ -4,9 +4,9 @@ Package libhoney is a client library for sending data to https://honeycomb.io
 Summary
 
 libhoney aims to make it as easy as possible to create events and send them on
-into honeycomb.
+into Honeycomb.
 
-See https://docs.honeycomb.io for background on how to use this library
+See https://honeycomb.io/docs for background on this library.
 
 Look in the examples/ directory for a complete example using libhoney.
 */

libhoney.go

@@ -9,6 +9,7 @@ import (
 	"errors"
 	"fmt"
 	"math/rand"
+	"net/http"
 	"reflect"
 	"strings"
 	"sync"
@@ -32,10 +33,36 @@ const (
 	defaultpendingWorkCapacity = 10000
 )
 
+// for validation
 var (
 	ptrKinds = []reflect.Kind{reflect.Ptr, reflect.Slice, reflect.Map}
 )
 
+// globals for singleton-like behavior
+var (
+	blockOnResponses          = false
+	tx               txClient = &txDefaultClient{
+		maxBatchSize:         defaultmaxBatchSize,
+		batchTimeout:         defaultbatchTimeout,
+		maxConcurrentBatches: defaultmaxConcurrentBatches,
+		pendingWorkCapacity:  defaultpendingWorkCapacity,
+	}
+	sd, _          = statsd.New(statsd.Mute(true)) // init working default, to be overridden
+	responses      = make(chan Response, 2*defaultpendingWorkCapacity)
+	defaultBuilder = &Builder{
+		APIHost:    defaultAPIHost,
+		SampleRate: defaultSampleRate,
+		dynFields:  make([]dynamicField, 0, 0),
+		fieldHolder: fieldHolder{
+			data: make(map[string]interface{}),
+		},
+	}
+)
+
+func init() {
+	tx.Start()
+}
+
 // UserAgentAddition is a variable set at compile time via -ldflags to allow you
 // to augment the "User-Agent" header that libhoney sends along with each event.
 // The default User-Agent is "libhoney-go/1.1.1". If you set this variable, its
@@ -88,6 +115,10 @@ type Config struct {
 	MaxConcurrentBatches uint          // how many batches can be inflight simultaneously
 	PendingWorkCapacity  uint          // how many events to allow to pile up
 
+	// Transport can be provided to the http.Client attempting to talk to
+	// Honeycomb servers. Intended for use in tests in order to assert on
+	// expected behavior.
+	Transport http.RoundTripper
 }
 
 type Event struct {
@@ -134,40 +165,21 @@ type fieldHolder struct {
 	lock sync.Mutex
 }
 
-// globals for singleton-like behavior
-var (
-	tx               txClient
-	responses        chan Response
-	blockOnResponses bool
-	sd               *statsd.Client
-	globalState      *Builder
-)
-
 type dynamicField struct {
 	name string
 	fn   func() interface{}
 }
 
-// initialize a default config to protect ourselves against using unitialized
-// values if someone forgets to run Init(). It's fine if things don't work
-// without running Init; it's not fine if they panic.
-func init() {
-	// initialize global statsd client as mute to provide a working default
-	sd, _ = statsd.New(statsd.Mute(true))
-	globalState = &Builder{
-		SampleRate: 1,
-		dynFields:  make([]dynamicField, 0, 0),
-	}
-	globalState.data = make(map[string]interface{})
-}
-
-// Init must be called once on app initialization. All fields in the Config
-// struct are optional. If WriteKey and DataSet are absent, they must be
-// specified later, either on a builder or an event. WriteKey, Dataset,
-// SampleRate, and APIHost can all be overridden on a per-builder or per-event
-// basis.
+// To configure default behavior, Init should be called on app initialization
+// and passed a Config struct. Use of package-level functions (e.g. SendNow())
+// require that WriteKey and Dataset are defined.
+//
+// Otherwise, if WriteKey and DataSet are absent or a Config is not provided,
+// they may be specified later, either on a Builder or an Event. WriteKey,
+// Dataset, SampleRate, and APIHost can all be overridden on a per-Builder or
+// per-Event basis.
 //
-// Make sure to call Close() to flush transmisison buffers.
+// Make sure to call Close() to flush buffers.
 func Init(config Config) error {
 	// Default sample rate should be 1. 0 is invalid.
 	if config.SampleRate == 0 {
@@ -189,32 +201,37 @@ func Init(config Config) error {
 		config.PendingWorkCapacity = defaultpendingWorkCapacity
 	}
 
-	sd, _ = statsd.New(statsd.Prefix("libhoney"))
+	blockOnResponses = config.BlockOnResponse
 
-	responses = make(chan Response, config.PendingWorkCapacity*2)
-
-	// spin up the global transmission
+	// reset the global transmission
+	tx.Stop()
 	tx = &txDefaultClient{
 		maxBatchSize:         config.MaxBatchSize,
 		batchTimeout:         config.SendFrequency,
 		maxConcurrentBatches: config.MaxConcurrentBatches,
 		pendingWorkCapacity:  config.PendingWorkCapacity,
 		blockOnSend:          config.BlockOnSend,
 		blockOnResponses:     config.BlockOnResponse,
+		transport:            config.Transport,
 	}
 
 	if err := tx.Start(); err != nil {
 		return err
 	}
 
-	globalState = &Builder{
+	sd, _ = statsd.New(statsd.Prefix("libhoney"))
+	responses = make(chan Response, config.PendingWorkCapacity*2)
+
+	defaultBuilder = &Builder{
 		WriteKey:   config.WriteKey,
 		Dataset:    config.Dataset,
 		SampleRate: config.SampleRate,
 		APIHost:    config.APIHost,
 		dynFields:  make([]dynamicField, 0, 0),
+		fieldHolder: fieldHolder{
+			data: make(map[string]interface{}),
+		},
 	}
-	globalState.data = make(map[string]interface{})
 
 	return nil
 }
@@ -249,26 +266,26 @@ func Responses() chan Response {
 // created and added as a field (with name as the key) to the newly created
 // event.
 func AddDynamicField(name string, fn func() interface{}) error {
-	return globalState.AddDynamicField(name, fn)
+	return defaultBuilder.AddDynamicField(name, fn)
 }
 
 // AddField adds a Field to the global scope. This metric will be inherited by
 // all builders and events.
 func AddField(name string, val interface{}) {
-	globalState.AddField(name, val)
+	defaultBuilder.AddField(name, val)
 }
 
 // Add adds its data to the global scope. It adds all fields in a struct or all
 // keys in a map as individual Fields. These metrics will be inherited by all
 // builders and events.
 func Add(data interface{}) error {
-	return globalState.Add(data)
+	return defaultBuilder.Add(data)
 }
 
 // Creates a new event prepopulated with any Fields present in the global
 // scope.
 func NewEvent() *Event {
-	return globalState.NewEvent()
+	return defaultBuilder.NewEvent()
 }
 
 // AddField adds an individual metric to the event or builder on which it is
@@ -475,7 +492,7 @@ func isFirstLower(s string) bool {
 // NewBuilder creates a new event builder. The builder inherits any
 // Dynamic or Static Fields present in the global scope.
 func NewBuilder() *Builder {
-	return globalState.Clone()
+	return defaultBuilder.Clone()
 }
 
 // AddDynamicField adds a dynamic field to the builder. Any events

libhoney_test.go

@@ -1,8 +1,11 @@
 package libhoney
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
+	"io/ioutil"
+	"net/http"
 	"runtime"
 	"testing"
 	"time"
@@ -14,7 +17,7 @@ import (
 // tests interact with the same variables in a way that is not like how it
 // would be used. This function resets things to a blank state.
 func resetPackageVars() {
-	globalState = &Builder{}
+	defaultBuilder = &Builder{}
 	sd, _ = statsd.New(statsd.Mute(true))
 }
 
@@ -411,7 +414,7 @@ func TestBuilderDynFields(t *testing.T) {
 	AddDynamicField("ints", myIntFn)
 	b := NewBuilder()
 	b.AddDynamicField("strs", myStrFn)
-	testEquals(t, len(globalState.dynFields), 1)
+	testEquals(t, len(defaultBuilder.dynFields), 1)
 	testEquals(t, len(b.dynFields), 2)
 
 	ev1 := NewEvent()
@@ -503,6 +506,30 @@ func TestSendTime(t *testing.T) {
 	}
 }
 
+type testTransport struct {
+	invoked bool
+}
+
+func (tr *testTransport) RoundTrip(r *http.Request) (*http.Response, error) {
+	tr.invoked = true
+	return &http.Response{Body: ioutil.NopCloser(bytes.NewReader(nil))}, nil
+}
+
+func TestSendTestTransport(t *testing.T) {
+	tr := &testTransport{}
+	Init(Config{
+		WriteKey:  "foo",
+		Dataset:   "bar",
+		Transport: tr,
+	})
+
+	err := SendNow(map[string]interface{}{"foo": 3})
+	tx.Stop()  // flush unsent events
+	tx.Start() // reopen tx.muster channel
+	testOK(t, err)
+	testEquals(t, tr.invoked, true)
+}
+
 func TestChannelMembers(t *testing.T) {
 	resetPackageVars()
 	Init(Config{})

test_helpers.go

@@ -2,6 +2,7 @@ package libhoney
 
 import (
 	"fmt"
+	"net/http"
 	"path/filepath"
 	"reflect"
 	"runtime"
@@ -35,32 +36,6 @@ func testNotEquals(t testing.TB, actual, expected interface{}, msg ...string) {
 	}
 }
 
-func testResponsesChannelEmpty(t testing.TB, chnl chan Response, msg ...string) {
-	expected := "no response"
-	var rsp string
-	select {
-	case _ = <-chnl:
-		rsp = "got response"
-	default:
-		rsp = "no response"
-	}
-	testEquals(t, rsp, expected)
-}
-
-func testChannelHasResponse(t testing.TB, chnl chan Response, msg ...string) {
-	expected := "got response"
-	var r Response
-	var rsp string
-	select {
-	case r = <-chnl:
-		rsp = "got response"
-	default:
-		rsp = "no response"
-	}
-	t.Log(r)
-	testEquals(t, rsp, expected)
-}
-
 func testCommonErr(t testing.TB, actual, expected interface{}, msg []string) {
 	message := strings.Join(msg, ", ")
 	_, file, line, _ := runtime.Caller(2)
@@ -77,6 +52,30 @@ func testCommonErr(t testing.TB, actual, expected interface{}, msg []string) {
 	)
 }
 
+func testGetResponse(t testing.TB, ch chan Response) Response {
+	_, file, line, _ := runtime.Caller(2)
+	var resp Response
+	select {
+	case resp = <-ch:
+	case <-time.After(5 * time.Millisecond): // block on read but prevent deadlocking tests
+		t.Errorf("%s:%d: expected response on channel and timed out waiting for it!", filepath.Base(file), line)
+	}
+	return resp
+}
+
+func testIsPlaceholderResponse(t testing.TB, actual Response, msg ...string) {
+	if actual.StatusCode != http.StatusTeapot {
+		message := strings.Join(msg, ", ")
+		_, file, line, _ := runtime.Caller(1)
+		t.Errorf(
+			"%s:%d placeholder expected -- %s",
+			filepath.Base(file),
+			line,
+			message,
+		)
+	}
+}
+
 func testDeref(v interface{}) interface{} {
 	switch t := v.(type) {
 	case *string:
@@ -94,21 +93,13 @@ func testDeref(v interface{}) interface{} {
 
 // for easy time manipulation during tests
 type fakeNower struct {
-	now  time.Time
 	iter int
 }
 
-// Now() returns the nth element of nows
+// Now() supports changing/increasing the returned Now() based on the number of
+// times it's called in succession
 func (f *fakeNower) Now() time.Time {
-	now := f.now.Add(time.Second * 10 * time.Duration(f.iter))
+	now := time.Unix(1277132645, 0).Add(time.Second * 10 * time.Duration(f.iter))
 	f.iter += 1
 	return now
 }
-func (f *fakeNower) set(t time.Time) {
-	f.now = t
-}
-func (f *fakeNower) init() {
-	f.iter = 0
-	t0, _ := time.Parse(time.RFC3339, "2010-06-21T15:04:05Z")
-	f.now = t0
-}