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

Fix crash when parsing previous version resultset.json #820

Closed
PragTob opened this issue Jan 12, 2020 · 3 comments
Closed

Fix crash when parsing previous version resultset.json #820

PragTob opened this issue Jan 12, 2020 · 3 comments

Comments

@PragTob
Copy link
Collaborator

PragTob commented Jan 12, 2020

We changed our file format with the 0.18.0 release series - unfortunately our code still seems to want to read the old file format and then break ;) We gotta be more careful there. I don't think we need to be able to read all old versions of files but I think we should rather capture the error and continue without processing the old results (easiest to achieve I believe).

This is my preliminary diagnosis, problems goes away when deleting the coverage directory.
Reported by @klyonrad #781 (comment)

bundle show simplecov
/Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1

bundle show simplecov-html
/Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-html-0.11.0.beta1

bundle exec rspec spec/policies
Run options: exclude {:future=>true}                                                                                 

Randomized with seed 25469
 237/237 |========================================== 100 ==========================================>| Time: 00:00:23 

Finished in 23.37 seconds (files took 10.5 seconds to load)
237 examples, 0 failures

Randomized with seed 25469
Traceback (most recent call last):
	15: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/defaults.rb:29:in `block in <top (required)>'
	14: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov.rb:164:in `run_exit_tasks!'
	13: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/configuration.rb:196:in `block in at_exit'
	12: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov.rb:74:in `result'
	11: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/result_merger.rb:83:in `merged_result'
	10: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/result_merger.rb:57:in `results'
	 9: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/result_merger.rb:57:in `each'
	 8: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/result_merger.rb:58:in `block in results'
	 7: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/result.rb:72:in `from_hash'
	 6: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/result.rb:86:in `symbolize_names_of_coverage_results'
	 5: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/result.rb:86:in `each_with_object'
	 4: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/result.rb:86:in `each'
	 3: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/result.rb:87:in `block in symbolize_names_of_coverage_results'
	 2: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/result.rb:87:in `each_with_object'
	 1: from /Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/result.rb:87:in `each'
/Users/username/.rvm/gems/ruby-2.6.5/gems/simplecov-0.18.0.beta1/lib/simplecov/result.rb:88:in `block (2 levels) in symbolize_names_of_coverage_results': undefined method `to_sym' for 1:Integer (NoMethodError)
Did you mean?  to_s

#781

@PragTob
Copy link
Collaborator Author

PragTob commented Jan 15, 2020

fix-crash-against-old-resultset-json has a scenario aimed at reproducing this.

Right now it's incomplete because of:

     if use_merging
        wait_for_other_processes
        SimpleCov::ResultMerger.store_result(@result) if result?
        @result = SimpleCov::ResultMerger.merged_result
      end

If store_result is executed before the result merging then this bug doesn't trigger. So gotta make result? be false in the scenario :)

@PragTob
Copy link
Collaborator Author

PragTob commented Jan 15, 2020

WIP fix/reproduction is up at #824

PragTob added a commit that referenced this issue Jan 16, 2020
Ensure a different name so merging doesn't just throw it out

ResultMerger#store_result:

          clear_resultset
          new_set = resultset
          command_name, data = result.to_hash.first

it might throw out the result, it might not 😱
PragTob added a commit that referenced this issue Jan 17, 2020
Ensure a different name so merging doesn't just throw it out

ResultMerger#store_result:

          clear_resultset
          new_set = resultset
          command_name, data = result.to_hash.first

it might throw out the result, it might not 😱
PragTob added a commit that referenced this issue Jan 18, 2020
Ensure a different name so merging doesn't just throw it out

ResultMerger#store_result:

          clear_resultset
          new_set = resultset
          command_name, data = result.to_hash.first

it might throw out the result, it might not 😱
@PragTob
Copy link
Collaborator Author

PragTob commented Jan 19, 2020

Fixed in #824

@PragTob PragTob closed this as completed Jan 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant