Skip to content

Commit

Permalink
[FIX] normalized use of stdout/stderr. Reports on non-output commands…
Browse files Browse the repository at this point in the history 
… will now go to stdout if there is no error otherwise to stderr. Fixed commands to use fmt.Fprintf/variants to the right stream.

Fixes #580

[CI] Refactored test cases to modernize the use of `ExecuteCmd` to not use Pipes and simply provide io.Writer for test output collection. This solves spurious test failures in Windows

Signed-off-by: Alberto Ricart <[email protected]>
  • Loading branch information
@aricart
aricart committed Jan 6, 2025
1 parent 0c8fabe commit a27e04a
Show file tree
Hide file tree
Showing 90 changed files with 1,830 additions and 1,820 deletions.
17 changes: 16 additions & 1 deletion cmd/accountusercontextparams_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
/*
* Copyright 2018-2025 The NATS Authors
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package cmd

import (
Expand Down Expand Up @@ -59,7 +74,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
18 changes: 15 additions & 3 deletions cmd/action.go
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 The NATS Authors
* Copyright 2018-2025 The NATS Authors
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand Down 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
14 changes: 7 additions & 7 deletions cmd/action_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018 The NATS Authors
* Copyright 2018-2025 The NATS Authors
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand Down 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")
}
45 changes: 22 additions & 23 deletions cmd/addaccount_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018-2022 The NATS Authors
* Copyright 2018-2025 The NATS Authors
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
Expand All @@ -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 a27e04a

Please sign in to comment.