Guard::MochaNode buffers output of progress formatter

[ashmoran]

I noticed that Guard::MochaNode works fine on line-based Mocha reporters (list, spec, etc) but that with single-line character-based reporters (dot, progress, etc), it buffers the entire line, which kinda defeats the purpose of these reporters.

After looking around I found the culprit is here: https://github.com/kanzeon/guard-mocha-node/blob/master/lib/guard/mocha_node/spec_state.rb#L24

I'm a little puzzled, because the gem version I've installed with Bundler has this on those lines:

# stream stdout immediately
until @stdout.eof?
  line = @stdout.gets
  print line if !line.strip.empty?
end

whereas the version in this repo has this:

@stdout.lines { |line| print line if !line.strip.empty? }

Is @neerolyte now pushing this gem to rubygems.org via https://github.com/neerolyte/guard-mocha-node/ ? If so, I don't have a way to report this issue directly, as he doesn't have issues enabled on his fork.

Anyway, I found a relatively easy fix to the problem, which is to print mocha's output one character at a time.

# stream stdout immediately
until @stdout.eof?
  output = @stdout.getc
  $stdout.putc output
end

Although that made me wonder, why is Guard::MochaNode separating the stdout and stderr output from mocha? Would there be a disadvantage of using Kernel#system instead of Open3.popen3 in the runner here: https://github.com/kanzeon/guard-mocha-node/blob/master/lib/guard/mocha_node/runner.rb#L27 - and just let mocha take responsibility for sequencing the output?

[ashmoran]

Also I found an article while I was reading up that suggests that using Open3 in the general case isn't as easy as it looks: http://coldattic.info/shvedsky/pro/blogs/a-foo-walks-into-a-bar/posts/63

The key being that reading stdout will block if the child process fills its stderr buffer, which creates a deadlock. So the current strategy for reading mocha's output may assume it doesn't write (much) to stderr. I had no idea this was an issue with Open3 before.

[neerolyte]

@ashmoran Thanks for all the updates. I've been pushing a couple of fixes up in to a gem intermittently as @Kanzeon's maintenance appeared to have died off.

I've turned on issues on my fork as well now: https://github.com/neerolyte/guard-mocha-node/issues (not sure why it was off) so you can report directly there or here and I'll try to sew stuff together :)

I'll see if I can get system passing things all the way through as suggested (IIRC it might be that the exit status was a problem with Kernel#system, so if I get stuck I'll test out the getc loop you've suggested too), but I'm not amazing with Ruby, so may take a while...

Cheers,
Dave

[neerolyte]

@ashmoran I tested out Kernel#system and it seems to have "just worked". See neerolyte/guard-mocha-node@master...use_system_call (sorry accidentally reindented some of the files along for the ride polluting that commit a little :/)

I'm a bit nervous about the quality of the tests around some the system call stuff though so I'd like a second sanity check if you're up for it on the changes I've made, you should be able to take it for a spin with this in a Gemfile:

gem "guard-mocha-node", :git => https://github.com/neerolyte/guard-mocha-node.git', :branch => 'use_system_call'

I've tested with the dot and progress reporters and they both look good now to me now, also ensured we haven't screwed up any of the tracking of which files are failing.

Cheers,
Dave

[ashmoran]

Hey @neerolyte Dave, that works perfectly for me :D

Re testing: normally what I'd do in this situation is just write tests for the generated command (ie Runner. mocha_node_command), there's no need to test Kernel.should_receive(:system) each time. Guard::MochaNode::Runner.stub(execute_mocha_node_command: true) (or false) would isolate that and work whether it was using Kernel or Open3. Testing the actual system call is impossible within the same RSpec process, you'd need to launch the runner in a separate process with a real mocha command and inspect the stdout/stderr.

I don't think any of this is worth worrying about though, as this tool is simple enough I think we can rely on manual inspection to check the output and the tests can check the correct args are used, and they're good enough for now in their current form.

BTW one little thing, as you say you're not a hardcore Rubyist - the Kernel module is mixed in to every Ruby object, so you can just do system "foo" as well as the longer Kernel.system("foo"). The same goes for eg Kernel.puts, which is how writing just puts prints to stdout.

Thanks for looking into this! I figure the intersection of Guard users and Mocha users must be quite small, so I was really pleased to find someone maintaining this.

[neerolyte]

@ashmoran Thanks for the advice (I think I follow most of it :) ), however to keep things simple (for me, for now), I've left the tests as they were and using the longer Kernel.system still (as changing it out would have meant changing the tests).

Either way, there's a new gem published (0.0.6), so should be able to use the gem again.

Cheers,
Dave

[ashmoran]

@neerolyte Thanks Dave, I'm on the gem version now. My explanation was probably a bit brief and blurry, don't worry about it, basically it's fine as it is. If this plugin needs any more work I can have a look at refactoring some bits of it, making the tests simpler etc.