-
Notifications
You must be signed in to change notification settings - Fork 492
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
Refactor pretty printer #686
Conversation
spec.add_development_dependency 'rspec', '~> 3.5.0' | ||
spec.add_development_dependency 'pry-byebug' | ||
spec.add_development_dependency 'pry' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to replace this because it's not supported on jruby (I think it's also a better option).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good
Dont really know how to test this to be honest
I am assuming it would have outputed some text to stdout...
|
||
describe '#run', :suppressed_output do | ||
it 'executes the script' do | ||
expect { runner.run }.to output('').to_stdout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@d4be4st this spec actually executes the command (in this case true
, which outputs nothing). So that at least covers calling Open3.popen3
and waiting for the process to end.
The part that is not tested (and I'm not sure how to test it) is handling SIGINT and output formatting.
Fixes #534. |
Replaces open4 with open3 to remove dependency on an external gem. With this fix the error
NotImplementedError: fork is not available on this platform
is fixed on jruby, so we can support that version of Ruby too (I tested this also by building the gem and trying it out on a project with jruby, it works.).