From fc77cbc96403a9e6711d8749cb4d5ed9e9d732de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 18 Jan 2021 23:25:45 +0100 Subject: [PATCH] Deprecate `test.ExpectLines` For asserting command output, exact string matches are preferred in most cases. In cases when a pattern match is needed, the test can use regexp ad hoc. --- pkg/cmd/alias/set/set_test.go | 11 +++++++++++ pkg/cmd/issue/list/list_test.go | 1 + pkg/cmd/issue/view/view_test.go | 4 ++++ pkg/cmd/pr/close/close_test.go | 1 + pkg/cmd/pr/merge/merge_test.go | 6 ++++++ pkg/cmd/pr/review/review_test.go | 6 ++++++ pkg/cmd/pr/shared/preserve_test.go | 1 + pkg/cmd/pr/view/view_test.go | 4 ++++ pkg/cmd/repo/fork/fork_test.go | 5 +++++ pkg/cmd/secret/list/list_test.go | 1 + test/helpers.go | 1 + 11 files changed, 41 insertions(+) diff --git a/pkg/cmd/alias/set/set_test.go b/pkg/cmd/alias/set/set_test.go index 95f3a5031..bb7e26325 100644 --- a/pkg/cmd/alias/set/set_test.go +++ b/pkg/cmd/alias/set/set_test.go @@ -86,7 +86,9 @@ func TestAliasSet_empty_aliases(t *testing.T) { t.Fatalf("unexpected error: %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Added alias") + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), "") expected := `aliases: @@ -108,6 +110,7 @@ func TestAliasSet_existing_alias(t *testing.T) { output, err := runCommand(cfg, true, "co 'pr checkout -Rcool/repo'") require.NoError(t, err) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Changed alias.*co.*from.*pr checkout.*to.*pr checkout -Rcool/repo") } @@ -120,8 +123,10 @@ func TestAliasSet_space_args(t *testing.T) { output, err := runCommand(cfg, true, `il 'issue list -l "cool story"'`) require.NoError(t, err) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), `Adding alias for.*il.*issue list -l "cool story"`) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, mainBuf.String(), `il: issue list -l "cool story"`) } @@ -156,7 +161,9 @@ func TestAliasSet_arg_processing(t *testing.T) { t.Fatalf("got unexpected error running %s: %s", c.Cmd, err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), c.ExpectedOutputLine) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, mainBuf.String(), c.ExpectedConfigLine) }) } @@ -178,6 +185,7 @@ aliases: diff: pr diff ` + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Adding alias for.*diff.*pr diff", "Added alias.") assert.Equal(t, expected, mainBuf.String()) } @@ -199,6 +207,7 @@ func TestAliasSet_existing_aliases(t *testing.T) { view: pr view ` + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Adding alias for.*view.*pr view", "Added alias.") assert.Equal(t, expected, mainBuf.String()) @@ -226,6 +235,7 @@ func TestShellAlias_flag(t *testing.T) { t.Fatalf("unexpected error: %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Adding alias for.*igrep") expected := `aliases: @@ -243,6 +253,7 @@ func TestShellAlias_bang(t *testing.T) { output, err := runCommand(cfg, true, "igrep '!gh issue list | grep'") require.NoError(t, err) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Adding alias for.*igrep") expected := `aliases: diff --git a/pkg/cmd/issue/list/list_test.go b/pkg/cmd/issue/list/list_test.go index 3e46a0c0b..98b954ee3 100644 --- a/pkg/cmd/issue/list/list_test.go +++ b/pkg/cmd/issue/list/list_test.go @@ -80,6 +80,7 @@ func TestIssueList_nontty(t *testing.T) { } eq(t, output.Stderr(), "") + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), `1[\t]+number won[\t]+label[\t]+\d+`, `2[\t]+number too[\t]+label[\t]+\d+`, diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index 448e5aedd..03548c554 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -198,6 +198,7 @@ func TestIssueView_nontty_Preview(t *testing.T) { assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } @@ -264,6 +265,7 @@ func TestIssueView_tty_Preview(t *testing.T) { assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } @@ -422,6 +424,7 @@ func TestIssueView_tty_Comments(t *testing.T) { } assert.NoError(t, err) assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } @@ -496,6 +499,7 @@ func TestIssueView_nontty_Comments(t *testing.T) { } assert.NoError(t, err) assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } diff --git a/pkg/cmd/pr/close/close_test.go b/pkg/cmd/pr/close/close_test.go index 0e5a1d942..09d8e521b 100644 --- a/pkg/cmd/pr/close/close_test.go +++ b/pkg/cmd/pr/close/close_test.go @@ -147,5 +147,6 @@ func TestPrClose_deleteBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr close` %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), `Closed pull request #96 \(The title of the PR\)`, `Deleted branch blueberries`) } diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index 3f2c55204..37049d0ef 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -325,6 +325,7 @@ func TestPrMerge_deleteBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) } @@ -357,6 +358,7 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), `Merged pull request #10 \(Blueberries are a good fruit\)`, `Deleted branch.*blueberries`) } @@ -459,6 +461,7 @@ func TestPrMerge_squash(t *testing.T) { t.Fatalf("error running command `pr merge`: %v", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Squashed and merged pull request #3") } @@ -498,6 +501,7 @@ func TestPrMerge_alreadyMerged(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "✔ Deleted branch blueberries and switched to branch master") } @@ -577,6 +581,7 @@ func TestPRMerge_interactive(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Merged pull request #3") } @@ -630,6 +635,7 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Merged pull request #3", "Deleted branch blueberries and switched to branch master") } diff --git a/pkg/cmd/pr/review/review_test.go b/pkg/cmd/pr/review/review_test.go index 749495c7a..2919526d3 100644 --- a/pkg/cmd/pr/review/review_test.go +++ b/pkg/cmd/pr/review/review_test.go @@ -218,6 +218,7 @@ func TestPRReview_url_arg(t *testing.T) { t.Fatalf("error running pr review: %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Approved pull request #123") } @@ -260,6 +261,7 @@ func TestPRReview_number_arg(t *testing.T) { t.Fatalf("error running pr review: %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Approved pull request #123") } @@ -293,6 +295,7 @@ func TestPRReview_no_arg(t *testing.T) { t.Fatalf("error running pr review: %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Reviewed pull request #123") } @@ -425,8 +428,10 @@ func TestPRReview_interactive(t *testing.T) { t.Fatalf("got unexpected error running pr review: %s", err) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Approved pull request #123") + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), "Got:", "cool.*story") @@ -532,5 +537,6 @@ func TestPRReview_interactive_blank_approve(t *testing.T) { t.Errorf("did not expect to see body printed in %s", output.String()) } + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Approved pull request #123") } diff --git a/pkg/cmd/pr/shared/preserve_test.go b/pkg/cmd/pr/shared/preserve_test.go index 28949d957..892973f26 100644 --- a/pkg/cmd/pr/shared/preserve_test.go +++ b/pkg/cmd/pr/shared/preserve_test.go @@ -98,6 +98,7 @@ func Test_PreserveInput(t *testing.T) { assert.NoError(t, err) if tt.wantPreservation { + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, errOut.String(), tt.wantErrLine) preserved := &IssueMetadataState{} assert.NoError(t, json.Unmarshal(data, preserved)) diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index aeaf5ee01..3ceac3526 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -338,6 +338,7 @@ func TestPRView_Preview_nontty(t *testing.T) { assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } @@ -483,6 +484,7 @@ func TestPRView_Preview(t *testing.T) { assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) } @@ -798,6 +800,7 @@ func TestPRView_tty_Comments(t *testing.T) { } assert.NoError(t, err) assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tt.expectedOutputs...) }) } @@ -876,6 +879,7 @@ func TestPRView_nontty_Comments(t *testing.T) { } assert.NoError(t, err) assert.Equal(t, "", output.Stderr()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.String(), tt.expectedOutputs...) }) } diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 2558f2c2c..ade9d4d7d 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -279,6 +279,7 @@ func TestRepoFork_in_parent_yes(t *testing.T) { } assert.Equal(t, "", output.String()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Added remote.*origin") @@ -304,6 +305,7 @@ func TestRepoFork_outside_yes(t *testing.T) { } assert.Equal(t, "", output.String()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Cloned fork") @@ -331,6 +333,7 @@ func TestRepoFork_outside_survey_yes(t *testing.T) { } assert.Equal(t, "", output.String()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Cloned fork") @@ -386,6 +389,7 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) { assert.Equal(t, "", output.String()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Renamed.*origin.*remote to.*upstream", @@ -450,6 +454,7 @@ func TestRepoFork_in_parent_match_protocol(t *testing.T) { assert.Equal(t, "", output.String()) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, output.Stderr(), "Created fork.*someone/REPO", "Added remote.*origin") diff --git a/pkg/cmd/secret/list/list_test.go b/pkg/cmd/secret/list/list_test.go index 601c13c27..a791bb13c 100644 --- a/pkg/cmd/secret/list/list_test.go +++ b/pkg/cmd/secret/list/list_test.go @@ -191,6 +191,7 @@ func Test_listRun(t *testing.T) { reg.Verify(t) + //nolint:staticcheck // prefer exact matchers over ExpectLines test.ExpectLines(t, stdout.String(), tt.wantOut...) }) } diff --git a/test/helpers.go b/test/helpers.go index 5a493dcae..f1dc5875f 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -91,6 +91,7 @@ type T interface { Errorf(string, ...interface{}) } +// Deprecated: prefer exact matches for command output func ExpectLines(t T, output string, lines ...string) { t.Helper() var r *regexp.Regexp