oasis-test-runner: Implement ParameterFlagSet.Clone()

.changelog/2897.internal.md

@@ -1,5 +1,6 @@
 oasis-test-runner: Refactor initialization of scenario flags
 
-Implementations of `Parameters()` scenario function have been revised and all
-scenario-settable flags are now explicitly initialized and accessed by using
-standard `FlagSet` getters and setters.
+Implementations of `Parameters()` function defined in test-runner's scenario
+interface have been revised. All scenario-settable flags are now explicitly
+initialized and scenarios call standard `FlagSet` accessors to fetch
+scenario-specific parameters.

go/oasis-test-runner/cmd/root.go

@@ -94,7 +94,7 @@ func RegisterNondefault(s scenario.Scenario) error {
 // RegisterTestParams registers given scenario parameters as string slices regardless of actual type.
 //
 // Later we combine specific parameter sets and execute tests with all parameter combinations.
-func RegisterTestParams(name string, p *flag.FlagSet) {
+func RegisterTestParams(name string, p *env.ParameterFlagSet) {
 	fs := flag.NewFlagSet(name, flag.ContinueOnError)
 	p.VisitAll(func(f *flag.Flag) {
 		fs.StringSlice(name+"."+f.Name, []string{f.Value.String()}, f.Usage)
@@ -342,7 +342,7 @@ func runRoot(cmd *cobra.Command, args []string) error {
 				childEnv, err := rootEnv.NewChild(n, &env.TestInstanceInfo{
 					Test:         v.Name(),
 					Instance:     filepath.Base(rootEnv.Dir()),
-					ParameterSet: &env.ParameterFlagSet{FlagSet: *v.Parameters()},
+					ParameterSet: v.Parameters(),
 					Run:          run,
 				})
 				if err != nil {

go/oasis-test-runner/env/env.go

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"os/exec"
 	"path/filepath"
+	"reflect"
 	"sync"
 	"syscall"
 	"time"
@@ -52,6 +53,27 @@ func (pfs *ParameterFlagSet) MarshalJSON() ([]byte, error) {
 	return json.Marshal(ps)
 }
 
+// Clone clones the parameter flagset.
+//
+// This function is usually called when cloning scenarios where we also want to clone existing
+// parameter values.
+func (pfs *ParameterFlagSet) Clone() *ParameterFlagSet {
+	// XXX: pfs.name and pfs.errorHandling are private. Use reflection.
+	pfsName := reflect.ValueOf(*pfs).FieldByName("name").String()
+	pfsEh := flag.ErrorHandling(reflect.ValueOf(*pfs).FieldByName("errorHandling").Int())
+	newPfs := &ParameterFlagSet{FlagSet: *flag.NewFlagSet(pfsName, pfsEh)}
+	pfs.VisitAll(func(f *flag.Flag) {
+		// Clone concrete members of Flag.
+		fl := *f
+		// Clone Value interface.
+		val := reflect.New(reflect.TypeOf(fl.Value).Elem())
+		fl.Value = val.Interface().(flag.Value)
+		newPfs.AddFlag(&fl)
+	})
+
+	return newPfs
+}
+
 // Env is a (nested) test environment.
 type Env struct {
 	name string

go/oasis-test-runner/scenario/e2e/e2e.go

@@ -26,7 +26,7 @@ type e2eImpl struct {
 	net    *oasis.Network
 	name   string
 	logger *logging.Logger
-	flags  *flag.FlagSet
+	flags  *env.ParameterFlagSet
 }
 
 func newE2eImpl(name string) *e2eImpl {
@@ -39,36 +39,27 @@ func newE2eImpl(name string) *e2eImpl {
 	sc := &e2eImpl{
 		name:   fullName,
 		logger: logging.GetLogger("scenario/" + fullName),
-		flags:  newE2eFlags(fullName),
+		flags:  &env.ParameterFlagSet{FlagSet: *flag.NewFlagSet(name, flag.ContinueOnError)},
 	}
+	sc.flags.String(cfgNodeBinary, "oasis-node", "path to the node binary")
 
 	return sc
 }
 
-func newE2eFlags(name string) *flag.FlagSet {
-	fs := flag.NewFlagSet(name, flag.ContinueOnError)
-	fs.String(cfgNodeBinary, "oasis-node", "path to the node binary")
-
-	return fs
-}
-
 func (sc *e2eImpl) Clone() e2eImpl {
-	newSc := &e2eImpl{
+	return e2eImpl{
 		net:    sc.net,
 		name:   sc.name,
 		logger: sc.logger,
-		flags:  newE2eFlags(sc.name),
+		flags:  sc.flags.Clone(),
 	}
-	scenario.CopyFlagValues(newSc.flags, sc.flags)
-
-	return *newSc
 }
 
 func (sc *e2eImpl) Name() string {
 	return sc.name
 }
 
-func (sc *e2eImpl) Parameters() *flag.FlagSet {
+func (sc *e2eImpl) Parameters() *env.ParameterFlagSet {
 	return sc.flags
 }
 

go/oasis-test-runner/scenario/e2e/multiple_runtimes.go

@@ -5,8 +5,6 @@ import (
 	"fmt"
 	"time"
 
-	flag "github.com/spf13/pflag"
-
 	"github.com/oasislabs/oasis-core/go/common"
 	epochtime "github.com/oasislabs/oasis-core/go/epochtime/api"
 	"github.com/oasislabs/oasis-core/go/oasis-test-runner/env"
@@ -28,11 +26,14 @@ const (
 
 var (
 	// MultipleRuntimes is a scenario which tests running multiple runtimes on one node.
-	MultipleRuntimes scenario.Scenario = func() scenario.Scenario {
+	MultipleRuntimes = func() scenario.Scenario {
 		sc := &multipleRuntimesImpl{
 			runtimeImpl: *newRuntimeImpl("multiple-runtimes", "simple-keyvalue-client", nil),
 		}
-		sc.flags = newE2eRuntimeMultipleRuntimesFlags(sc.name)
+		sc.flags.Int(cfgNumComputeRuntimes, 2, "number of compute runtimes per worker")
+		sc.flags.Int(cfgNumComputeRuntimeTxns, 2, "number of transactions to perform")
+		sc.flags.Int(cfgNumComputeWorkers, 2, "number of workers to initiate")
+		sc.flags.Int(cfgExecutorGroupSize, 2, "number of executor workers in committee")
 
 		return sc
 	}()
@@ -42,25 +43,10 @@ type multipleRuntimesImpl struct {
 	runtimeImpl
 }
 
-func newE2eRuntimeMultipleRuntimesFlags(name string) *flag.FlagSet {
-	// Inherit E2ERuntime flags.
-	fs := newE2eRuntimeFlags(name)
-	fs.Int(cfgNumComputeRuntimes, 2, "number of compute runtimes per worker")
-	fs.Int(cfgNumComputeRuntimeTxns, 2, "number of transactions to perform")
-	fs.Int(cfgNumComputeWorkers, 2, "number of workers to initiate")
-	fs.Int(cfgExecutorGroupSize, 2, "number of executor workers in committee")
-
-	return fs
-}
-
 func (mr *multipleRuntimesImpl) Clone() scenario.Scenario {
-	newSc := &multipleRuntimesImpl{
+	return &multipleRuntimesImpl{
 		runtimeImpl: *mr.runtimeImpl.Clone().(*runtimeImpl),
 	}
-	newSc.flags = newE2eRuntimeMultipleRuntimesFlags(mr.name)
-	scenario.CopyFlagValues(newSc.flags, mr.flags)
-
-	return newSc
 }
 
 func (mr *multipleRuntimesImpl) Fixture() (*oasis.NetworkFixture, error) {
@@ -90,14 +76,8 @@ func (mr *multipleRuntimesImpl) Fixture() (*oasis.NetworkFixture, error) {
 	f.Network.EpochtimeMock = true
 
 	// Add some more consecutive runtime IDs with the same binary.
-	numComputeRuntimes, err := mr.flags.GetInt(cfgNumComputeRuntimes)
-	if err != nil {
-		return nil, err
-	}
-	executorGroupSize, err := mr.flags.GetInt(cfgExecutorGroupSize)
-	if err != nil {
-		return nil, err
-	}
+	numComputeRuntimes, _ := mr.flags.GetInt(cfgNumComputeRuntimes)
+	executorGroupSize, _ := mr.flags.GetInt(cfgExecutorGroupSize)
 	for i := 1; i <= numComputeRuntimes; i++ {
 		// Increase LSB by 1.
 		id[len(id)-1]++
@@ -141,10 +121,7 @@ func (mr *multipleRuntimesImpl) Fixture() (*oasis.NetworkFixture, error) {
 	}
 
 	// Use numComputeWorkers compute worker fixtures.
-	numComputeWorkers, err := mr.flags.GetInt(cfgNumComputeWorkers)
-	if err != nil {
-		return nil, err
-	}
+	numComputeWorkers, _ := mr.flags.GetInt(cfgNumComputeWorkers)
 	f.ComputeWorkers = []oasis.ComputeWorkerFixture{}
 	for i := 0; i < numComputeWorkers; i++ {
 		f.ComputeWorkers = append(f.ComputeWorkers, oasis.ComputeWorkerFixture{Entity: 1})
@@ -167,10 +144,7 @@ func (mr *multipleRuntimesImpl) Run(childEnv *env.Env) error {
 
 	// Submit transactions.
 	epoch := epochtime.EpochTime(3)
-	numComputeRuntimeTxns, err := mr.flags.GetInt(cfgNumComputeRuntimeTxns)
-	if err != nil {
-		return err
-	}
+	numComputeRuntimeTxns, _ := mr.flags.GetInt(cfgNumComputeRuntimeTxns)
 	for _, r := range mr.net.Runtimes() {
 		rt := r.ToRuntimeDescriptor()
 		if rt.Kind == registry.KindCompute {

go/oasis-test-runner/scenario/e2e/runtime.go

@@ -10,8 +10,6 @@ import (
 	"strings"
 	"time"
 
-	flag "github.com/spf13/pflag"
-
 	"github.com/oasislabs/oasis-core/go/common"
 	"github.com/oasislabs/oasis-core/go/common/cbor"
 	"github.com/oasislabs/oasis-core/go/common/node"
@@ -82,33 +80,21 @@ func newRuntimeImpl(name, clientBinary string, clientArgs []string) *runtimeImpl
 		clientBinary: clientBinary,
 		clientArgs:   clientArgs,
 	}
-	sc.flags = newE2eRuntimeFlags(fullName)
+	sc.flags.String(cfgClientBinaryDir, "", "path to the client binaries directory")
+	sc.flags.String(cfgRuntimeBinaryDir, "", "path to the runtime binaries directory")
+	sc.flags.String(cfgRuntimeLoader, "oasis-core-runtime-loader", "path to the runtime loader")
+	sc.flags.String(cfgTEEHardware, "", "TEE hardware to use")
+	sc.flags.Bool(cfgIasMock, true, "if mock IAS service should be used")
 
 	return sc
 }
 
-func newE2eRuntimeFlags(name string) *flag.FlagSet {
-	// Inherit E2E flags.
-	fs := newE2eFlags(name)
-	fs.String(cfgClientBinaryDir, "", "path to the client binaries directory")
-	fs.String(cfgRuntimeBinaryDir, "", "path to the runtime binaries directory")
-	fs.String(cfgRuntimeLoader, "oasis-core-runtime-loader", "path to the runtime loader")
-	fs.String(cfgTEEHardware, "", "TEE hardware to use")
-	fs.Bool(cfgIasMock, true, "if mock IAS service should be used")
-
-	return fs
-}
-
 func (sc *runtimeImpl) Clone() scenario.Scenario {
-	newSc := &runtimeImpl{
+	return &runtimeImpl{
 		e2eImpl:      sc.e2eImpl.Clone(),
 		clientBinary: sc.clientBinary,
 		clientArgs:   sc.clientArgs,
 	}
-	newSc.flags = newE2eRuntimeFlags(sc.name)
-	scenario.CopyFlagValues(newSc.flags, sc.flags)
-
-	return newSc
 }
 
 func (sc *runtimeImpl) PreInit(childEnv *env.Env) error {

go/oasis-test-runner/scenario/remotesigner/remotesigner.go

@@ -12,6 +12,7 @@ import (
 )
 
 const (
+	// cfgServerBinary is path to remote-signer server executable.
 	cfgServerBinary = "binary"
 )
 
@@ -24,8 +25,7 @@ var (
 type remoteSignerImpl struct {
 	name   string
 	logger *logging.Logger
-
-	flags *flag.FlagSet
+	flags  *env.ParameterFlagSet
 }
 
 func newRemoteSignerImpl(name string) *remoteSignerImpl {
@@ -37,38 +37,27 @@ func newRemoteSignerImpl(name string) *remoteSignerImpl {
 
 	sc := &remoteSignerImpl{
 		name:   fullName,
-		logger: logging.GetLogger("scenario/remote-signer/" + name),
-		flags:  newRemoteSignerFlags(fullName),
+		logger: logging.GetLogger("scenario/" + fullName),
+		flags:  &env.ParameterFlagSet{FlagSet: *flag.NewFlagSet(fullName, flag.ContinueOnError)},
 	}
+	sc.flags.String(cfgServerBinary, "oasis-remote-signer", "runtime binary")
 
 	return sc
 }
 
-func newRemoteSignerFlags(name string) *flag.FlagSet {
-	fs := flag.NewFlagSet(name, flag.ContinueOnError)
-
-	// path to remote-signer server executable.
-	fs.String(cfgServerBinary, "oasis-remote-signer", "runtime binary")
-
-	return fs
-}
-
 func (sc *remoteSignerImpl) Clone() remoteSignerImpl {
-	newSc := remoteSignerImpl{
+	return remoteSignerImpl{
 		name:   sc.name,
 		logger: sc.logger,
-		flags:  newRemoteSignerFlags(sc.name),
+		flags:  sc.flags.Clone(),
 	}
-	scenario.CopyFlagValues(newSc.flags, sc.flags)
-
-	return newSc
 }
 
 func (sc *remoteSignerImpl) Name() string {
 	return sc.name
 }
 
-func (sc *remoteSignerImpl) Parameters() *flag.FlagSet {
+func (sc *remoteSignerImpl) Parameters() *env.ParameterFlagSet {
 	return sc.flags
 }
 

go/oasis-test-runner/scenario/scenario.go

@@ -22,7 +22,7 @@ type Scenario interface {
 	Name() string
 
 	// Parameters returns the settable test parameters.
-	Parameters() *flag.FlagSet
+	Parameters() *env.ParameterFlagSet
 
 	// PreInit performs initial scenario initialization. It is guaranteed to be called before
 	// a new fixture is initialized in Fixture.