search

neovim / neovim-ruby

Ruby support for Neovim
MIT License
340 stars 17 forks source link

disable ShaDa when running tests #17

Closed jpleau closed 8 years ago

jpleau commented 8 years ago

This prevents the tests from altering the user's own shada history if he/she runs the tests on a local install.

Fixes #16

jpleau commented 8 years ago

I also re-ordered the arguments passed to nvim, so they're all consistent.

One issue though. When disabling ShaDa for tests, they take a very long time to run. I am not sure if that's a side-effect of disabling it or something in the tests themselves that cause it.

alexgenco commented 8 years ago

Wow yeah this change takes the test suite from 11s to >2min for me. The simplest repro step I can find is time rspec spec/neovim_spec.rb -e ".attach_child". The test is 1 second slower if you add -i NONE to the spawn arguments. Note that getting rid of either the -i or -u options fixes the delay. @justinmk do you have any thoughts? Sorry I'd look into it deeper but I'm on vacation for the next week.

@jpleau this seems like a good opportunity to dry up some of these tests. How do you feel about adding a Support.child_argv helper or something to spec/support.rb and using that in tests rather than repeating them?

jpleau commented 8 years ago

@alexgenco Added 6e16d0f6230dadcd3bdfab262da893dded210af0 that adds a function called Support.child_argv like you suggested.

Note: I haven't done ruby in a while, don't hesitate to nit picks anything I could have done better !

I made it "customizable" so if we want to run an entire command we can still use this function, add extra stuff (like --headless or +q like some tests already did)

jpleau commented 8 years ago

Will fix the failing tests with ruby 1.9 tomorrow, shouldn't change the code too much

alexgenco commented 8 years ago

I think I fixed the performance problem with https://github.com/alexgenco/neovim-ruby/commit/b31bee127c072f4a2dbaeee7d942da38ce824971. Go ahead and rebase onto master.

jpleau commented 8 years ago

All done, would you prefer me to squash to only have one commit ?

alexgenco commented 8 years ago

No need to squash if each commit passes rake. Thanks!

jpleau commented 8 years ago

Thanks as well.

By the way, this shada issue was discovered while packaging neovim-ruby for Debian. Forgot to let you know in advance that I planned to package this, so here we go ;)

alexgenco commented 8 years ago

Awesome!