-
-
Notifications
You must be signed in to change notification settings - Fork 753
Add a spec for interrupt trapping in Runner #2096
Conversation
It looks like this failed some Ruby versions in the Travis CI build, namely all JRuby and 1.9.2 MRI. I'm going to investigate to see if I can determine the issue. |
The |
JRuby claims to, though I wouldn't be surprised if there were differences. I'm currently working to diagnose the JRuby issue. If it turns out that this is non suppported, what's your recommendation? Is the spec not viable, or is it possible to limit this to MRI? What about 1.9.2? I'm having trouble getting the script to even run in that version though it's obviously setup on my end causing the issue. |
I can confirm that I'm able to trap SIGINT in a basic Ruby script with jruby 1.7.19 (1.9.3p551), and changing the trap syntax to pass in a proc object doesn't seem to help at all. Unfortunately I'm probably beyond my capacity on this one, but will continue poking. |
I have this gist which runs successfully. Per Charles Nutter I'm filing this as a JRuby bug, but don't seem to be able to reproduce in just JRuby. jruby/jruby#3413 |
@myrridin is this still something you'd like to continue with? Would love to see this merged |
I can definitely take a fresh look. I've got an issue open on the jruby repo, but this doesn't appear to be an actual jruby bug. I'll do what I can to get this across the finish line. Thanks for checking in :-) |
@myrridin cool, thanks. Are you likely to be able to push more code in the next week? |
If I'm going to get it, it will be in the next week. If my second attempt doesn't work out I'll have to pass along as beyond my capabilities (or temporal capacity) :-) I'll do my best to update here regardles of the outcome. |
I changed up my approach here. This is borrowed directly from Rubocop: https://github.com/bbatsov/rubocop/blob/33aa8bcc3f50e545f97043b7eb7fd7228c4476da/spec/rubocop/cli_spec.rb#L2176 The idea is that instead of sending a SIGINT directly to the process as it runs, you capture the interrupt handlers that were created and manually call them. The other solution was to add a sleep into the previous test, which felt wrong to do. The problem with signal handling is that there's no way I could figure out to block until the signal was processed. If I artificially delayed before the end of the spec I had full green every time, but the whole approach seemed to be going about it in the wrong way. A friend of mine pointed me to this file and it seemed like a much better way to go about it. |
That passes jruby but fails MRI now :-( Please hold. I will definitely complete this. On a mission now. |
The Runner.trap_interrupt method was missing a spec. My goal is to extend interrupt handling per tenderlove's issue #2055, but I wanted to make sure there was a spec in place first.
[skip ci]
This avoids a problem some users have seen where files are double-required. Fixes rspec#2099.
[skip ci]
[skip ci]
... by using Array for lines until final rendering.
It allows detail or failure/error line to be used as the first line by omitting preceding lines, rather than specifying content of detail or failure/error as description.
Ripper might fail to parse a Ruby source even if the current runtime parsed it properly, because some versions of JRuby have different implementation of runtime parser and Ripper parser. jruby/jruby#2427 So we handle the syntax error in Ripper and fall back to the simple single line extraction in that case.
- Better warning when an invalid value is passed for `--fail-fast` (including silencing call site since it will not be in user code). - No need to stub `reporter.failures_required`. - No need for the `failures_required` method at all. - Use `fail_fast_limit_met?` in more situations. - Remove unneeded `ExampleGroup.fail_fast?` method.
The return value of RubyProject.add_to_load_path is neither useful nor used anywhere. In this case, #each communicates intent better than #map.
The existing wording was confusing for this situation: * You have two specs, a and b. * Spec a always fails. * Spec b fails only if a runs first. In such a situation, your failures are order dependent, but our bisector printed "failure is not order dependent", which was quite confusing. The new wording, "failures(s) do not require any non-failures to run first" is more accurate.
`Exception.new.backtrace` is `nil` so we can't count on there always being a backtrace. Before now we relied on the `format_backtrace` caller handling this (passing `ex.backtrace || []`), but with rspec#2074 now also including the `exception.cause` in failure output, the code there did not guard the `format_backtrace` call with `|| []` and is causing users to get a `NoMethodError`. It's easiest if `format_backtrace just handles the `nil` case so each caller does not have to remember to handle it. Fixes rspec#2117.
[ci skip]
This is useful because at the moment the only way to determine if an example is currently executing a context hook is to look at it's inspect string. Here, we add a simple boolean flag which is triggered in before and after context hook execution. This is needed by rspec/rspec-rails#1501.
ef765b7
to
e537d91
Compare
I went back to the basic version and tried to rebase it, along with a merge from master. I'm not used to working on larger repos like this, so let me know if I did something I shouldn't have. Expect these to pass this time, though I'll keep an eye on them. Edit: Oh jeez, that destroyed the PR view :-( Should I just extract the specs into a new branch and resubmit a new PR? |
Ok I remade this PR to clean up the git issues I introduced: #2132 |
Thanks :) |
The Runner.trap_interrupt method was missing a spec. My goal is to extend interrupt handling per tenderlove's feature request rspec/rspec#13, but I wanted to make sure there was a spec in place first.