luan / vimfiles

The Vim Configuration. Uses vim-plug to manage plugins.
172 stars 58 forks source link

Add tests for running tests #118

Open tom025 opened 6 years ago

tom025 commented 6 years ago

I ultimately want to add python/pytest support but felt that the code in lib/functions.vim was too poorly factored to reasonably extend.

This is a start in the direction of adding tests to this repo and is in no way finished. I want to push this for some early feedback.

Specifically it would be good to get feedback on:

luan commented 6 years ago

Let's first talk about the test runner issue, I'll address adding tests to this repo in another comment.

I think I like the idea of using vim-test. We should make an effort to try and not change the current experience while enhancing the rest of the functionality. It seems like it'd be trivial to add vipe support to vim-test in the config itself by injecting a custom strategy. But it'd be even cooler to add that to the plugin directly as a PR. I might play with vim-test myself this weekend but I'd be open to a PR replacing the custom functions with it.

luan commented 6 years ago

Now about the tests, almost 4 years ago I tried to do something similar to what you're doing here and gave up because I didn't think it was worth the effort. You can see what I was doing on this branch which is by no means complete or funcional, nor is it apply-able to the current version of master.

Having a thorough test suite here would be kinda of nice, potentially enabling more contributions, so it's something I'm interested in exploring.

You should take a look at vader.vim. It seems to a more self contained solution with a better DSL for testing vim. I haven't tried it, but at first glace it feels like a good alternative. Should work with both vim and neovim.

It that doesn't work, I really enjoyed using cucumber for it back in the day because it describes the usage of vim in a more declarative way, so I'd encourage you to give that a shot. I don't like how you mention a dependency on macvim on your README.md there, I'd like to stay away from macvim. You should be able to get the tests running in a container dependencies other than vim itself.

Now, having said all this, I want to encourage you to try and explore the testing options carefully. If they are going to be a big time sink as far as debugging non-valuable failures I'll claim it not worth it and probably will revert/not-merge since I want this repo to be easy to maintain, even if that means the developer contributing needs more context to do so effectively. So keep that in mind as you spend your valuable time adding tests. The cost of fixing issues currently is very low for me, and the cost of adding feaures is also very low. So I would want the test suite to either not change that or make it better.

Thanks for all the work and thought put into this, hope to hear from you again soon :)

tom025 commented 6 years ago

Thanks for the feedback! Also thanks very much for this repo!

I don't like how you mention a dependency on macvim on your README.md there, I'd like to stay away from macvim.

I am not happy with the macvim dependency as well. I would not look to support it longer term. It happened to be the easiest way to get these tests running on my machine. I wanted to start a conversation with some working code.

That said I would still expect all the code in this repo to work on vim, neovim, macvim, gvim so would be good to have tests to verify across all of those implementations. I feel that is a later goal though.

I will have a look into more maintainable ways to write the sort of tests I think would be valuable. Perhaps running all these tests in some form of docker container with a proper x11 setup could help (or hinder).

Having a thorough test suite here would be kinda of nice, potentially enabling more contributions, so it's something I'm interested in exploring.

I agree. Much of this repo is configuring and gluing various plugins together. I feel that having executable examples that show how this vim config is intended to be used with various languages would be useful and worth maintaining. I think it would also help users get more value from this config.

You should take a look at vader.vim.

I will take a look at vader and see if I can reasonably replicate the tests here in a separate PR.

Now, having said all this, I want to encourage you to try and explore the testing options carefully. If they are going to be a big time sink as far as debugging non-valuable failures I'll claim it not worth it and probably will revert/not-merge since I want this repo to be easy to maintain, even if that means the developer contributing needs more context to do so effectively. So keep that in mind as you spend your valuable time adding tests. The cost of fixing issues currently is very low for me, and the cost of adding feaures is also very low. So I would want the test suite to either not change that or make it better.

I think it is very important to choose a testing setup that is easy to maintain and reliable as I do not have the time to commit to keeping all the tests running all the time on this repo. I would hope that any test suite that gets added would be useful enough for you and other contributors that they would be maintained. I would hope to plant the seed of a test suite that can be extended easily and others contribute to it. If it goes nowhere then 🤷‍♂️ . I would have personally learnt what it would take to test vim configurations which is of some interest to me (at the moment at least).

Thanks!

tom025 commented 6 years ago

Had a play with vim-test.

Was able to get vim-test executing pytest via vipe with a minimal amount of code:

function! VipeStrategy(cmd)
  call vipe#push(a:cmd)
endfunction

let g:test#custom_strategies = {'vipe': function('VipeStrategy')}
let g:test#strategy = 'vipe'

let test#python#runner = 'pytest'
let test#python#pytest#options = {
      \ 'nearest': '--verbose'
      \ }

nmap <leader>t :TestFile<CR>
nmap <leader>T :TestNearest<CR>
nmap <leader>A :TestSuite<CR>
luan commented 6 years ago

@tom025 that looks great! Do you want me to try and integrate that into this repo for the other languages we currently support or do you plan on making a PR?