go/oasis-test-runner: List all available scenarios #3044
Conversation
tjanez
requested review from
kostko,
peterjgilbert,
pro-wh and
Yawning
as code owners
| @@ -75,7 +75,7 @@ func RootCmd() *cobra.Command { | |||
| // Execute spawns the main entry point after handing the config file. | |||
| func Execute() { | |||
| if err := rootCmd.Execute(); err != nil { | |||
| nodeCommon.EarlyLogAndExit(err) | |||
There was a problem hiding this comment.
The nodeCommon.EarlyLogAndExit() prints the error to stderr:
oasis-core/go/oasis-node/cmd/common/common.go
Lines 79 to 86 in ffa5454
which results in the same error message being outputted twice.
For example:
$ .buildkite/scripts/test_e2e.sh --test storage-sync
+ WORKDIR=/home/tadej/Oasis/oasis-core
+ runtime_target=default
+ [[ '' == \i\n\t\e\l\-\s\g\x ]]
+ [[ '' != '' ]]
+ node_binary=/home/tadej/Oasis/oasis-core/go/oasis-node/oasis-node
+ test_runner_binary=/home/tadej/Oasis/oasis-core/go/oasis-test-runner/oasis-test-runner
+ [[ '' != '' ]]
+ ias_mock=true
+ set +x
+ /home/tadej/Oasis/oasis-core/go/oasis-test-runner/oasis-test-runner --basedir.no_cleanup --e2e.node.binary /home/tadej/Oasis/oasis-core/go/oasis-node/oasis-node --e2e/runtime.client.binary_dir /home/tadej/Oasis/oasis-core/target/default/debug --e2e/runtime.runtime.binary_dir /home/tadej/Oasis/oasis-core/target/default/debug --e2e/runtime.runtime.loader /home/tadej/Oasis/oasis-core/target/default/debug/oasis-core-runtime-loader --e2e/runtime.tee_hardware '' --e2e/runtime.ias.mock=true --remote-signer.binary /home/tadej/Oasis/oasis-core/go/oasis-remote-signer/oasis-remote-signer --log.level info --test storage-sync
level=error module=test-runner caller=root.go:279 ts=2020-06-24T08:24:40.945179021Z msg="unknown scenario" scenario=storage-sync
Error: root: unknown scenario: storage-sync
Available scenarios:
e2e/identity-cli
e2e/runtime/byzantine/executor-honest
e2e/runtime/byzantine/executor-straggler
e2e/runtime/byzantine/executor-wrong
e2e/runtime/byzantine/merge-honest
e2e/runtime/byzantine/merge-straggler
e2e/runtime/byzantine/merge-wrong
e2e/runtime/debond
e2e/runtime/dump-restore
e2e/runtime/early-query
e2e/runtime/gas-fees/runtimes
e2e/runtime/gas-fees/staking
e2e/runtime/gas-fees/staking-dump-restore
e2e/runtime/halt-restore
e2e/runtime/keymanager-replication
e2e/runtime/keymanager-restart
e2e/runtime/keymanager-upgrade
e2e/runtime/late-start
e2e/runtime/multiple-runtimes
e2e/runtime/node-shutdown
e2e/runtime/node-upgrade
e2e/runtime/node-upgrade-cancel
e2e/runtime/registry-cli
e2e/runtime/runtime
e2e/runtime/runtime-dynamic
e2e/runtime/runtime-encryption
e2e/runtime/runtime-prune
e2e/runtime/sentry
e2e/runtime/sentry-encryption
e2e/runtime/stake-cli
e2e/runtime/storage-sync
e2e/runtime/txsource-multi
e2e/runtime/txsource-multi-short
remote-signer/basic
root: unknown scenario: storage-sync
Available scenarios:
e2e/identity-cli
e2e/runtime/byzantine/executor-honest
e2e/runtime/byzantine/executor-straggler
e2e/runtime/byzantine/executor-wrong
e2e/runtime/byzantine/merge-honest
e2e/runtime/byzantine/merge-straggler
e2e/runtime/byzantine/merge-wrong
e2e/runtime/debond
e2e/runtime/dump-restore
e2e/runtime/early-query
e2e/runtime/gas-fees/runtimes
e2e/runtime/gas-fees/staking
e2e/runtime/gas-fees/staking-dump-restore
e2e/runtime/halt-restore
e2e/runtime/keymanager-replication
e2e/runtime/keymanager-restart
e2e/runtime/keymanager-upgrade
e2e/runtime/late-start
e2e/runtime/multiple-runtimes
e2e/runtime/node-shutdown
e2e/runtime/node-upgrade
e2e/runtime/node-upgrade-cancel
e2e/runtime/registry-cli
e2e/runtime/runtime
e2e/runtime/runtime-dynamic
e2e/runtime/runtime-encryption
e2e/runtime/runtime-prune
e2e/runtime/sentry
e2e/runtime/sentry-encryption
e2e/runtime/stake-cli
e2e/runtime/storage-sync
e2e/runtime/txsource-multi
e2e/runtime/txsource-multi-short
remote-signer/basic
There was a problem hiding this comment.
Who prints the first error message, does cobra do that internally?
There was a problem hiding this comment.
Yes, Cobra does that in ExecuteC():
// If root command has SilentErrors flagged,
// all subcommands should respect it
if !cmd.SilenceErrors && !c.SilenceErrors {
c.Println("Error:", err.Error())
}There was a problem hiding this comment.
I like our helper better as it prints to stderr while cobra seems to print to stdout? Could we instead set SilenceErrors (maybe this needs to be done in other places as well as we use EarlyLogAndExit everywhere)?
There was a problem hiding this comment.
I like our helper better as it prints to stderr while cobra seems to print to stdout?
Cobra's Command.Println() defaults to printing to stderr.
There was a problem hiding this comment.
Why do we have this EarlyLogAndExit at all then?
There was a problem hiding this comment.
I guess it is still useful for cases where we want to exit and print an error before we execute Cobra's Execute().
I found two such cases in go/oasis-test-runner:
oasis-core/go/oasis-test-runner/cmd/root.go
Line 550 in f5f906b
I've removed other uses in go/oasis-test-runner.
For other parts of the code base, I can file an issue to systematically check that.
There was a problem hiding this comment.
For other parts of the code base, I can file an issue to systematically check that.
Done: #3048.
Codecov Report
@@ Coverage Diff @@
## master #3044 +/- ##
==========================================
- Coverage 68.90% 68.52% -0.39%
==========================================
Files 370 370
Lines 36401 36401
==========================================
- Hits 25083 24943 -140
- Misses 8143 8276 +133
- Partials 3175 3182 +7
Continue to review full report at Codecov.
|
tjanez
force-pushed
the
tjanez/e2e-scenarios
branch
from
546b6c9 to
6786ad0
Compare
List all available scenarios if oasis-test-runner command is invoked with an unknown scenario.
Previously, Execute() called nodeCommon.EarlyLogAndExit() if there was an error running rootCmd.Execute() which in turn printed the error to stderr. The error message is printed to stdout via github.com/spf13/cobra's ExecuteC() already, so this resulted in the same error message being printed twice.
tjanez
force-pushed
the
tjanez/e2e-scenarios
branch
from
6786ad0 to
83a464a
Compare
Instead, return an ordinary error which is then propagated and printed to logs accordingly.
tjanez
force-pushed
the
tjanez/e2e-scenarios
branch
from
83a464a to
669b335
Compare
List all available scenarios if command was invoked with a particular scenario that was not found.
For example, running
.buildkite/scripts/test_e2e.sh --test storage-syncwherestorage-syncscenario doesn't exist, now returns: