Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

args_test: add helper functions #1426

Merged
merged 3 commits into from
Nov 16, 2021
Merged

args_test: add helper functions #1426

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0fd7782
args_test: add helper function expectSuccess
umarcor Mar 18, 2019
9895902
args_test: add helper function getCommand
umarcor Mar 18, 2019
4120f6e
args_test: add additional helper functions
umarcor Mar 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
248 changes: 103 additions & 145 deletions args_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,229 +5,187 @@ import (
"testing"
)

func TestNoArgs(t *testing.T) {
c := &Command{Use: "c", Args: NoArgs, Run: emptyRun}
func getCommand(args PositionalArgs, withValid bool) *Command {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm not sure if I'm a huge fan of using helper methods to abstract away creating a command struct. What benefit do we get adding this complexity when using literal structs inline gets us the same thing?

Copy link
Contributor Author

@umarcor umarcor Nov 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two main reasons: verbosity and human abstraction.

  • get_command is used 16 times, and it contains 9 lines. That's 16*7~=112 lines of duplicated code. It's 40% of the file.
  • Using this abstraction makes it easier to understand that two orthogonal features are tested: different types of argument constraints and usage of ValidArgs. The orthogonality of those features is fixed in args_test refactorization after generalizing ValidArgs #841. In fact, this PR is a subset of that one.

As you can see in https://github.com/spf13/cobra/pull/841/files#diff-a48034865676903aed3da7be846f56efab79615c941b4b0089d0eb17584d391a, the complexity of this file would explode in #841 if we didn't have this (and other) helper function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also these two dialogues with @jharshman where he did some suggestions about renaming tests and using sub-tests, which I applied:

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough! I say let's get it in and if we need to refactor, we can go from there

c := &Command{
Use: "c",
Args: args,
Run: emptyRun,
}
if withValid {
c.ValidArgs = []string{"one", "two", "three"}
}
return c
}

output, err := executeCommand(c)
func expectSuccess(output string, err error, t *testing.T) {
if output != "" {
t.Errorf("Unexpected string: %v", output)
t.Errorf("Unexpected output: %v", output)
}
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
}

func TestNoArgsWithArgs(t *testing.T) {
c := &Command{Use: "c", Args: NoArgs, Run: emptyRun}

_, err := executeCommand(c, "illegal")
func validWithInvalidArgs(err error, t *testing.T) {
if err == nil {
t.Fatal("Expected an error")
}
got := err.Error()
expected := `invalid argument "a" for "c"`
if got != expected {
t.Errorf("Expected: %q, got: %q", expected, got)
}
}

func noArgsWithArgs(err error, t *testing.T) {
if err == nil {
t.Fatal("Expected an error")
}
got := err.Error()
expected := `unknown command "illegal" for "c"`
if got != expected {
t.Errorf("Expected: %q, got: %q", expected, got)
}
}

func TestOnlyValidArgs(t *testing.T) {
c := &Command{
Use: "c",
Args: OnlyValidArgs,
ValidArgs: []string{"one", "two"},
Run: emptyRun,
func minimumNArgsWithLessArgs(err error, t *testing.T) {
if err == nil {
t.Fatal("Expected an error")
}
got := err.Error()
expected := "requires at least 2 arg(s), only received 1"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
}
}

output, err := executeCommand(c, "one", "two")
if output != "" {
t.Errorf("Unexpected output: %v", output)
func maximumNArgsWithMoreArgs(err error, t *testing.T) {
if err == nil {
t.Fatal("Expected an error")
}
if err != nil {
t.Fatalf("Unexpected error: %v", err)
got := err.Error()
expected := "accepts at most 2 arg(s), received 3"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
}
}

func TestOnlyValidArgsWithInvalidArgs(t *testing.T) {
c := &Command{
Use: "c",
Args: OnlyValidArgs,
ValidArgs: []string{"one", "two"},
Run: emptyRun,
func exactArgsWithInvalidCount(err error, t *testing.T) {
if err == nil {
t.Fatal("Expected an error")
}
got := err.Error()
expected := "accepts 2 arg(s), received 3"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
}
}

_, err := executeCommand(c, "three")
func rangeArgsWithInvalidCount(err error, t *testing.T) {
if err == nil {
t.Fatal("Expected an error")
}

got := err.Error()
expected := `invalid argument "three" for "c"`
expected := "accepts between 2 and 4 arg(s), received 1"
if got != expected {
t.Errorf("Expected: %q, got: %q", expected, got)
t.Fatalf("Expected %q, got %q", expected, got)
}
}

func TestNoArgs(t *testing.T) {
c := getCommand(NoArgs, false)
output, err := executeCommand(c)
expectSuccess(output, err, t)
}

func TestNoArgsWithArgs(t *testing.T) {
c := getCommand(NoArgs, false)
_, err := executeCommand(c, "illegal")
noArgsWithArgs(err, t)
}

func TestOnlyValidArgs(t *testing.T) {
c := getCommand(OnlyValidArgs, true)
output, err := executeCommand(c, "one", "two")
expectSuccess(output, err, t)
}

func TestOnlyValidArgsWithInvalidArgs(t *testing.T) {
c := getCommand(OnlyValidArgs, true)
_, err := executeCommand(c, "a")
validWithInvalidArgs(err, t)
}

func TestArbitraryArgs(t *testing.T) {
c := &Command{Use: "c", Args: ArbitraryArgs, Run: emptyRun}
c := getCommand(ArbitraryArgs, false)
output, err := executeCommand(c, "a", "b")
if output != "" {
t.Errorf("Unexpected output: %v", output)
}
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
expectSuccess(output, err, t)
}

func TestMinimumNArgs(t *testing.T) {
c := &Command{Use: "c", Args: MinimumNArgs(2), Run: emptyRun}
c := getCommand(MinimumNArgs(2), false)
output, err := executeCommand(c, "a", "b", "c")
if output != "" {
t.Errorf("Unexpected output: %v", output)
}
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
expectSuccess(output, err, t)
}

func TestMinimumNArgsWithLessArgs(t *testing.T) {
c := &Command{Use: "c", Args: MinimumNArgs(2), Run: emptyRun}
c := getCommand(MinimumNArgs(2), false)
_, err := executeCommand(c, "a")

if err == nil {
t.Fatal("Expected an error")
}

got := err.Error()
expected := "requires at least 2 arg(s), only received 1"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
}
minimumNArgsWithLessArgs(err, t)
}

func TestMaximumNArgs(t *testing.T) {
c := &Command{Use: "c", Args: MaximumNArgs(3), Run: emptyRun}
c := getCommand(MaximumNArgs(3), false)
output, err := executeCommand(c, "a", "b")
if output != "" {
t.Errorf("Unexpected output: %v", output)
}
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
expectSuccess(output, err, t)
}

func TestMaximumNArgsWithMoreArgs(t *testing.T) {
c := &Command{Use: "c", Args: MaximumNArgs(2), Run: emptyRun}
c := getCommand(MaximumNArgs(2), false)
_, err := executeCommand(c, "a", "b", "c")

if err == nil {
t.Fatal("Expected an error")
}

got := err.Error()
expected := "accepts at most 2 arg(s), received 3"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
}
maximumNArgsWithMoreArgs(err, t)
}

func TestExactArgs(t *testing.T) {
c := &Command{Use: "c", Args: ExactArgs(3), Run: emptyRun}
c := getCommand(ExactArgs(3), false)
output, err := executeCommand(c, "a", "b", "c")
if output != "" {
t.Errorf("Unexpected output: %v", output)
}
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
expectSuccess(output, err, t)
}

func TestExactArgsWithInvalidCount(t *testing.T) {
c := &Command{Use: "c", Args: ExactArgs(2), Run: emptyRun}
c := getCommand(ExactArgs(2), false)
_, err := executeCommand(c, "a", "b", "c")

if err == nil {
t.Fatal("Expected an error")
}

got := err.Error()
expected := "accepts 2 arg(s), received 3"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
}
exactArgsWithInvalidCount(err, t)
}

func TestExactValidArgs(t *testing.T) {
c := &Command{Use: "c", Args: ExactValidArgs(3), ValidArgs: []string{"a", "b", "c"}, Run: emptyRun}
output, err := executeCommand(c, "a", "b", "c")
if output != "" {
t.Errorf("Unexpected output: %v", output)
}
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
c := getCommand(ExactValidArgs(3), true)
output, err := executeCommand(c, "three", "one", "two")
expectSuccess(output, err, t)
}

func TestExactValidArgsWithInvalidCount(t *testing.T) {
c := &Command{Use: "c", Args: ExactValidArgs(2), Run: emptyRun}
_, err := executeCommand(c, "a", "b", "c")

if err == nil {
t.Fatal("Expected an error")
}

got := err.Error()
expected := "accepts 2 arg(s), received 3"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
}
c := getCommand(ExactValidArgs(2), false)
_, err := executeCommand(c, "three", "one", "two")
exactArgsWithInvalidCount(err, t)
}

func TestExactValidArgsWithInvalidArgs(t *testing.T) {
c := &Command{
Use: "c",
Args: ExactValidArgs(1),
ValidArgs: []string{"one", "two"},
Run: emptyRun,
}

_, err := executeCommand(c, "three")
if err == nil {
t.Fatal("Expected an error")
}

got := err.Error()
expected := `invalid argument "three" for "c"`
if got != expected {
t.Errorf("Expected: %q, got: %q", expected, got)
}
c := getCommand(ExactValidArgs(3), true)
_, err := executeCommand(c, "three", "a", "two")
validWithInvalidArgs(err, t)
}

func TestRangeArgs(t *testing.T) {
c := &Command{Use: "c", Args: RangeArgs(2, 4), Run: emptyRun}
c := getCommand(RangeArgs(2, 4), false)
output, err := executeCommand(c, "a", "b", "c")
if output != "" {
t.Errorf("Unexpected output: %v", output)
}
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
expectSuccess(output, err, t)
}

func TestRangeArgsWithInvalidCount(t *testing.T) {
c := &Command{Use: "c", Args: RangeArgs(2, 4), Run: emptyRun}
c := getCommand(RangeArgs(2, 4), false)
_, err := executeCommand(c, "a")

if err == nil {
t.Fatal("Expected an error")
}

got := err.Error()
expected := "accepts between 2 and 4 arg(s), received 1"
if got != expected {
t.Fatalf("Expected %q, got %q", expected, got)
}
rangeArgsWithInvalidCount(err, t)
}

func TestRootTakesNoArgs(t *testing.T) {
Expand Down