-
Notifications
You must be signed in to change notification settings - Fork 92
Segmentation fault on Jenkins #7
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
Comments
It fails on JRuby 1.7.4 for this app, also. |
Hey -- Sorry you've run into an issue. What behavior are you seeing under JRuby? Also -- Are you able to try the latest patchlevel of 1.9.3 on MRI? |
Oh, I should have said I also ran it with mri 1.9.3-p448 The JRuby failure might be a red herring. It ran out of memory. That's why I added it as an afterthought. |
Got it. Can you confirm you are able to run with standard SimpleCov and generate one of it's HTML reports? Our code does not do much beyond SimpleCov (just an HTTPS POST). |
I am having the same problem on a Rails project that is using ruby 1.9.3-p448. SimpleCov reports work just fine. |
@ecbypi -- Can you also paste your full output into a comment or Gist? Unfortunately, since our gem is pure Ruby and pretty simple, I don't see how we could be causing a segfault. It feels like it would have to be a bug in SimpleCov or Ruby. Since SimpleCov reports are working okay (for @ecbypi at least), it may be a bug in MRI. (Since the contract as I understand it is that: No pure Ruby code is supposed to trigger a segfault.) |
Found it, submitted a fix PR in #15, unfortunately, had some typos and I'm doing this via the Web UI. |
I can confirm that @bf4's patch fixes the issue for me as well. |
I am a bit confused about this for a couple reasons.
|
@noahd1 Thanks for reviewing this re: reproducing it, apparently the way that the rational (800/22) was output isn't the same as typing (800/22) in irb. My bad. I don't know much about Rationals in ruby. re: the missed change (oops): the failing method for me was specifically "file.covered_percent", so I guess "result.covered_percent.round(2)" is a Float or something valid for rounding. "file.covered_strength" for example, didn't need to be converted to a Float. |
@noahd1 Normally my submission would be higher quality and I'd rebase it.. but for reasons I only have access to the github web UI right now... I apologize for that. |
Looks like the number that blows up is actually the rational (200/10) This was reproducible in the context of my test, but not in a regular session covered_percent = Marshal.load("\x04\bU:\rRational[\ai\x01\xC8i\x0F")
covered_percent.round(2) The YAML dump is |
I'll follow up on this at home tonight to track down which gem needs to be required to cause the segfault when running the above code. It's something in my test environment after rails loads. |
Thanks Ben. Sounds worthwhile. Otherwise we could end up inadvertently reproducing this behavior later. |
For a covered percent of Rational(200/10) covered_percent = Marshal.load("\x04\bU:\rRational[\ai\x01\xC8i\x0F") covered_percent.round(2) => segfault Float(covered_percent).round(2) => 20.0 The segfault depends on as yet unknown gems to be required
Simplecov supports merging coverage from multiple test suites by writing them at different command names in the resultset file, then merging them when building the report. We were getting this for free pre-1.0 because the resultsets would be merged before being passed to the custom formatter. Post-1.0 we were (mistakenly) assuming it'd just be one command name in the JSON file, hence we were only reading: SimpleCov::Result.new(results.values.fetch(0).fetch("coverage")) discarding all but the first command-name. This is both broken and more work than we need to do. The file has a well-defined schema, and we can rely more on Simplcov for parsing it. Something like: SimpleCov::Result.from_hash(results.first) would've been functionally equivalent, and shows more clearly the bug. What we really want is: results.map do |k, v| SimpleCov::Result.from_hash(k => v) end Then with an array of results, we can in-line some of Simplecov::ResultMerger to merge them together into a single Simplecov::Result that we'll format for upload. Unfortunately, we do have to in-line the method we need because this object has multiple responsibilities (yay!), including caching and re-writing files on disk, which we _don't_ want. Fixes #7
Simplecov supports merging coverage from multiple test suites by writing them at different command names in the resultset file, then merging them when building the report. We were getting this for free pre-1.0 because the resultsets would be merged before being passed to the custom formatter. Post-1.0 we were (mistakenly) assuming it'd just be one command name in the JSON file, hence we were only reading: SimpleCov::Result.new(results.values.fetch(0).fetch("coverage")) discarding all but the first command-name. This is both broken and more work than we need to do. The file has a well-defined schema, and we can rely more on Simplcov for parsing it. Something like: SimpleCov::Result.from_hash(results.first) would've been functionally equivalent, and shows more clearly the bug. What we really want is: results.map do |k, v| SimpleCov::Result.from_hash(k => v) end Then with an array of results, we can in-line some of Simplecov::ResultMerger to merge them together into a single Simplecov::Result that we'll format for upload. Unfortunately, we do have to in-line the method we need because this object has multiple responsibilities (yay!), including caching and re-writing files on disk, which we _don't_ want. Fixes #7
full gist
jenkins shell commands are
Where rake ci does stuff and calls ENV['CODECLIMATE_REPO_TOKEN']='itsasecret' then
Rake::Task['ci:spec'].invoke(*args)
The text was updated successfully, but these errors were encountered: