Skip to content
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

"warning: global variable `$ERROR_INFO' not initialized" #400

Merged
merged 2 commits into from
Aug 27, 2015
Merged

"warning: global variable `$ERROR_INFO' not initialized" #400

merged 2 commits into from
Aug 27, 2015

Conversation

amatsuda
Copy link
Member

This eliminates a Ruby ⚠️ when run with RUBYOPT="-w".

You can easily reproduce the warning as follows:
% ruby -w -rsimplecov -e 'SimpleCov.pid = Process.pid'

@xaviershay
Copy link
Collaborator

Where is this defined in the first place? Makes me nervous that we're papering over a bigger issue...

@amatsuda
Copy link
Member Author

Indeed. I'm sorry that I didn't care about that...

I found that $ERROR_INFO was defined here in the stdlib English.rb as just an alias to $!.

% ruby -e '1/0 rescue p $ERROR_INFO'
nil

% ruby -rEnglish -e '1/0 rescue p $ERROR_INFO'
#<ZeroDivisionError: divided by 0>

Updated the code to check $!. I think the code looks OK with the existing comment.

@xaviershay
Copy link
Collaborator

LGTM, not sure why the build is failing :(

@amatsuda
Copy link
Member Author

Haven't tracked down deeper yet, but aruba 0.8.x seems broken.
Bundling down aruba to 0.7.4 fixes the problem on my local environment.

@@ -47,7 +47,7 @@
# If we are in a different process than called start, don't interfere.
next if SimpleCov.pid != Process.pid

if $ERROR_INFO # was an exception thrown?
if $! # was an exception thrown?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should instead require 'English' at the top of the file?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working on another app and got this failure, and it was sufficient to do

$ERROR_INFO ||= $!
if $ERROR_INFO.....

This is important because it is also used below

@exit_status = $ERROR_INFO.is_a?(SystemExit) ? $ERROR_INFO.status : SimpleCov::ExitCodes::EXCEPTION

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bf4 Thank you for the hint! I wasn't aware of the other $ERROR_INFO here right after the first emergence 😲
And changing all $ERROR_INFOs to $! made the build green finally!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great, so rebase and merge?

I have a patch in my library now to add the alias :(

@craiglittle
Copy link
Contributor

Hey there! I stopped by to offer a similar patch.

As @bf4 suggested, it seems like the better fix would be to require 'english' instead of assuming it's been done by another library, which will allow us to keep the readability of the English alias.

As @amatsuda pointed out, if we want to get the build running as soon as possible, locking aruba to the v0.7.x series will at least get the build running again, and then debugging to get on the v0.8.x series could be done as a separate step.

@amatsuda
Copy link
Member Author

@bf4 @craiglittle Thank you for the suggestion, but to me $ERROR_INFO doesn't really add no more "readability" over # was an exception thrown?.
Requiring 'English' would add 25 global variables. I don't want to see such a "side effect" just in order to check $! value.
$! is a relatively popular one among these Perl-ish magical gvars, so I think it's fine here with the comment.

@xaviershay
Copy link
Collaborator

My two cents: $ERROR_INFO is more confusing. It's obvious that $! is a special ruby global so is populated by magic, the comment explains. $ERROR_INFO feels like it was defined by us somewhere.

@craiglittle
Copy link
Contributor

Fair enough, works for me!

@amatsuda Were you going to get the build green by locking down to the v0.7.x series of aruba?

@bf4
Copy link
Collaborator

bf4 commented Jul 30, 2015

I'll merge once this is green. Thanks @amatsuda ! (and everyone else commenting)

@craiglittle
Copy link
Contributor

@amatsuda Looks like there's a different issue now, eh?

@amatsuda
Copy link
Member Author

@craiglittle Yeah, this is weird. I cannot reproduce the failure on my local machine... 😟

@bf4
Copy link
Collaborator

bf4 commented Aug 23, 2015

I'm working on another app and got this failure, and it was sufficient to do

$ERROR_INFO ||= $!
if $ERROR_INFO.....

This is important because it is also used below

@exit_status = $ERROR_INFO.is_a?(SystemExit) ? $ERROR_INFO.status : SimpleCov::ExitCodes::EXCEPTION

Would that make people happy?

"$ERROR_INFO is more confusing. It's obvious that $! is a special ruby global so is populated by magic,
the comment explains. $ERROR_INFO feels like it was defined by us somewhere."
#400 (comment)
amatsuda added a commit that referenced this pull request Aug 27, 2015
"warning: global variable `$ERROR_INFO' not initialized"
@amatsuda amatsuda merged commit 0d85bd0 into simplecov-ruby:master Aug 27, 2015
@amatsuda amatsuda deleted the warning branch August 27, 2015 06:18
@bf4
Copy link
Collaborator

bf4 commented Aug 27, 2015

Alright! Let's update the CHANGELOG and we can release 0.10.1!

@craiglittle
Copy link
Contributor

We're so close to the finish line. Looking forward to getting this warning out of my build!

@kdawgwilk
Copy link

👍 to see this released!

@bf4
Copy link
Collaborator

bf4 commented Sep 30, 2015

Would someone mind helping update the CHANGELOG? I am elsewhere

jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Dec 2, 2015
0.11.0 2015-11-29 ([changes](simplecov-ruby/simplecov@v0.10.0...v0.10.11))
=================

## Enhancements

* Added `SimpleCov.minimum_coverage_by_file` for per-file coverage thresholds. See [#392](simplecov-ruby/simplecov#392) (thanks @ptashman)
* Added `track_files` configuration option to specify a glob to always include in coverage results, whether or not those files are required. See [#422](simplecov-ruby/simplecov#422) (thanks @hugopeixoto)
* Speed up `root_filter` by an order of magnitude. See [#396](simplecov-ruby/simplecov#396) (thanks @raszi)

## Bugfixes

* Fix warning about global variable `$ERROR_INFO`. See [#400](simplecov-ruby/simplecov#400) (thanks @amatsuda)
* Actually recurse upward looking for `.simplecov`, as claimed by the documentation, rather than only the working directory. See [#423](simplecov-ruby/simplecov#423) (thanks @alexdowad)
craiglittle pushed a commit to craiglittle/simplecov that referenced this pull request Feb 13, 2016
"$ERROR_INFO is more confusing. It's obvious that $! is a special ruby global so is populated by magic,
the comment explains. $ERROR_INFO feels like it was defined by us somewhere."
simplecov-ruby#400 (comment)
@ShamoX
Copy link

ShamoX commented May 14, 2018

Hi,

I'm having back this warning :

$ bundle exec rspec
....
Randomized with seed 8732

*****/vendor/bundle/ruby/2.5.0/gems/simplecov-0.16.1/lib/simplecov.rb:177: warning: global variable `$ERROR_INFO' not initialized
Coverage report generated for RSpec ******

It seems that the English library doesn't declare it (I guess I'm wrong, but: https://github.com/rubyworks/english/search?utf8=✓&q=%24ERROR_INFO&type=).

I happens to me since I've included dry-validation-matcher.
But I checked in a clean environment:

$ bundle exec pry
[1] pry(main)> require 'english'
=> true
[2] pry(main)> defined? $ERROR_INFO
=> nil
[3] pry(main)> defined? $!
=> "global-variable"
[4] pry(main)>

Fix proposal

lib/simplecov.rb:177
@exit_exception = defined?($ERROR_INFO) ? $ERROR_INFO : $!

@bf4
Copy link
Collaborator

bf4 commented May 15, 2018

@ShamoX This was fixed nearly three years ago. Either you're on an old version or this is a new issue. 'English' is part of the Ruby standard library.

@simplecov-ruby simplecov-ruby locked as resolved and limited conversation to collaborators May 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants