guard / guard-rspec

Guard::RSpec automatically run your specs (much like autotest)
https://rubygems.org/gems/guard-rspec
MIT License
1.31k stars 241 forks source link

unneeded 2 times run #355

Open kvokka opened 8 years ago

kvokka commented 8 years ago

If use Guard-Rspec with Guard-rubocop with autocorrect option yo will get 2 imes run test artefact.

  1. run spec
  2. run autocorrector
  3. run spec vs corrected code

Fuctionally, corrected code is the same version of code. It will be the best, of it will be able to run test in this case only once.

I mean, that it will be perfect to check spec names query and make it qnic before run

e2 commented 8 years ago

Running guard-rubocop with autocorrect isn't philosophically a good idea.

Basically, if you change any code, you'll want to run tests.

Ideally, you'd want to run guard-rubocop autocorrect only when importing new code into a project.

If you really create lots of editing problems that you want fixed, it's best to run rubocop autocorrect even before the specs.

kvokka commented 8 years ago

I understand, that generally autocorrection better looks in pre-comit hooks But I found it very usefull in guard. I need not more beuatify or think about string format. Ctrl+s and thats it. Of course I made own .rubocop.yml, which do not crush everything, and after I can feel free with this tool.

I looked in guard gem, and did not found a task query, it mean that it will not be fast PR for me. As current solution I swich off autocorrect rubocop for specs with watch(/^(?!spec).*rb$/) in Guardfile in rubocop settings. It`s not best way, but nowday good enought for me.

Also, I can add, that after some time with rubocop you`ll be wery close with it and be able to ajust it for current tasks.

And really will be perfect, if it will be able to run rubocop after test like an option. I think it`ll be the easyest emplimentation of this reature

e2 commented 8 years ago

The problem you have is that rubocop autocorrect is like editing a file.

So when you press ctrl-s:

  1. the file is saved (triggers filesystem event)
  2. you run specs
  3. you run autocorrect (triggers filesystem event again)

If you really want a workflow, it's best to create a folder "work", copy the source files you work on there, and setup guard so it autocorrects files and copies them to 'lib'.

I think the simplest approach would be to just integrate rubocop autocorrect with your editor (so the file is autocorrected before your file in the editor is saved to the disk).

kvokka commented 8 years ago

Actually, the line of running is little bit different. rubocop -> spec -> spec. It means that there is a query of running, but it is too low level for me. I look up in ib-inotify gem, but it realized like a wrapper (also fix bug with error message on exit :), gou can see it in my fork https://github.com/kvokka/rb-inotify)

Later i'll try to look up in guard sourse, mayby I'll recognize how query works (but I didn`t got it it first time) and cut secon running in guard core.

Thank you for idea with moving files, I`ll do somthing like it, if dont find how to fix it in guard. Unforchantly, rubocop works on save anyway( any known way)

kvokka commented 8 years ago

I`ve made PR to fix it up https://github.com/guard/guard-rspec/pull/357

e2 commented 8 years ago

Even if rubocop needs to work on a file, usually editors support filters you can run on a buffer.

Sometimes you can save that buffer to a file, run rubocop on it, and then read the changed file as a result buffer. (Which can then be saved to a file).

This lets you edit a file, and then save-and-autocorrect in one keystroke.

Is saw the PR and I don't think it's a good fit for guard-rspec. Sometimes it's good to just "touch" a file to rerun the tests for that file. So a "hard rule" of "file must be different" is inconvenient, because you'd have to add a special comment into the file just so it's MD5 is changed.

Also, the MD5 approach doesn't fit in with the philosophy of supporting editors. Editors do all kinds of things with the source files (renames, deletes, multiple writes, temp files), and guard-listen uses tricky workarounds to make this work reliably.

As for other ideas, you could try the following:

  1. use something like guard-yield or guard-shell to run rubocop and save each output to a file (e.g. rubocop/foo.rb.rubocop_log)
  2. whenever a *.rubocop_log file changes, touch the spec file of the original (e.g. touch foo_spec.rb)

Of course you'd have to watch out with infinite loops (e.g. you don't want guard-rspec responding to changes in spec files, or you'll get a loop).

This means you'd need to rewrite the Guardfile rules for guard-rspec to work on *.rubocop_log files instead of source files. But, I'd be happy to merge PRs for guard-rspec to fix the Guard::RSpec::Dsl class to support this better.

kvokka commented 8 years ago

If I understend you right, you recommend to:

  1. move edited file to temp
  2. run rubocop on it
  3. run tests
  4. move file back but it is really bad idea. whe watchers will still work inselected folders and i`ll need every time loop prevent, also the cases os changing a_controller.rb and a_contoller_spec.rb are different. It will be strange unneded solution

If I understand right, not too mush people have the same problem. May be it'll be easyer us move this funcionality to options?

also, if you use rubocop u will run tests on all changes of controllers or specs. it will be enougth just add tailing spase anywhere, which'll be deleted by rubocop and it will run the tests. simle and quinck. And if user does not whant it, user may do not use the flag. name it like uniq and thas it

kvokka commented 8 years ago

I thought about your algoritm, but all implementations are very heavy. Guard have no query, and, in case of it you can not corectlly manage task line. Of course there is the way to add it, but it is too hard for me, and, I don know for what. Implementation with copy to temp directory will touch other gems, and it is worse, that some limitations with my implementation.

Anyway, if user need to run again the test, with ourt trigger, do not forget, that guard use pry It mean, thay user can activate shell-mode in pry and run rspec from guard manually.

Of course it is limitation, but not so big, as for me. I`ve made new PR, https://github.com/guard/guard-rspec/pull/358

e2 commented 8 years ago

I didn't talk about moving files. I meant that things like beautifying code should be done by the editor.

Check out: https://github.com/bbatsov/rubocop#editor-integration and https://github.com/pderichs/sublime_rubocop

RuboCop autocorrect doesn't fit into Guard's philosophy, because the input file is also the output file.

If the input file is also the output file for any tool (like a beautifier) it belongs in the editor.

So you have 2 "philosophically correct" options:

  1. use RuboCop autocorrect in your editor (and not through Guard)
  2. make it so that running RuboCop autocorrect on a file creates "fake" output files - and then change the Guard::RSpec rules to watch for changes on those "fake" output files.

So your choices are:

  1. Use RuboCop autocorrect in the editor
  2. Break up the sequence into "fake output files" so you can use Guard

For the second option, consider these pseudo rules:

  1. copy all your sources files from lib to rubocop/lib
  2. edit source files only in rubocop/ dir
  3. watch files with Guard::Yield, e.g. rubocop/lib/foo.rb -> system("rubocop -a rubocop/lib/foo.rb && cmp rubocop/foo.rb lib/foo.rb || { cp rubocop/lib/foo.rb lib/foo.rb ; }")

This way your "ugly" sources are in ./rubocop/lib, your autocorrected files are in lib and everything else is the same. So every time you change something in ./rubocop, it is autocorrected, then copied (only if it changed) to ./lib, and everything else works without configuration.

P.S. Don't change the version.rb file in PRs - that's changed only when releasing new gems, by gem owners.

kvokka commented 8 years ago

Sublime addon was starting stone where everything began, becourse, ST rubocop can correct only saved file(ST rubocop addon is just a wrapper), and it means double beautify check, only with manual start. IMHO, it is not the best way. In this case it mean, that variant 1 is not good enogth. And beautify is a script, becourse of it it works with out saving. Of course there is the way to add it in ST-plug in, but for me is the problem in phyton(I did not dig it ST plugins way yet). And yes, this desigion is elegant.

I whant to thak you for idea with Guard::Yield, and I'll try later and write here, may be this will be usefull. If it will work with out this PR it'll be brilliant. Also, I whant to sorry with changing version.rb. I'll know it in future. Thank you.

PS: Leak in your way: if you touch file, specs will not run, but you'll need do few file copies in temp folder. I spent ~2 hours trying to do something beauty in Guardfile, but now I becoming in shure, that there are 2 ways: 1) make uniq option like in my PR. Not perfect, but elegant and may be usefull 2) make sublime plug-in, which'll work on a fly. Second way is much better, but I can not realize it. First one gives basics compatibility for thouse, who need it. And may be others'll do it better.

e2 commented 8 years ago

if you touch file, specs will not run, but you'll need do few file copies in temp folder.

I don't understand.

kvokka commented 8 years ago

You told, that in my algoritm the promlem is, that if you do touch fil_spec.rb second time, spec will not run. And then you offer your idea, how to relize it. And I told, that you realization have the same issue, but also, you will need to make some file copyes. After rubocop -a it does not matter how you will compare files: by cmp or my taking md5. Anyway test will not run with out file content change.

e2 commented 8 years ago

You could consider pausing Guard while rubocop is running using https://github.com/guard/guard/wiki/Interacting-with-Guard#guard-signals

So it would be:

  1. Saving a file triggers Guard::Yield
  2. Guard::Yield pauses guard watching
  3. Guard::Yield then runs rubocop
  4. Guard::Yield then unpauses Guard
  5. Guard::Yield then touches the file (if necessary - I don't remember how pause worked)
kvokka commented 8 years ago

WOW! I did not knew that Guard have this funcionality :) This feature makes possible really good realization.

Great thanks for this link, it makes me free :) I've missed tins moment when read Guard's wiki

e2 commented 8 years ago

It should work as expected, because pausing Guard pauses Listen, and file events are accumulated.

Then, when unpaused, those events are reduced to single unique events, so it should only run specs once.

May not work on JRuby though, since the JVM handles those signals in it's own way.