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.

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

Generate table tests using gotests tool via :GoTableTests #808

Closed
wants to merge 1 commit into from

Conversation

bradleyfalzon
Copy link
Contributor

@bradleyfalzon bradleyfalzon commented Apr 21, 2016

Extends vim-go to use https://github.com/cweill/gotests to generate table tests https://github.com/golang/go/wiki/TableDrivenTests as discussed in issue #807 (see also cweill/gotests#20)

Only supports generating table tests on the function under the cursor, i.e. does not add support for generating table tests for all selected functions when using visual mode.

Does not support refreshing buffers or tabs if the corresponding _test.go file is already open.

:GoInstallBinaries does not display the path github.com/cweill/gotests/... correctly when installing, which is the path gotests project expects you use to install it. Instead, this change references the path to the binary github.com/cweill/gotests/gotests. This should not be a problem and tested OK, but may need to be reconsidered if issues do arise.

Extends vim-go to use https://github.com/cweill/gotests to
generate table tests https://github.com/golang/go/wiki/TableDrivenTests
as discussed in issue fatih#807 (see also cweill/gotests/fatih#20)

Only supports generating table tests on the function under the
cursor, i.e. does not add support for generating table tests for
all selected functions when using visual mode.

Does not support refreshing buffers or tabs if the corresponding
_test.go file is already open.

:GoInstallBinaries does not display the path
`github.com/cweill/gotests/...` correctly when installing, which
is the path gotests project expects you use to install it. Instead,
this change references the path to the binary `github.com/cweill/gotests/gotests`.
This should not be a problem and tested OK, but may need to be
reconsidered if issues do arise.
@bradleyfalzon
Copy link
Contributor Author

Reopening buffers/tabs may require existing buffer to be saved first, gotests executed and then buffer refresh contents, further complications with tabs may ensue. However, this should be done at a later stage. Note, gotests can write changes to _test.go file or stdout, currently gotest writes to the file itself.

@bradleyfalzon
Copy link
Contributor Author

Note wasn't able to get v:shell_error to ever be true when there was an error gotests does set non-zero exit codes on error. Am I checking this wrong?

@bradleyfalzon
Copy link
Contributor Author

Note, I completely expect feedback on this before a merge, as I'm OK if this isn't yet release ready.

@itsjamie
Copy link

This is the work for #807. Thank you @bradleyfalzon!

@fatih
Copy link
Owner

fatih commented Apr 28, 2016

Hi @bradleyfalzon

Thanks for the work you put. I've went over the tool gotests first and thinking not to merge it. Let me explain why.

First of all, the reason is there are tons of other tools, such as stringer that generates code for you. It's just too much for vim-go, because in the end I'll be the guy to maintain this. There are many of other stuff that breaks and needs maintenance, and honestly something like this is just too much for vim-go.

Second this is something that is not widely used and only a certain percentage of users would use. It's not something like :GoCoverage or co, where it's a part of everyones everyday's workflow. It produces a test for a certain kind (table tests) and that too can be implemented in various ways too.

I know you put some work, but I have to neglect it. I was going to write this under #807, but you started it already before I even could write something.

I hope it's ok for you, I appreciate your work and I hope you don't get me wrong.

Thanks

@bradleyfalzon
Copy link
Contributor Author

bradleyfalzon commented Apr 28, 2016

Those are valid reasons fatih, and this doesn't preclude me from creating its own plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants