Add seperate testsuites for value and receiver

This completes work on seperating tests and brings test coverage
to the ~90%. More corner cases will be covered as they arise.

engine/context_test.go

@@ -7,33 +7,18 @@ package engine
 import (
 	"bytes"
 	"fmt"
-	"io/ioutil"
 	"net/http"
-	"os"
 	"reflect"
 	"testing"
 )
 
-func writeTempScript(name, script string) (*os.File, error) {
-	file, err := ioutil.TempFile("", name)
-	if err != nil {
-		return nil, err
-	}
-
-	if _, err := file.WriteString(script); err != nil {
-		file.Close()
-		os.Remove(file.Name())
-
-		return nil, err
-	}
-
-	return file, nil
+func TestContextStart(t *testing.T) {
+	e, _ = New()
+	t.SkipNow()
 }
 
 func TestContextNew(t *testing.T) {
-	e, _ = New()
 	c, err := NewContext()
-
 	if err != nil {
 		t.Fatalf("NewContext(): %s", err)
 	}
@@ -69,13 +54,13 @@ func TestContextExec(t *testing.T) {
 	c.Output = &w
 
 	for _, tt := range execTests {
-		file, err := writeTempScript(tt.name, tt.script)
+		script, err := NewScript(tt.name, tt.script)
 		if err != nil {
 			t.Errorf("Could not create temporary file for testing: %s", tt.name, err)
 			continue
 		}
 
-		if err := c.Exec(file.Name()); err != nil {
+		if err := c.Exec(script.Name()); err != nil {
 			t.Errorf("Context.Exec('%s'): Execution failed: %s", tt.name, err)
 			continue
 		}
@@ -87,8 +72,7 @@ func TestContextExec(t *testing.T) {
 			t.Errorf("Context.Exec('%s'): Expected `%s', actual `%s'", tt.name, tt.expected, actual)
 		}
 
-		file.Close()
-		os.Remove(file.Name())
+		script.Remove()
 	}
 
 	c.Destroy()
@@ -276,7 +260,7 @@ func TestContextBind(t *testing.T) {
 	c, _ := NewContext()
 	c.Output = &w
 
-	file, err := writeTempScript("evaltest.php", `<?php $i = (isset($i)) ? $i += 1 : 0; echo serialize($$i);`)
+	script, err := NewScript("bind.php", "<?php $i = (isset($i)) ? $i += 1 : 0; echo serialize($$i);")
 	if err != nil {
 		t.Fatalf("Could not create temporary file for testing: %s", err)
 	}
@@ -287,7 +271,7 @@ func TestContextBind(t *testing.T) {
 			continue
 		}
 
-		if err := c.Exec(file.Name()); err != nil {
+		if err := c.Exec(script.Name()); err != nil {
 			t.Errorf("Context.Exec(): %s", err)
 			continue
 		}
@@ -300,9 +284,7 @@ func TestContextBind(t *testing.T) {
 		}
 	}
 
-	file.Close()
-	os.Remove(file.Name())
-
+	script.Remove()
 	c.Destroy()
 }
 
@@ -311,8 +293,14 @@ func TestContextDestroy(t *testing.T) {
 	c.Destroy()
 
 	if c.context != nil || c.values != nil {
-		t.Errorf("Destroy(): Did not set internal fields to `nil`")
+		t.Errorf("Context.Destroy(): Did not set internal fields to `nil`")
 	}
 
+	// Attempting to destroy a context twice should be a no-op.
+	c.Destroy()
+}
+
+func TestContextEnd(t *testing.T) {
 	e.Destroy()
+	t.SkipNow()
 }

engine/engine_test.go

@@ -5,9 +5,36 @@
 package engine
 
 import (
+	"io/ioutil"
+	"os"
 	"testing"
 )
 
+type Script struct {
+	*os.File
+}
+
+func NewScript(name, contents string) (*Script, error) {
+	file, err := ioutil.TempFile("", name)
+	if err != nil {
+		return nil, err
+	}
+
+	if _, err := file.WriteString(contents); err != nil {
+		file.Close()
+		os.Remove(file.Name())
+
+		return nil, err
+	}
+
+	return &Script{file}, nil
+}
+
+func (s *Script) Remove() {
+	s.Close()
+	os.Remove(s.Name())
+}
+
 var e *Engine
 
 func TestEngineNew(t *testing.T) {
@@ -34,23 +61,33 @@ func TestEngineNewContext(t *testing.T) {
 }
 
 func TestEngineDefine(t *testing.T) {
-	err := e.Define("TestDefine", func(args []interface{}) interface{} {
+	ctor := func(args []interface{}) interface{} {
 		return nil
-	})
+	}
 
+	err := e.Define("TestDefine", ctor)
 	if err != nil {
 		t.Errorf("Define(): %s", err)
 	}
 
 	if len(e.receivers) != 1 {
 		t.Errorf("Define(): `Engine.receivers` length is %d, should be 1", len(e.receivers))
 	}
+
+	err = e.Define("TestDefine", ctor)
+	if err == nil {
+		t.Errorf("Define(): Incorrectly defined duplicate receiver")
+	}
+
 }
 
 func TestEngineDestroy(t *testing.T) {
 	e.Destroy()
 
 	if e.engine != nil || e.contexts != nil || e.receivers != nil {
-		t.Errorf("Destroy(): Did not set internal fields to `nil`")
+		t.Errorf("Engine.Destroy(): Did not set internal fields to `nil`")
 	}
+
+	// Attempting to destroy an engine instance twice should be a no-op.
+	e.Destroy()
 }

engine/receiver.go

@@ -96,11 +96,21 @@ func receiverNew(rcvr unsafe.Pointer, args unsafe.Pointer) unsafe.Pointer {
 	vi := reflect.Indirect(v)
 
 	for i := 0; i < v.NumMethod(); i++ {
+		// Skip unexported methods.
+		if v.Type().Method(i).PkgPath != "" {
+			continue
+		}
+
 		obj.methods[v.Type().Method(i).Name] = v.Method(i)
 	}
 
 	if vi.Kind() == reflect.Struct {
 		for i := 0; i < vi.NumField(); i++ {
+			// Skip unexported fields.
+			if vi.Type().Field(i).PkgPath != "" {
+				continue
+			}
+
 			obj.values[vi.Type().Field(i).Name] = vi.Field(i)
 		}
 	}
@@ -113,7 +123,7 @@ func receiverGet(obj unsafe.Pointer, name *C.char) unsafe.Pointer {
 	o := (*object)(obj)
 	n := C.GoString(name)
 
-	if _, exists := o.values[n]; !exists {
+	if _, exists := o.values[n]; !exists || !o.values[n].CanInterface() {
 		return nil
 	}
 
@@ -185,8 +195,8 @@ func receiverCall(obj unsafe.Pointer, name *C.char, args unsafe.Pointer) unsafe.
 	// element, otherwise returns a slice of values.
 	if len(ret) > 1 {
 		t := make([]interface{}, len(ret))
-		for _, v := range ret {
-			t = append(t, v.Interface())
+		for i, v := range ret {
+			t[i] = v.Interface()
 		}
 
 		result = t

engine/receiver_test.go

@@ -0,0 +1,133 @@
+// Copyright 2016 Alexander Palaistras. All rights reserved.
+// Use of this source code is governed by the MIT license that can be found in
+// the LICENSE file.
+
+package engine
+
+import (
+	"bytes"
+	"testing"
+)
+
+func TestReceiverStart(t *testing.T) {
+	e, _ = New()
+	t.SkipNow()
+}
+
+type testReceiver struct {
+	Var    string
+	hidden int64
+}
+
+func (t *testReceiver) Ignore() {
+}
+
+func (t *testReceiver) Hello(p string) string {
+	return "Hello " + p
+}
+
+func (t *testReceiver) Goodbye(p string) (string, string) {
+	return "Goodbye", p
+}
+
+func (t *testReceiver) invalid() string {
+	return "I'm afraid I can't let you do that, Dave"
+}
+
+func newTestReceiver(args []interface{}) interface{} {
+	value := "Foo"
+
+	if len(args) > 0 {
+		switch v := args[0].(type) {
+		case bool:
+			return nil
+		case string:
+			value = v
+		}
+	}
+
+	return &testReceiver{Var: value, hidden: 42}
+}
+
+var newReceiverTests = []struct {
+	script   string
+	expected string
+}{
+	{"$t = new TestReceiver; echo is_object($t);", "1"},
+	{`try {
+		$t = new TestReceiver(false);
+	} catch (Exception $e) {
+		echo $e->getMessage();
+	}`, "Failed to instantiate method receiver"},
+
+	{"$t = new TestReceiver; echo $t->Var;", "Foo"},
+	{"$t = new TestReceiver; echo $t->hidden;", ""},
+	{"$t = new TestReceiver('wow'); echo $t->Var;", "wow"},
+
+	{"$t = new TestReceiver; $t->Var = 'Bar'; echo $t->Var;", "Bar"},
+	{"$t = new TestReceiver; $t->hello = 'wow'; echo $t->hello;", ""},
+
+	{"$t = new TestReceiver; echo $t->Ignore();", ""},
+	{"$t = new TestReceiver; echo $t->Hello('World');", "Hello World"},
+	{"$t = new TestReceiver; echo json_encode($t->Goodbye('Doge'));", `["Goodbye","Doge"]`},
+	{"$t = new TestReceiver; echo $t->invalid();", ""},
+
+	{"$t = new TestReceiver; echo ($t->Var) ? 1 : 0;", "1"},
+	{"$t = new TestReceiver; echo isset($t->Var) ? 1 : 0;", "1"},
+	{"$t = new TestReceiver; echo empty($t->Var) ? 1 : 0;", "0"},
+
+	{"$t = new TestReceiver; echo isset($t->hidden) ? 1 : 0;", "0"},
+}
+
+func TestNewReceiver(t *testing.T) {
+	var w bytes.Buffer
+
+	c, _ := NewContext()
+	c.Output = &w
+
+	r, err := NewReceiver("TestReceiver", newTestReceiver)
+	if err != nil {
+		t.Fatalf("NewReceiver(): Failed to define method receiver: %s", err)
+	}
+
+	for _, tt := range newReceiverTests {
+		_, err := c.Eval(tt.script)
+		if err != nil {
+			t.Errorf("Context.Eval('%s'): %s", tt.script, err)
+			continue
+		}
+
+		actual := w.String()
+		w.Reset()
+
+		if actual != tt.expected {
+			t.Errorf("Context.Eval('%s'): Expected output '%s', actual '%s'", tt.script, tt.expected, actual)
+		}
+	}
+
+	r.Destroy()
+	c.Destroy()
+}
+
+func TestReceiverDestroy(t *testing.T) {
+	c, _ := NewContext()
+	defer c.Destroy()
+
+	r, err := NewReceiver("TestReceiver", newTestReceiver)
+	if err != nil {
+		t.Fatalf("NewReceiver(): Failed to define method receiver: %s", err)
+	}
+
+	r.Destroy()
+	if r.create != nil || r.objects != nil {
+		t.Errorf("Receiver.Destroy(): Did not set internal fields to `nil`")
+	}
+
+	// Attempting to destroy a receiver twice should be a no-op.
+	r.Destroy()
+}
+
+func TestReceiverEnd(t *testing.T) {
+	e.Destroy()
+	t.SkipNow()
+}