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: instance variable @enable_for_subprocesses not initialized #926

Closed
tagliala opened this issue Sep 7, 2020 · 11 comments · Fixed by #938
Closed

warning: instance variable @enable_for_subprocesses not initialized #926

tagliala opened this issue Sep 7, 2020 · 11 comments · Fixed by #938
Labels

Comments

@tagliala
Copy link
Contributor

tagliala commented Sep 7, 2020

Hi,

I would like to submit a PR by myself but I cannot find time at the moment and since I'm not using the subprocess functionality it would be hard to debug

#881 introduced a warning on variable @enable_for_subprocesses, which is going to be used even when it is not initialized (value is nil).

I don't know if it is correct to use the same or-equal approach as at_exit configuration

You can see the warning here: https://travis-ci.org/github/tagliala/coveralls-ruby-reborn/jobs/724960704

Log Snippet

............./home/travis/build/tagliala/coveralls-ruby-reborn/spec/coveralls/simple_cov/formatter_spec.rb:15: warning: Passing the keyword argument as the last hash parameter is deprecated
/home/travis/build/tagliala/coveralls-ruby-reborn/vendor/bundle/ruby/2.7.0/gems/simplecov-0.19.0/lib/simplecov/result.rb:28: warning: The called method `initialize' is defined here
......./home/travis/build/tagliala/coveralls-ruby-reborn/vendor/bundle/ruby/2.7.0/gems/simplecov-0.19.0/lib/simplecov/configuration.rb:203: warning: instance variable @enable_for_subprocesses not initialized
@PragTob PragTob added the Bug label Sep 7, 2020
@PragTob
Copy link
Collaborator

PragTob commented Sep 7, 2020

👋

Hi there,

thanks for the report. Curious. We're supposed to catch warnings like that in CI but maybe in the CI migration something went wrong. Dang.

@PragTob
Copy link
Collaborator

PragTob commented Sep 7, 2020

yeah emitting warnings is only tested in rspec not in cucumber and the feature was only covered by cukes. Sorry about that :)

@tagliala
Copy link
Contributor Author

tagliala commented Sep 7, 2020

No problems, thanks for the heads-up

Apologies for not having submitted a PR, I think you may know if ||= has been intentionally avoided

@JonJagger
Copy link

Data point... I just got bitten by this too.

@EdwardIII
Copy link

EdwardIII commented Oct 12, 2020

Hey, thanks for all your work on a brilliant useful tool! Can we upgrade to specific version for now to get rid of the warning, or is a fix still in the works?

We see the error in minitest with these gems installed on the project. Happy to try a pre-release test or pitch in with a solution if you can point me in the right direction.

$ bundle install
Using rake 13.0.1
Using concurrent-ruby 1.1.7
Using i18n 1.8.5
Using minitest 5.14.2
Using thread_safe 0.3.6
Using tzinfo 1.2.7
Using zeitwerk 2.4.0
Using activesupport 6.0.3.4
Using ast 2.4.1
Using bundler 2.1.2
Using byebug 11.1.3
Using coderay 1.1.3
Using docile 1.3.2
Using method_source 1.0.0
Using parallel 1.19.2
Using parser 2.7.2.0
Using pry 0.13.1
Using pry-byebug 3.9.0
Using rainbow 3.0.0
Using regexp_parser 1.8.1
Using rexml 3.2.4
Using rubocop-ast 0.7.1
Using ruby-progressbar 1.10.1
Using unicode-display_width 1.7.0
Using rubocop 0.92.0
Using simplecov-html 0.12.3
Using simplecov 0.19.0
Bundle complete! 6 Gemfile dependencies, 27 gems now installed.

task:

Rake::TestTask.new(:test) do |t|
  t.test_files = FileList['test/test_helper.rb', 'test/*_test.rb', 'test/**/*_test.rb']
end
desc 'Run tests'

test_helper.rb:

# frozen_string_literal: true

require 'simplecov'
SimpleCov.start

@PragTob
Copy link
Collaborator

PragTob commented Oct 18, 2020

Hey there,

sorry I haven't managed to dedicate a lot of time to OSS lately. First fix didn't go fast as we had lots of fun with the CI being broken.... aiming for a release some time next week depending on how it goes.

Cheers,
Tobi

@EdwardIII
Copy link

Thanks for the update. Anything I can help with on the CI? I've setup CIs using Jenkins, CodeBuild, gitlab runners etc so might be able to pitch in if that's useful.

@PragTob
Copy link
Collaborator

PragTob commented Oct 20, 2020

@EdwardIII nah but thanks. We just had CI fail randomly on acceptance test related specs which presumably was due to a Chrome upgrade that broke something. See #925

@tagliala
Copy link
Contributor Author

Thanks, also the feature test to make sure that there are no warnings is a great addition

@PragTob
Copy link
Collaborator

PragTob commented Oct 26, 2020

Yeah I always wanted to have something like it... we are already checking it for the rspec unit test (I wrote essentially a small library for this) but the thing wasn't unit tested. I was never sure how to correctly implement it as a feature test as ideally I'd want to check it in all tests (just like rspec) but I realized that one broad high level test running for most of the paths is better than nothing 😁

One of the reasons this took so long, because I wanted to have it covered. Sorry and here's to no more such problems!

@JonJagger
Copy link

Nice :-) Thank you for doing this. I look forward to moving to this on the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants