Repository: yarpc/yab (go)

Issue:
yab panic when using raw encoding and `--disable-thrift-envelope`
```
yab -e raw -p http://127.0.0.1:31337/ anything --procedure here --disable-thrift-envelope
```
```
panic: interface conversion: encoding.rawSerializer is not main.noEnveloper: missing method WithoutEnvelopes

goroutine 1 [running]:
main.withTransportSerializer(0x2, 0x1130740, 0xc42057b9b0, 0xc42057b8ac, 0x3, 0x0, 0x0, 0x7ffcb32a48fb, 0x4, 0xc4200f8020, ...)
	src/github.com/yarpc/yab/main.go:370 +0x5c
main.runWithOptions(0xc42057b8ac, 0x3, 0x0, 0x0, 0x7ffcb32a48fb, 0x4, 0xc4200f8020, 0x0, 0x0, 0x0, ...)
	src/github.com/yarpc/yab/main.go:320 +0x808
main.parseAndRun(0x1130a40, 0xc42000e018)
	src/github.com/yarpc/yab/main.go:206 +0x4ba
main.main()
	src/github.com/yarpc/yab/main.go:83 +0x46
```

Pull request description:
encoding: Determine whether to envelope before creating serializer
After refactoring transport/encoding detection (#282), we can now
determine whether to envelope before creating the serializer, rather
than creating the serializer, then upcasting to disable envelopes.

Fixes #214 (no longer have interface upcasting issues)

Patch:
diff --git a/encoding/encoding.go b/encoding/encoding.go
index 05b4479..b6b1c75 100644
--- a/encoding/encoding.go
+++ b/encoding/encoding.go
@@ -26,6 +26,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/yarpc/yab/thrift"
 	"github.com/yarpc/yab/transport"
 	"github.com/yarpc/yab/unmarshal"
 )
