Skip to content

Commit

Permalink
Refactor test command execution to improve readability.
Browse files Browse the repository at this point in the history
Updated command invocations to use consistent argument formatting with slices. This change removes redundant values from returned results, simplifies error handling, and improves code clarity across tests.

The output stream for messages is now more deterministic.

Signed-off-by: Alberto Ricart <alberto@synadia.com>
  • Loading branch information
aricart committed Jan 6, 2025
1 parent e838251 commit 161e433
Show file tree
Hide file tree
Showing 89 changed files with 1,552 additions and 1,705 deletions.
2 changes: 1 addition & 1 deletion cmd/accountusercontextparams_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func Test_AccountUserContextParams(t *testing.T) {
// A, a
inputs := []interface{}{0, 0}

_, _, err := ExecuteInteractiveCmd(cmd, inputs)
_, err := ExecuteInteractiveCmd(cmd, inputs)
require.NoError(t, err)
require.Equal(t, "A", params.AccountContextParams.Name)
require.Equal(t, "a", params.UserContextParams.Name)
Expand Down
16 changes: 14 additions & 2 deletions cmd/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,27 @@ func run(ctx ActionCtx, action interface{}) error {

rs, err := e.Run(ctx)
if rs != nil {
ctx.CurrentCmd().Println(rs.Message())
// if we have an error or the reports has an error, send to stderr
// so they can see it
stream := ctx.CurrentCmd().OutOrStdout()
if err != nil {
stream = ctx.CurrentCmd().ErrOrStderr()
} else if store.IsReport(rs) {
r := store.ToReport(rs)
if r.HasErrors() {
stream = ctx.CurrentCmd().ErrOrStderr()
}
}

_, _ = fmt.Fprintln(stream, rs.Message())
sum, ok := rs.(store.Summarizer)
if ok {
m, err := sum.Summary()
if err != nil {
return err
}
if m != "" {
ctx.CurrentCmd().Println(strings.TrimSuffix(m, "\n"))
_, _ = fmt.Fprintln(stream, strings.TrimSuffix(m, "\n"))
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions cmd/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestActionContextSet(t *testing.T) {
},
}

_, _, err := ExecuteCmd(cmd, "hello", "world")
_, err := ExecuteCmd(cmd, "hello", "world")
require.NoError(t, err)
}

Expand Down Expand Up @@ -88,7 +88,7 @@ func TestActionInteractive(t *testing.T) {
return nil
},
}
_, _, err := ExecuteInteractiveCmd(cmd, []interface{}{})
_, err := ExecuteInteractiveCmd(cmd, []interface{}{})
require.NoError(t, err)
require.Equal(t, 2, count)
}
Expand Down Expand Up @@ -118,7 +118,7 @@ func TestActionNothingToDoEmpty(t *testing.T) {
}

cmd.Flags().StringVarP(&v, "name", "n", "", "account name")
_, _, err := ExecuteCmd(cmd)
_, err := ExecuteCmd(cmd)
require.NoError(t, err)
require.True(t, nothingToDo)
}
Expand Down Expand Up @@ -148,7 +148,7 @@ func TestActionNothingToDoSet(t *testing.T) {
}

cmd.Flags().StringVarP(&v, "name", "n", "", "account name")
_, _, err := ExecuteCmd(cmd, "--name", "a")
_, err := ExecuteCmd(cmd, "--name", "a")
require.NoError(t, err)
require.False(t, nothingToDo)
}
Expand Down Expand Up @@ -226,7 +226,7 @@ func TestActionMessage(t *testing.T) {
},
}

out, _, err := ExecuteCmd(cmd)
out, err := ExecuteCmd(cmd)
require.NoError(t, err)
require.Contains(t, "This is a test message", out)
require.Contains(t, out.Out, "this is a test message")
}
43 changes: 21 additions & 22 deletions cmd/addaccount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ import (

"github.com/nats-io/jwt/v2"
"github.com/nats-io/nkeys"
"github.com/stretchr/testify/require"

"github.com/nats-io/nsc/v2/cmd/store"
"github.com/stretchr/testify/require"
)

