search

stripe / stripe-cli

A command-line tool for Stripe
https://stripe.com/docs/stripe-cli
Apache License 2.0
1.59k stars 374 forks source link

Removes test of default editor passthrough from git #1080

Closed fhats-stripe closed 1 year ago

fhats-stripe commented 1 year ago

Reviewers

r? @charliecruzan-stripe cc @stripe/developer-products

Summary

I set up the stripe CLI for the first time on my laptop today and ran make test as suggested by the README. Everything passed except for one test, which tests that when an editor preference is not given to git that it uses a preference from the current environment (the EDITOR variable). I keep EDITOR set to vim on my system, which causes the test to fail as it wants vi explicitly.

My gut feeling is that this test isn't really covering functionality that's unique to the Stripe CLI, and is instead covering how git's editor defaults are computed. I thought about setting EDITOR in the test's setup, which would have to change based on runtime.GOOS. Then I figured that we might just be better off without the test at all, if all we're doing is checking that git grabs its editor default from the environment? 🤷

Anyway, happy to reverse course on this and keep the test with some setup to make it more durable when EDITOR is set if we'd prefer to keep it! Just let me know :)

Testing

Originally fails in the following manner on my system:

fhats@st-fhats2:~/stripe/stripe-cli$make test                                                                                                                                                                                                                   master 14:07:46
go test  -failfast -race -coverpkg=./... -covermode=atomic -coverprofile=coverage.txt ./... -run . -timeout=2m
?       github.com/stripe/stripe-cli/cmd/stripe [no test files]
?       github.com/stripe/stripe-cli/pkg/ansi   [no test files]
ok      github.com/stripe/stripe-cli/pkg/cmd    2.378s  coverage: 17.8% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/cmd/logs   [no test files]
ok      github.com/stripe/stripe-cli/pkg/cmd/plugin 1.398s  coverage: 15.9% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/cmd/resource   0.776s  coverage: 18.1% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/cmd/samples    [no test files]
ok      github.com/stripe/stripe-cli/pkg/config 1.569s  coverage: 16.7% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/fixtures   2.209s  coverage: 20.2% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/gen    1.183s  coverage: 16.1% of statements in ./...
--- FAIL: TestGetDefaultGitEditor (0.19s)
    --- FAIL: TestGetDefaultGitEditor/no_GIT_EDITOR_or_EDITOR_defaults_to_vi (0.06s)
        editor_test.go:101:
                Error Trace:    /Users/fhats/stripe/stripe-cli/pkg/git/editor_test.go:101
                Error:          Not equal:
                                expected: "vi"
                                actual  : "vim"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -vi
                                +vim
                Test:           TestGetDefaultGitEditor/no_GIT_EDITOR_or_EDITOR_defaults_to_vi
FAIL
coverage: 16.2% of statements in ./...
FAIL    github.com/stripe/stripe-cli/pkg/git    2.016s
ok      github.com/stripe/stripe-cli/pkg/login  0.440s  coverage: 18.9% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/login/acct 0.597s  coverage: 16.5% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/login/keys 2.047s  coverage: 17.8% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/logout [no test files]
ok      github.com/stripe/stripe-cli/pkg/logtailing 0.705s  coverage: 15.9% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/open   [no test files]
ok      github.com/stripe/stripe-cli/pkg/parsers    0.611s  coverage: 17.4% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/plugins    0.771s  coverage: 20.4% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/plugins/proto  [no test files]
ok      github.com/stripe/stripe-cli/pkg/proxy  0.549s  coverage: 17.7% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/requests   0.549s  coverage: 19.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/rpcservice 0.557s  coverage: 30.5% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/samples    0.503s  coverage: 16.6% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/spec   3.582s  coverage: 16.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/status 0.550s  coverage: 16.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/stripe 0.772s  coverage: 17.5% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/stripeauth 0.615s  coverage: 16.7% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/terminal   [no test files]
ok      github.com/stripe/stripe-cli/pkg/terminal/p400  0.681s  coverage: 16.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/useragent  0.799s  coverage: 15.8% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/validators 0.638s  coverage: 16.5% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/version    0.742s  coverage: 15.8% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/websocket  0.750s  coverage: 17.5% of statements in ./...
?       github.com/stripe/stripe-cli/rpc    [no test files]
FAIL
make: *** [test] Error 1

and it's reproducable based on whatever you set EDITOR to (like /bin/shuf, a nefarious trick):