@@ -90,7 +91,8 @@ func (e Encoding) GetHealth(serviceName string) (Serializer, error) {
 	switch e {
 	case Thrift:
 		method, spec := getHealthSpec()
-		return thriftSerializer{method, spec, defaultOpts}, nil
+		opts := thrift.Options{} // Meta::health is TChannel-specific, which doesn't use envelopes.
+		return thriftSerializer{method, spec, opts}, nil
 	case Protobuf:
 		return protoHealthSerializer{serviceName: serviceName}, nil
 	default:
diff --git a/encoding/thrift_request.go b/encoding/thrift_request.go
index b2c57a6..7002906 100644
--- a/encoding/thrift_request.go
+++ b/encoding/thrift_request.go
@@ -36,13 +36,9 @@ import (
 
 const _multiplexedSeparator = ":"
 
-var (
-	// ErrSpecifyThriftFile is returned if no Thrift file is specified
-	// for a Thrift request.
-	ErrSpecifyThriftFile = errors.New("specify a Thrift file using --thrift")
-
-	defaultOpts = thrift.Options{UseEnvelopes: true}
-)
+// ErrSpecifyThriftFile is returned if no Thrift file is specified
+// for a Thrift request.
+var ErrSpecifyThriftFile = errors.New("specify a Thrift file using --thrift")
 
 type thriftSerializer struct {
 	methodName string
@@ -50,21 +46,30 @@ type thriftSerializer struct {
 	opts       thrift.Options
 }
 
+// ThriftParams contains the parameters for the NewThrift function.
+// We use a struct as there are multiple consecutive arguments of the same type.
+type ThriftParams struct {
+	File        string
+	Method      string
+	Multiplexed bool
+	Envelope    bool
+}
+
 // NewThrift returns a Thrift serializer.
-func NewThrift(thriftFile, methodName string, multiplexed bool) (Serializer, error) {
-	if thriftFile == "" {
+func NewThrift(p ThriftParams) (Serializer, error) {
+	if p.File == "" {
 		return nil, ErrSpecifyThriftFile
 	}
-	if isFileMissing(thriftFile) {
-		return nil, fmt.Errorf("cannot find Thrift file: %q", thriftFile)
+	if isFileMissing(p.File) {
+		return nil, fmt.Errorf("cannot find Thrift file: %q", p.File)
 	}
 
-	parsed, err := thrift.Parse(thriftFile)
+	parsed, err := thrift.Parse(p.File)
 	if err != nil {
 		return nil, fmt.Errorf("could not parse Thrift file: %v", err)
 	}
 
-	thriftSvc, thriftMethod, err := thrift.SplitMethod(methodName)
+	thriftSvc, thriftMethod, err := thrift.SplitMethod(p.Method)
 	if err != nil {
 		return nil, err
 	}
@@ -79,12 +84,14 @@ func NewThrift(thriftFile, methodName string, multiplexed bool) (Serializer, err
 		return nil, err
 	}
 
-	opts := defaultOpts
-	if multiplexed {
+	opts := thrift.Options{
+		UseEnvelopes: p.Envelope,
+	}
+	if p.Multiplexed {
 		opts.EnvelopeMethodPrefix = thriftSvc + _multiplexedSeparator
 	}
 
-	return thriftSerializer{methodName, spec, opts}, nil
+	return thriftSerializer{p.Method, spec, opts}, nil
 }
 
 func (e thriftSerializer) Encoding() Encoding {
@@ -130,12 +137,6 @@ func (e thriftSerializer) CheckSuccess(res *transport.Response) error {
 	return thrift.CheckSuccess(e.spec, res.Body, e.opts)
 }
 
-func (e thriftSerializer) WithoutEnvelopes() Serializer {
-	// We're modifying a copy of e.
-	e.opts.UseEnvelopes = false
-	return e
-}
-
 func findMethod(service *compile.ServiceSpec, methodName string) (*compile.FunctionSpec, error) {
 	// Try to find the function in the service or any of the inherited services.
 	for cur := service; cur != nil; cur = cur.Parent {
diff --git a/etc/bash_completion.d/yab b/etc/bash_completion.d/yab
new file mode 100644
index 0000000..2e118fa
--- /dev/null
+++ b/etc/bash_completion.d/yab
@@ -0,0 +1,20 @@
+# bash completion for yab executable
+# based on https://godoc.org/github.com/jessevdk/go-flags#hdr-Completion
+# depends on https://salsa.debian.org/debian/bash-completion
+
+
+have yab &&
+_yab()
+{
+    # All arguments except the first one
+    args=("${COMP_WORDS[@]:1:$COMP_CWORD}")
+
+    # Only split on newlines
+    local IFS=$'\n'
+
+    # Call completion (note that the first element of COMP_WORDS is
+    # the executable itself)
+    COMPREPLY=($(GO_FLAGS_COMPLETION=1 ${COMP_WORDS[0]} "${args[@]}"))
+    return 0
+} &&
+complete -F _yab yab
diff --git a/internal/yamlalias/wrap.go b/internal/yamlalias/wrap.go
deleted file mode 100644
index a70a5ec..0000000
--- a/internal/yamlalias/wrap.go
+++ /dev/null
@@ -1,123 +0,0 @@
-// Package yamlalias adds support for aliases when parsing YAML.
-//
-// Aliases are specified as a comma-separated list specified via
-// "yaml-aliases" struct tag.
-//
-// Example:
-//
-//   type Config struct {
-//     UserName string `yaml:"username" yaml-aliases:"userName,user-name"`
-//   }
-//
-// This will parse the following YAML examples in the same way:
-//   {"username": "John Doe"}
-//   {"userName": "John Doe"}
-//   {"user-name": "John Doe"}
-//
-// Note: It only adds aliases for top-level fields in the struct.
-package yamlalias
-
-import (
-	"fmt"
-	"reflect"
-	"strings"
-
-	"gopkg.in/yaml.v2"
-)
-
-// Unmarshal is a wrapper for yaml.Unmarshal but adds
-// support for any yaml-aliases specified in out.
-func Unmarshal(in []byte, out interface{}) error {
-	return yaml.Unmarshal(in, addAliases(out))
-}
-
-// UnmarshalStrict is a wrapper for yaml.UnmarshalStrict but adds
-// support for any yaml-aliases specified in out.
-func UnmarshalStrict(in []byte, out interface{}) error {
-	return yaml.UnmarshalStrict(in, addAliases(out))
-}
-
-// addAliases generates a new struct for unmarshalling that contains all exported
-// fields from the passed in struct, but also adds additional fields for any
-// aliases.
-//
-// Given a struct such as:
-//
-//   type Config struct {
-//     UserName string `yaml:"username" yaml-aliases:"userName,user-name"`
-//     TTL time.Duration `yaml:"ttl"`
-//   }
-//
-// It will create a new struct with a pointer field for each field in the
-// original struct and a new field for each alias.
-//
-//   struct {
-//     UserName *string `yaml:"username" yaml-aliases:"userName,user-name"`
-//     UserNameYamlAlias1 *string `yaml:"userName"`
-//     UserNameYamlAlias2 *string `yaml:"user-name"`
-//     TTL *time.Duration `yaml:"ttl"`
-//   }
-//
-// A value of that struct is returned with the pointer fields pointing to
-// fields of the original struct.
-//
-// out.UserName = &dest.UserName
-// out.UserNameYamlAlias1 = &dest.UserName
-// out.UserNameYamlAlias2 = &dest.UserName
-// out.TTL = &dest.TTL
-// return out
-//
-// The YAML library respects existing pointers when unmarshalling, and
-// does not replace them:
-// https://gist.github.com/prashantv/fa4f92b4b95f936d68495be250ed3506
-func addAliases(dest interface{}) interface{} {
-	rv := reflect.ValueOf(dest).Elem()
-	rt := rv.Type()
-
-	fields := make([]reflect.StructField, 0, rt.NumField())
-	dests := make([]reflect.Value, 0, rt.NumField())
-	for i := 0; i < rt.NumField(); i++ {
-		f := rt.Field(i)
-
-		// Ignore unexported fields.
-		if f.PkgPath != "" {
-			continue
-		}
-
-		// Add a pointer field that matches the original field in the struct.
-		fields = append(fields, reflect.StructField{
-			Name: f.Name,
-			Type: reflect.PtrTo(f.Type),
-			Tag:  f.Tag,
-		})
-		dests = append(dests, rv.Field(i).Addr())
-
-		allAliases, ok := f.Tag.Lookup("yaml-aliases")
-		if !ok {
-			// No aliases specified. No other fields to add.
-			continue
-		}
-
-		aliases := strings.Split(allAliases, ",")
-		for j, alias := range aliases {
-			// Add a new field that is a pointer to the original field.
-			// We generate a name that is unlikely to clash. Clashes will cause panics.
-			fields = append(fields, reflect.StructField{
-				Name: fmt.Sprintf("%vYamlAlias%v", f.Name, j),
-				Type: reflect.PtrTo(f.Type),
-				Tag:  reflect.StructTag(fmt.Sprintf(`yaml:%q`, alias)),
-			})
-			dests = append(dests, rv.Field(i).Addr())
-		}
-	}
-
-	// The struct we generated has pointers to the original fields.
-	// Set all the pointers to the fields in the original struct.
-	generated := reflect.StructOf(fields)
-	out := reflect.New(generated)
-	for i, dest := range dests {
-		out.Elem().Field(i).Set(dest)
-	}
-
-	return out.Interface()
-}
diff --git a/main.go b/main.go
index 8f33518..0041fa6 100644
--- a/main.go
+++ b/main.go
@@ -338,8 +338,6 @@ func runWithOptions(opts Options, out output, logger *zap.Logger) {
 		out.Fatalf("Failed while parsing options: %v\n", err)
 	}
 
-	serializer = withTransportSerializer(transport.Protocol(), serializer, opts.ROpts)
-
 	// req is the transport.Request that will be used to make a call.
 	req, err := serializer.Request(reqInput)
 	if err != nil {
@@ -361,10 +359,6 @@ func runWithOptions(opts Options, out output, logger *zap.Logger) {
 	})
 }
 
-type noEnveloper interface {
-	WithoutEnvelopes() encoding.Serializer
-}
-
 func createJaegerTracer(opts Options, out output) (opentracing.Tracer, io.Closer) {
 	// yab must set the `SynchronousInitialization` flag to indicate that
 	// the Jaeger client must fetch debug credits synchronously. In a
@@ -406,18 +400,6 @@ func getTracer(opts Options, out output) (opentracing.Tracer, io.Closer) {
 	return opentracing.NoopTracer{}, nil
 }
 
-// withTransportSerializer may modify the serializer for the transport used.
-// E.g. Thrift payloads are not enveloped when used with TChannel or gRPC.
-func withTransportSerializer(p transport.Protocol, s encoding.Serializer, rOpts RequestOptions) encoding.Serializer {
-	// TODO: We can move this logic into NewSerializer as we resolved transport protocol and encoding together.
-	switch {
-	case (p == transport.TChannel || p == transport.GRPC) && s.Encoding() == encoding.Thrift,
-		rOpts.ThriftDisableEnvelopes:
-		s = s.(noEnveloper).WithoutEnvelopes()
-	}
-	return s
-}
-
 type resolvedProtocolEncoding struct {
 	protocol transport.Protocol
 	enc      encoding.Encoding
diff --git a/options.go b/options.go
index f11657c..d594ec6 100644
--- a/options.go
+++ b/options.go
@@ -174,6 +174,10 @@ func (s *stringAlias) UnmarshalFlag(value string) error {
 	return nil
 }
 
+func (s *stringAlias) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	return unmarshal(s.dest)
+}
+
 func setEncodingOptions(opts *Options) {
 	if opts.ROpts.Aliases.JSON {
 		opts.ROpts.Encoding = encoding.JSON
diff --git a/request.go b/request.go
index 8baf98c..d8d26d9 100644
--- a/request.go
+++ b/request.go
@@ -100,7 +100,19 @@ func NewSerializer(opts Options, resolved resolvedProtocolEncoding) (encoding.Se
 	// procedure check for non-Thrift encodings.
 	switch resolved.enc {
 	case encoding.Thrift:
-		return encoding.NewThrift(opts.ROpts.ThriftFile, opts.ROpts.Procedure, opts.ROpts.ThriftMultiplexed)
+		envelope := !opts.ROpts.ThriftDisableEnvelopes
+
+		// TChannel and gRPC never use envelopes.
+		if resolved.protocol == transport.TChannel || resolved.protocol == transport.GRPC {
+			envelope = false
+		}
+
+		return encoding.NewThrift(encoding.ThriftParams{
+			File:        opts.ROpts.ThriftFile,
+			Method:      opts.ROpts.Procedure,
+			Envelope:    envelope,
+			Multiplexed: opts.ROpts.ThriftMultiplexed,
+		})
 	case encoding.Protobuf:
 		descSource, err := newProtoDescriptorProvider(opts.ROpts, opts.TOpts, resolved)
 		if err != nil {
diff --git a/template.go b/template.go
index c2a3710..18a0f28 100644
--- a/template.go
+++ b/template.go
@@ -27,25 +27,41 @@ import (
 	"strings"
 	"time"
 
-	"github.com/yarpc/yab/internal/yamlalias"
 	"github.com/yarpc/yab/templateargs"
 
 	"gopkg.in/yaml.v2"
 )
 
 type template struct {
-	Peers                 []string `yaml:"peers"`
-	Peer                  string   `yaml:"peer"`
-	PeerList              string   `yaml:"peerList" yaml-aliases:"peerlist,peer-list"`
-	Caller                string   `yaml:"caller"`
-	Service               string   `yaml:"service"`
-	Thrift                string   `yaml:"thrift"`
-	Procedure             string   `yaml:"procedure" yaml-aliases:"method"`
-	DisableThriftEnvelope *bool    `yaml:"disableThriftEnvelope" yaml-aliases:"disablethriftenvelope,disable-thrift-envelope"`
-
-	ShardKey        string `yaml:"shardKey" yaml-aliases:"shardkey,shard-key,sk"`
-	RoutingKey      string `yaml:"routingKey" yaml-aliases:"routingkey,routing-key,rk"`
-	RoutingDelegate string `yaml:"routingDelegate" yaml-aliases:"routingdelegate,routing-delegate,rd"`
+	Peers                     []string    `yaml:"peers"`
+	Peer                      string      `yaml:"peer"`
+	PeerList                  string      `yaml:"peerList"`
+	Peerlist                  stringAlias `yaml:"peerlist"`
+	PeerDashList              stringAlias `yaml:"peer-list"`
+	Caller                    string      `yaml:"caller"`
+	Service                   string      `yaml:"service"`
+	Thrift                    string      `yaml:"thrift"`
+	Procedure                 string      `yaml:"procedure"`
+	Method                    stringAlias `yaml:"method"`
+	DisableThriftEnvelope     *bool       `yaml:"disableThriftEnvelope"`
+	DisableDashThriftEnvelope **bool      `yaml:"disable-thrift-envelope"`
+	Disablethriftenvelope     **bool      `yaml:"disablethriftenvelope"`
+
+	ShardKey        string `yaml:"shardKey"`
+	RoutingKey      string `yaml:"routingKey"`
+	RoutingDelegate string `yaml:"routingDelegate"`
+
+	Shardkey        stringAlias `yaml:"shardkey"`
+	Routingkey      stringAlias `yaml:"routingkey"`
+	Routingdelegate stringAlias `yaml:"routingdelegate"`
+
+	ShardDashKey        stringAlias `yaml:"shard-key"`
+	RoutingDashKey      stringAlias `yaml:"routing-key"`
+	RoutingDashDelegate stringAlias `yaml:"routing-delegate"`
+
+	SK stringAlias `yaml:"sk"`
+	RK stringAlias `yaml:"rk"`
+	RD stringAlias `yaml:"rd"`
 
 	Headers map[string]string           `yaml:"headers"`
 	Baggage map[string]string           `yaml:"baggage"`
@@ -80,12 +96,11 @@ func readYAMLFile(yamlTemplate string, templateArgs map[string]string, opts *Opt
 }
 
 func readYAMLRequest(base string, contents []byte, templateArgs map[string]string, opts *Options) error {
-	var t template
-	if err := yamlalias.UnmarshalStrict(contents, &t); err != nil {
+	t, err := unmarshalTemplate(contents)
+	if err != nil {
 		return err
 	}
 
-	var err error
 	t.Request, err = templateargs.ProcessMap(t.Request, templateArgs)
 	if err != nil {
 		return err
@@ -151,6 +166,33 @@ func overrideParam(s *string, newS string) {
 	}
 }
 
+func unmarshalTemplate(bytes []byte) (*template, error) {
+	t := &template{}
+
+	t.Method.dest = &t.Procedure
+
+	t.Peerlist.dest = &t.PeerList
+	t.PeerDashList.dest = &t.PeerList
+
+	t.Shardkey.dest = &t.ShardKey
+	t.Routingkey.dest = &t.RoutingKey
+	t.Routingdelegate.dest = &t.RoutingDelegate
+
+	t.ShardDashKey.dest = &t.ShardKey
+	t.RoutingDashKey.dest = &t.RoutingKey
+	t.RoutingDashDelegate.dest = &t.RoutingDelegate
+
+	t.SK.dest = &t.ShardKey
+	t.RK.dest = &t.RoutingKey
+	t.RD.dest = &t.RoutingDelegate
+
+	t.Disablethriftenvelope = &t.DisableThriftEnvelope
+	t.DisableDashThriftEnvelope = &t.DisableThriftEnvelope
+
+	err := yaml.Unmarshal(bytes, &t)
+	return t, err
+}
+
 type headers map[string]string
 
 // In these cases, the existing item (target, from flags) overrides the source


Test patch:
diff --git a/bench_method_test.go b/bench_method_test.go
index c1b67a1..0e3b79f 100644
--- a/bench_method_test.go
+++ b/bench_method_test.go
@@ -51,8 +51,6 @@ func benchmarkMethodForROpts(t *testing.T, rOpts RequestOptions, p transport.Pro
 	})
 	require.NoError(t, err, "Failed to create Thrift serializer")
 
-	serializer = withTransportSerializer(p, serializer, rOpts)
-
 	req, err := serializer.Request(nil)
 	require.NoError(t, err, "Failed to serialize Thrift body")
 
diff --git a/encoding/thrift_request_test.go b/encoding/thrift_request_test.go
index e186ebb..dac0dda 100644
--- a/encoding/thrift_request_test.go
+++ b/encoding/thrift_request_test.go
@@ -90,7 +90,13 @@ func TestNewThriftSerializer(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			got, err := NewThrift(tt.file, tt.method, tt.multiplexed)
+			p := ThriftParams{
+				File:        tt.file,
+				Method:      tt.method,
+				Multiplexed: tt.multiplexed,
+				Envelope:    true,
+			}
+			got, err := NewThrift(p)
 			if tt.errMsg == "" {
 				require.NoError(t, err)
 				require.NotNil(t, got, "successful case should return Serializer")
@@ -139,7 +145,12 @@ func TestRequest(t *testing.T) {
 			method = "foo"
 		}
 
-		serializer, err := NewThrift(validThrift, "Simple::"+method, false /* multiplexed */)
+		serializer, err := NewThrift(ThriftParams{
+			File:        validThrift,
+			Method:      "Simple::" + method,
+			Multiplexed: false,
+			Envelope:    true,
+		})
 		require.NoError(t, err, "Failed to create serializer")
 
 		got, err := serializer.Request(tt.bs)
@@ -277,13 +288,14 @@ func TestFindMethod(t *testing.T) {
 
 func TestWithoutEnvelopes(t *testing.T) {
 	tests := []struct {
-		desc             string
-		multiplexed      bool
-		withoutEnvelopes bool
-		want             []byte
+		desc        string
+		multiplexed bool
+		envelope    bool
+		want        []byte
 	}{
 		{
-			desc: "with envelope",
+			desc:     "with envelope",
+			envelope: true,
 			want: []byte{
 				0x80, 0x01, 0x00, 0x01, // version | type = 1 | call
 				0x00, 0x00, 0x00, 0x03, 'f', 'o', 'o', // "foo"
@@ -294,6 +306,7 @@ func TestWithoutEnvelopes(t *testing.T) {
 		{
 			desc:        "with envelope, multiplexed",
 			multiplexed: true,
+			envelope:    true,
 			want: []byte{
 				0x80, 0x01, 0x00, 0x01, // version | type = 1 | call
 				0x00, 0x00, 0x00, 0x0A, // length of method
@@ -303,25 +316,27 @@ func TestWithoutEnvelopes(t *testing.T) {
 			},
 		},
 		{
-			desc:             "without envelope",
-			withoutEnvelopes: true,
-			want:             []byte{0x00},
+			desc:     "without envelope",
+			envelope: false,
+			want:     []byte{0x00},
 		},
 		{
-			desc:             "without envelope, multiplexed",
-			multiplexed:      true, // has no effect when there are no envelopes.
-			withoutEnvelopes: true,
-			want:             []byte{0x00},
+			desc:        "without envelope, multiplexed",
+			multiplexed: true, // has no effect when there are no envelopes.
+			envelope:    false,
+			want:        []byte{0x00},
 		},
 	}
 
 	for _, tt := range tests {
-		serializer, err := NewThrift(validThrift, "Simple::foo", tt.multiplexed)
-		require.NoError(t, err, "Failed to create serializer")
 
-		if tt.withoutEnvelopes {
-			serializer = serializer.(thriftSerializer).WithoutEnvelopes()
-		}
+		serializer, err := NewThrift(ThriftParams{
+			File:        validThrift,
+			Method:      "Simple::foo",
+			Multiplexed: tt.multiplexed,
+			Envelope:    tt.envelope,
+		})
+		require.NoError(t, err, "Failed to create serializer")
 
 		got, err := serializer.Request([]byte("{}"))
 		require.NoError(t, err, "%v: serialize failed", tt.desc)
diff --git a/internal/yamlalias/wrap_test.go b/internal/yamlalias/wrap_test.go
deleted file mode 100644
index d1b4ea4..0000000
--- a/internal/yamlalias/wrap_test.go
+++ /dev/null
@@ -1,164 +0,0 @@
-package yamlalias
-
-import (
-	"reflect"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"gopkg.in/yaml.v2"
-)
-
-func stringPtr(s string) *string {
-	return &s
-}
-
-func TestWrap(t *testing.T) {
-	type Simple struct {
-		Foo string `yaml:"foo" yaml-aliases:"bar,baz"`
-	}
-
-	type ManyTypes struct {
-		Str   string            `yaml-aliases:"string"`
-		Int   int               `yaml-aliases:"Int"`
-		Slice []string          `yaml-aliases:"list"`
-		Map   map[string]string `yaml-aliases:"dict"`
-	}
-
-	type AliasToPtr struct {
-		Foo *string `yaml:"foo" yaml-aliases:"bar"`
-	}
-
-	tests := []struct {
-		msg  string
-		data string
-		want interface{}
-	}{
-		{
-			msg:  "No aliases",
-			data: `{"foo": "foo", "bar": "bar"}`,
-			want: &struct {
-				Foo string
-				Bar string
-			}{"foo", "bar"},
-		},
-		{
-			msg:  "set field by normal name",
-			data: `{"foo": "foo"}`,
-			want: &Simple{"foo"},
-		},
-		{
-			msg:  "set field by first alias",
-			data: `{"bar": "bar"}`,
-			want: &Simple{"bar"},
-		},
-		{
-			msg:  "set field by second alias",
-			data: `{"baz": "baz"}`,
-			want: &Simple{"baz"},
-		},
-		{
-			msg:  "set field with multiple aliases, last one wins",
-			data: `{"bar": "bar", "foo": "foo", "baz": "baz"}`,
-			want: &Simple{"baz"},
-		},
-		{
-			msg:  "pointer field that is not set",
-			data: `{}`,
-			want: &AliasToPtr{},
-		},
-		{
-			msg:  "pointer field set directly",
-			data: `{"foo": "foo"}`,
-			want: &AliasToPtr{stringPtr("foo")},
-		},
-		{
-			msg:  "pointer field that is set by alias",
-			data: `{"bar": "bar"}`,
-			want: &AliasToPtr{stringPtr("bar")},
-		},
-		{
-			msg: "Unexported fields are ignored",
-			want: &struct {
-				Foo string `yaml:"foo" yaml-aliases:"bar"`
-				bar string `yaml:"bar" yaml-aliases:"baz"`
-			}{
-				Foo: "bar",
-				bar: "",
-			},
-			data: `{"bar": "bar"}`,
-		},
-		{
-			msg: "Alias with different case",
-			want: &struct {
-				Foo string `yaml:"foo" yaml-aliases:"Foo"`
-			}{
-				Foo: "Foo",
-			},
-			data: `{"Foo": "Foo"}`,
-		},
-		{
-			msg: "Alias with dashes",
-			want: &struct {
-				Foo string `yaml:"foo" yaml-aliases:"foo-bar"`
-			}{
-				Foo: "foo-bar",
-			},
-			data: `{"foo-bar": "foo-bar"}`,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.msg, func(t *testing.T) {
-			// To allow use of anonymous structs, we use reflection to create
-			// a new empty struct.
-			v := reflect.New(reflect.TypeOf(tt.want).Elem()).Interface()
-			require.NoError(t, Unmarshal([]byte(tt.data), v))
-			assert.Equal(t, tt.want, v)
-		})
-	}
-}
-
-func TestWrapPointerField(t *testing.T) {
-	var dest string
-	type StructWithPointer struct {
-		Foo *string `yaml:"foo" yaml-aliases:"bar"`
-	}
-
-	s := StructWithPointer{Foo: &dest}
-	require.NoError(t, yaml.Unmarshal([]byte(`{"foo": "initial"}`), &s), "failed to unmarshal")
-	assert.Equal(t, "initial", dest, "dest expected to be updated")
-
-	require.NoError(t, Unmarshal([]byte(`{"bar": "updated"}`), &s), "failed to unmarshal with alias")
-	assert.Equal(t, "updated", dest, "expected dest to be updated via alias")
-}
-
-func TestWrapInvalid(t *testing.T) {
-	tests := []struct {
-		msg string
-		t   interface{}
-	}{
-		{
-			msg: "Alias clashes with other field",
-			t: &struct {
-				Foo string `yaml:"foo" yaml-aliases:"bar,baz"`
-				Bar string `yaml:"bar"`
-			}{},
-		},
-		{
-			msg: "Alias clashes with another alias",
-			t: &struct {
-				Foo string `yaml:"foo" yaml-aliases:"baz"`
-				Bar string `yaml:"bar" yaml-aliases:"baz"`
-			}{},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.msg, func(t *testing.T) {
-			assert.Panics(t, func() {
-				Unmarshal([]byte(`{}`), &tt.t)
-			})
-		})
-	}
-}
diff --git a/main_test.go b/main_test.go
index 96fed58..b7523c4 100644
--- a/main_test.go
+++ b/main_test.go
@@ -567,7 +567,7 @@ func TestGetOptionsPrintsPluginErrors(t *testing.T) {
 	require.Contains(t, warnBuf.String(), "WARNING: Error adding plugin-based custom flags")
 }
 
-func TestAliases(t *testing.T) {
+func TestAlises(t *testing.T) {
 	type cmdArgs []string
 
 	tests := []struct {
@@ -577,11 +577,12 @@ func TestAliases(t *testing.T) {
 	}{
 		{
 			args: []cmdArgs{
-				{"--timeout", "5s"},
-				{"--timeout", "5000"},
+				{"--timeout", "1s"},
+				{"--timeout", "1000"},
+				{"-t", "1000"},
 			},
 			validate: func(args cmdArgs, opts *Options) {
-				assert.Equal(t, 5*time.Second, opts.ROpts.Timeout.Duration(), "Args: %v", args)
+				assert.Equal(t, time.Second, opts.ROpts.Timeout.Duration(), "Args: %v", args)
 			},
 		},
 		{
@@ -998,7 +999,7 @@ func TestResolveProtocolEncoding(t *testing.T) {
 	}
 }
 
-func TestWithTransportSerializer(t *testing.T) {
+func TestDetectThriftEnvelopes(t *testing.T) {
 	validRequestOpts := RequestOptions{
 		ThriftFile: validThrift,
 		Procedure:  fooMethod,
@@ -1056,7 +1057,6 @@ func TestWithTransportSerializer(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.msg, func(t *testing.T) {
-
 			resolved := resolvedProtocolEncoding{
 				protocol: tt.protocol,
 				enc:      encoding.Thrift,
@@ -1064,7 +1064,6 @@ func TestWithTransportSerializer(t *testing.T) {
 			serializer, err := NewSerializer(Options{ROpts: tt.rOpts}, resolved)
 			require.NoError(t, err, "Failed to create serializer for %+v", tt.rOpts)
 
-			serializer = withTransportSerializer(tt.protocol, serializer, tt.rOpts)
 			req, err := serializer.Request(nil)
 			require.NoError(t, err, "Failed to serialize request for %+v", tt.rOpts)
 
diff --git a/template_test.go b/template_test.go
index 5a3f773..cffe7c3 100644
--- a/template_test.go
+++ b/template_test.go
@@ -29,7 +29,6 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"github.com/yarpc/yab/internal/yamlalias"
 )
 
 func mustReadYAMLFile(t *testing.T, yamlTemplate string, opts *Options) {
@@ -293,10 +292,9 @@ func TestTemplateAlias(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		for _, in := range tt.templates {
-			t.Run(in, func(t *testing.T) {
-				var templ template
-				err := yamlalias.Unmarshal([]byte(in), &templ)
+		for _, template := range tt.templates {
+			t.Run(template, func(t *testing.T) {
+				templ, err := unmarshalTemplate([]byte(template))
 				require.NoError(t, err)
 				assert.Equal(t, tt.wantShardKey, templ.ShardKey, "shard key aliases expanded")
 				assert.Equal(t, tt.wantRoutingKey, templ.RoutingKey, "routing key aliases expanded")
@@ -314,23 +312,23 @@ func TestDisableThriftEnvelopeOverride(t *testing.T) {
 		wantDisableThriftEnvelope bool
 	}{
 		{
-			initial:                   false,
-			yaml:                      "method: foo",
+			initial: false,
+			yaml:    "method: foo",
 			wantDisableThriftEnvelope: false,
 		},
 		{
-			initial:                   true,
-			yaml:                      "method: foo",
+			initial: true,
+			yaml:    "method: foo",
 			wantDisableThriftEnvelope: true,
 		},
 		{
-			initial:                   false,
-			yaml:                      "disable-thrift-envelope: true",
+			initial: false,
+			yaml:    "disable-thrift-envelope: true",
 			wantDisableThriftEnvelope: true,
 		},
 		{
-			initial:                   true,
-			yaml:                      "disable-thrift-envelope: true",
+			initial: true,
+			yaml:    "disable-thrift-envelope: true",
 			wantDisableThriftEnvelope: true,
 		},
 	}