func Test_AddAccount(t *testing.T) {
Expand All @@ -41,11 +40,11 @@ func Test_AddAccount(t *testing.T) {

tests := CmdTests{
{CreateAddAccountCmd(), []string{"add", "account"}, nil, []string{"account name is required"}, true},
{CreateAddAccountCmd(), []string{"add", "account", "--name", "A"}, nil, []string{"generated and stored account key", "added account"}, false},
{CreateAddAccountCmd(), []string{"add", "account", "--name", "A"}, []string{"generated and stored account key", "added account"}, nil, false},
{CreateAddAccountCmd(), []string{"add", "account", "--name", "A"}, nil, []string{"the account \"A\" already exists"}, true},
{CreateAddAccountCmd(), []string{"add", "account", "--name", "B", "--public-key", bar}, nil, nil, false},
{CreateAddAccountCmd(), []string{"add", "account", "--name", "*"}, nil, []string{"generated and stored account key", "added account"}, false},
{CreateAddAccountCmd(), []string{"add", "account", "--name", "*"}, nil, []string{"generated and stored account key", "added account"}, false}, // should make a new name
{CreateAddAccountCmd(), []string{"add", "account", "--name", "*"}, []string{"generated and stored account key", "added account"}, nil, false},
{CreateAddAccountCmd(), []string{"add", "account", "--name", "*"}, []string{"generated and stored account key", "added account"}, nil, false}, // should make a new name
{CreateAddAccountCmd(), []string{"add", "account", "--name", "X", "--public-key", cpk}, nil, []string{"specified key is not a valid account nkey"}, true},
{CreateAddAccountCmd(), []string{"add", "account", "--name", "badexp", "--expiry", "30d"}, nil, nil, false},
}
Expand All @@ -56,7 +55,7 @@ func Test_AddAccount(t *testing.T) {
func Test_AddAccountNoStore(t *testing.T) {
// reset the store
require.NoError(t, ForceStoreRoot(t, ""))
_, _, err := ExecuteCmd(CreateAddAccountCmd())
_, err := ExecuteCmd(CreateAddAccountCmd())
require.NotNil(t, err)
require.Equal(t, "no stores available", err.Error())
}
Expand All @@ -65,7 +64,7 @@ func Test_AddAccountValidateOutput(t *testing.T) {
ts := NewTestStore(t, "test")
defer ts.Done(t)

_, _, err := ExecuteCmd(CreateAddAccountCmd(), "--name", "A", "--start", "2018-01-01", "--expiry", "2050-01-01")
_, err := ExecuteCmd(CreateAddAccountCmd(), "--name", "A", "--start", "2018-01-01", "--expiry", "2050-01-01")
require.NoError(t, err)
validateAddAccountClaims(t, ts)
ts.List(t)
Expand All @@ -79,7 +78,7 @@ func Test_AddAccountInteractive(t *testing.T) {

cmd := CreateAddAccountCmd()
HoistRootFlags(cmd)
_, _, err := ExecuteInteractiveCmd(cmd, inputs)
_, err := ExecuteInteractiveCmd(cmd, inputs)
require.NoError(t, err)
validateAddAccountClaims(t, ts)
}
Expand Down Expand Up @@ -124,7 +123,7 @@ func Test_AddAccountManagedStore(t *testing.T) {
ts := NewTestStoreWithOperatorJWT(t, string(m["operator"]))
defer ts.Done(t)

_, _, err := ExecuteCmd(CreateAddAccountCmd(), "--name", "A", "--start", "2018-01-01", "--expiry", "2050-01-01")
_, err := ExecuteCmd(CreateAddAccountCmd(), "--name", "A", "--start", "2018-01-01", "--expiry", "2050-01-01")
require.NoError(t, err)
}

Expand All @@ -140,13 +139,13 @@ func Test_AddAccountManagedStoreWithSigningKey(t *testing.T) {
token, err := oc.Encode(kp)
require.NoError(t, err)
tf := filepath.Join(ts.Dir, "O.jwt")
err = Write(tf, []byte(token))
err = WriteFile(tf, []byte(token))
require.NoError(t, err)
_, _, err = ExecuteCmd(createAddOperatorCmd(), "--url", tf)
_, err = ExecuteCmd(createAddOperatorCmd(), "--url", tf)
require.NoError(t, err)
// sign with the signing key
inputs := []interface{}{"A", true, "0", "0", 0, string(s1)}
_, _, err = ExecuteInteractiveCmd(HoistRootFlags(CreateAddAccountCmd()), inputs)
_, err = ExecuteInteractiveCmd(HoistRootFlags(CreateAddAccountCmd()), inputs)
require.NoError(t, err)
accJWT, err := os.ReadFile(filepath.Join(ts.StoreDir, "O", "accounts", "A", "A.jwt"))
require.NoError(t, err)
Expand All @@ -157,7 +156,7 @@ func Test_AddAccountManagedStoreWithSigningKey(t *testing.T) {
require.True(t, oc.DidSign(ac))
// sign with the account key
inputs = []interface{}{"B", true, "0", "0", 1, string(s1)}
_, _, err = ExecuteInteractiveCmd(HoistRootFlags(CreateAddAccountCmd()), inputs)
_, err = ExecuteInteractiveCmd(HoistRootFlags(CreateAddAccountCmd()), inputs)
require.NoError(t, err)
accJWT, err = os.ReadFile(filepath.Join(ts.StoreDir, "O", "accounts", "B", "B.jwt"))
require.NoError(t, err)
Expand All @@ -172,12 +171,12 @@ func Test_AddAccountInteractiveSigningKey(t *testing.T) {
defer ts.Done(t)

s1, pk1, _ := CreateOperatorKey(t)
_, _, err := ExecuteCmd(createEditOperatorCmd(), "--sk", pk1)
_, err := ExecuteCmd(createEditOperatorCmd(), "--sk", pk1)
require.NoError(t, err)

// sign with the custom key
inputs := []interface{}{"A", true, "0", "0", 1, string(s1)}
_, _, err = ExecuteInteractiveCmd(HoistRootFlags(CreateAddAccountCmd()), inputs)
_, err = ExecuteInteractiveCmd(HoistRootFlags(CreateAddAccountCmd()), inputs, []string{}...)
require.NoError(t, err)

d, err := ts.Store.Read(store.JwtName("O"))
Expand All @@ -196,7 +195,7 @@ func Test_AddAccountNameArg(t *testing.T) {
ts := NewTestStore(t, "O")
defer ts.Done(t)

_, _, err := ExecuteCmd(HoistRootFlags(CreateAddAccountCmd()), "A")
_, err := ExecuteCmd(HoistRootFlags(CreateAddAccountCmd()), "A")
require.NoError(t, err)

_, err = ts.Store.ReadAccountClaim("A")
Expand All @@ -214,7 +213,7 @@ func Test_AddAccountWithExistingKey(t *testing.T) {
pk, err := kp.PublicKey()
require.NoError(t, err)

_, _, err = ExecuteCmd(CreateAddAccountCmd(), "A", "--public-key", pk)
_, err = ExecuteCmd(CreateAddAccountCmd(), "A", "--public-key", pk)
require.NoError(t, err)
}

Expand All @@ -232,7 +231,7 @@ func Test_AddManagedAccountWithExistingKey(t *testing.T) {
pk, err := kp.PublicKey()
require.NoError(t, err)

_, _, err = ExecuteCmd(CreateAddAccountCmd(), "A", "--public-key", pk)
_, err = ExecuteCmd(CreateAddAccountCmd(), "A", "--public-key", pk)
require.NoError(t, err)

// inspect the pushed JWT before it was resigned
Expand All @@ -254,15 +253,15 @@ func Test_AddAccountWithSigningKeyOnly(t *testing.T) {
require.NoError(t, err)
require.True(t, ts.KeyStore.HasPrivateKey(pk))

_, _, err = ExecuteCmd(createEditOperatorCmd(), "--sk", pk)
_, err = ExecuteCmd(createEditOperatorCmd(), "--sk", pk)
require.NoError(t, err)
oc, err := ts.Store.ReadOperatorClaim()
require.NoError(t, err)
require.NotNil(t, oc)
require.NoError(t, ts.KeyStore.Remove(oc.Subject))
require.False(t, ts.KeyStore.HasPrivateKey(oc.Subject))

_, _, err = ExecuteCmd(CreateAddAccountCmd(), "--name", "A")
_, err = ExecuteCmd(CreateAddAccountCmd(), "--name", "A")
require.NoError(t, err)

_, err = ts.Store.ReadAccountClaim("A")
Expand All @@ -273,7 +272,7 @@ func Test_AddAccount_Pubs(t *testing.T) {
ts := NewTestStore(t, "edit user")
defer ts.Done(t)

_, _, err := ExecuteCmd(CreateAddAccountCmd(), "-n", "A", "--allow-pub", "a,b", "--allow-pubsub", "c", "--deny-pub", "foo", "--deny-pubsub", "bar")
_, err := ExecuteCmd(CreateAddAccountCmd(), "-n", "A", "--allow-pub", "a,b", "--allow-pubsub", "c", "--deny-pub", "foo", "--deny-pubsub", "bar")
require.NoError(t, err)

cc, err := ts.Store.ReadAccountClaim("A")
Expand All @@ -289,7 +288,7 @@ func Test_AddAccountBadName(t *testing.T) {
ts := NewTestStore(t, "O")
defer ts.Done(t)

_, _, err := ExecuteCmd(CreateAddAccountCmd(), "A/B")
_, err := ExecuteCmd(CreateAddAccountCmd(), "A/B")
require.Error(t, err)
require.Contains(t, err.Error(), "name cannot contain '/' or '\\'")
}
Loading

0 comments on commit 161e433

Please sign in to comment.