fhats@st-fhats2:~/stripe/stripe-cli$EDITOR="/bin/shuf" make test                                                                                                                                                                                 fhats/fix-editor-test 14:22:32
go test  -failfast -race -coverpkg=./... -covermode=atomic -coverprofile=coverage.txt ./... -run . -timeout=2m
?       github.com/stripe/stripe-cli/cmd/stripe [no test files]
?       github.com/stripe/stripe-cli/pkg/ansi   [no test files]
ok      github.com/stripe/stripe-cli/pkg/cmd    2.291s  coverage: 17.8% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/cmd/logs   [no test files]
ok      github.com/stripe/stripe-cli/pkg/cmd/plugin 0.759s  coverage: 15.9% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/cmd/resource   0.906s  coverage: 18.1% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/cmd/samples    [no test files]
ok      github.com/stripe/stripe-cli/pkg/config 1.164s  coverage: 16.7% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/fixtures   1.548s  coverage: 20.2% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/gen    1.355s  coverage: 16.1% of statements in ./...
--- FAIL: TestGetDefaultGitEditor (0.12s)
    --- FAIL: TestGetDefaultGitEditor/no_GIT_EDITOR_or_EDITOR_defaults_to_vi (0.03s)
        editor_test.go:101:
                Error Trace:    /Users/fhats/stripe/stripe-cli/pkg/git/editor_test.go:101
                Error:          Not equal:
                                expected: "vi"
                                actual  : "/bin/shuf"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1 @@
                                -vi
                                +/bin/shuf
                Test:           TestGetDefaultGitEditor/no_GIT_EDITOR_or_EDITOR_defaults_to_vi
FAIL
coverage: 16.2% of statements in ./...
FAIL    github.com/stripe/stripe-cli/pkg/git    0.744s
ok      github.com/stripe/stripe-cli/pkg/login  0.458s  coverage: 18.9% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/login/acct 1.847s  coverage: 16.5% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/login/keys 2.139s  coverage: 17.8% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/logout [no test files]
ok      github.com/stripe/stripe-cli/pkg/logtailing 0.505s  coverage: 15.9% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/open   [no test files]
ok      github.com/stripe/stripe-cli/pkg/parsers    0.528s  coverage: 17.4% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/plugins    0.729s  coverage: 20.4% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/plugins/proto  [no test files]
ok      github.com/stripe/stripe-cli/pkg/proxy  0.540s  coverage: 17.7% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/requests   0.489s  coverage: 19.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/rpcservice 0.739s  coverage: 30.5% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/samples    0.591s  coverage: 16.6% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/spec   3.692s  coverage: 16.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/status 0.435s  coverage: 16.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/stripe 0.574s  coverage: 17.5% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/stripeauth 0.558s  coverage: 16.7% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/terminal   [no test files]
ok      github.com/stripe/stripe-cli/pkg/terminal/p400  0.406s  coverage: 16.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/useragent  0.457s  coverage: 15.8% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/validators 0.527s  coverage: 16.5% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/version    0.502s  coverage: 15.8% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/websocket  0.475s  coverage: 17.5% of statements in ./...
?       github.com/stripe/stripe-cli/rpc    [no test files]
FAIL
make: *** [test] Error 1

After this change tests now pass for me locally, even when EDITOR is set to something else:

fhats@st-fhats2:~/stripe/stripe-cli$EDITOR="/bin/shuf" make test                                                                                                                                                                                 fhats/fix-editor-test 14:55:31
go test  -failfast -race -coverpkg=./... -covermode=atomic -coverprofile=coverage.txt ./... -run . -timeout=2m
?       github.com/stripe/stripe-cli/cmd/stripe [no test files]
?       github.com/stripe/stripe-cli/pkg/ansi   [no test files]
ok      github.com/stripe/stripe-cli/pkg/cmd    2.400s  coverage: 17.8% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/cmd/logs   [no test files]
ok      github.com/stripe/stripe-cli/pkg/cmd/plugin 0.597s  coverage: 15.9% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/cmd/resource   1.022s  coverage: 18.1% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/cmd/samples    [no test files]
ok      github.com/stripe/stripe-cli/pkg/config 0.850s  coverage: 16.7% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/fixtures   1.603s  coverage: 20.2% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/gen    1.982s  coverage: 16.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/git    1.751s  coverage: 16.2% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/login  1.814s  coverage: 18.9% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/login/acct 0.431s  coverage: 16.5% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/login/keys 2.272s  coverage: 17.8% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/logout [no test files]
ok      github.com/stripe/stripe-cli/pkg/logtailing 0.623s  coverage: 15.9% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/open   [no test files]
ok      github.com/stripe/stripe-cli/pkg/parsers    0.609s  coverage: 17.4% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/plugins    0.712s  coverage: 20.4% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/plugins/proto  [no test files]
ok      github.com/stripe/stripe-cli/pkg/proxy  0.658s  coverage: 17.7% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/requests   0.639s  coverage: 19.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/rpcservice 1.069s  coverage: 30.5% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/samples    0.622s  coverage: 16.6% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/spec   3.906s  coverage: 16.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/status 0.579s  coverage: 16.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/stripe 0.625s  coverage: 17.5% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/stripeauth 0.626s  coverage: 16.7% of statements in ./...
?       github.com/stripe/stripe-cli/pkg/terminal   [no test files]
ok      github.com/stripe/stripe-cli/pkg/terminal/p400  0.583s  coverage: 16.1% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/useragent  0.543s  coverage: 15.8% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/validators 0.497s  coverage: 16.5% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/version    0.396s  coverage: 15.8% of statements in ./...
ok      github.com/stripe/stripe-cli/pkg/websocket  0.450s  coverage: 17.5% of statements in ./...
?       github.com/stripe/stripe-cli/rpc    [no test files]