From 2b8d28fc11a9681c1bcffcdf536f3697da312fe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matev=C5=BE=20Jekovec?= Date: Wed, 27 May 2020 17:04:26 +0200 Subject: [PATCH] oasis-test-runner: Implement ParameterFlagSet.Clone() --- .changelog/2897.internal.md | 7 +-- go/oasis-test-runner/cmd/root.go | 4 +- go/oasis-test-runner/env/env.go | 22 +++++++++ go/oasis-test-runner/scenario/e2e/e2e.go | 21 +++------ .../scenario/e2e/multiple_runtimes.go | 46 ++++--------------- go/oasis-test-runner/scenario/e2e/runtime.go | 26 +++-------- .../scenario/remotesigner/remotesigner.go | 27 ++++------- go/oasis-test-runner/scenario/scenario.go | 2 +- 8 files changed, 59 insertions(+), 96 deletions(-) diff --git a/.changelog/2897.internal.md b/.changelog/2897.internal.md index f02b378ac8e..93beaa68e61 100644 --- a/.changelog/2897.internal.md +++ b/.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. diff --git a/go/oasis-test-runner/cmd/root.go b/go/oasis-test-runner/cmd/root.go index 75c06b15ecf..288ac24ecc6 100644 --- a/go/oasis-test-runner/cmd/root.go +++ b/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 { diff --git a/go/oasis-test-runner/env/env.go b/go/oasis-test-runner/env/env.go index 675841e36da..4ee41e08fea 100644 --- a/go/oasis-test-runner/env/env.go +++ b/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 diff --git a/go/oasis-test-runner/scenario/e2e/e2e.go b/go/oasis-test-runner/scenario/e2e/e2e.go index e6b813cc46a..4dd6018ef8a 100644 --- a/go/oasis-test-runner/scenario/e2e/e2e.go +++ b/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 } diff --git a/go/oasis-test-runner/scenario/e2e/multiple_runtimes.go b/go/oasis-test-runner/scenario/e2e/multiple_runtimes.go index 39b92f9be9c..af0eb0ee071 100644 --- a/go/oasis-test-runner/scenario/e2e/multiple_runtimes.go +++ b/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 { diff --git a/go/oasis-test-runner/scenario/e2e/runtime.go b/go/oasis-test-runner/scenario/e2e/runtime.go index a86d6892c15..507f14be7de 100644 --- a/go/oasis-test-runner/scenario/e2e/runtime.go +++ b/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 { diff --git a/go/oasis-test-runner/scenario/remotesigner/remotesigner.go b/go/oasis-test-runner/scenario/remotesigner/remotesigner.go index 001aa6f54c9..408af1bbf74 100644 --- a/go/oasis-test-runner/scenario/remotesigner/remotesigner.go +++ b/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 } diff --git a/go/oasis-test-runner/scenario/scenario.go b/go/oasis-test-runner/scenario/scenario.go index 7d1d906a72f..c2111aef836 100644 --- a/go/oasis-test-runner/scenario/scenario.go +++ b/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.