codeclimate / ruby-test-reporter

DEPRECATED Uploads Ruby test coverage data to Code Climate
https://codeclimate.com
Other
92 stars 92 forks source link

CodeClimate no longer merges the results from different commands (regression in v1.0+) #162

Closed glebm closed 7 years ago

glebm commented 7 years ago

SimpleCov: coverage.zip

All Files (93.98% covered at 1245.21 hits/line) 70 files in total. 2393 relevant lines. 2249 lines covered and 144 lines missed

CodeClimate: github/glebm/i18n-tasks/coverage

43.19% Test Coverage

Travis: glebm/i18n-tasks/jobs/186878995

Coverage report generated for RSpec, bin/i18n-tasks, bin/i18n-tasks --version to /home/travis/build/glebm/i18n-tasks/coverage. 2243 / 2387 LOC (93.97%)** covered. ... I, [2016-12-27T01:38:32.942710 #4441] INFO -- : Reporting coverage data to Code Climate. Sending report to https://codeclimate.com for branch master... done. SimpleCov failed to recognize the test framework and/or suite used. Please specify manually using SimpleCov.command_name 'Unit Tests'. Coverage report generated for RSpec, Unknown Test Framework, bin/i18n-tasks, bin/i18n-tasks --version to /home/travis/build/glebm/i18n-tasks/coverage. 2243 / 2243 LOC (100.0%) covered.

pbrisbin commented 7 years ago

It looks like your Travis build performed three steps:

bundle exec rspec --format d
bundle exec rubocop
bundle exec codeclimate-test-reporter

If the middle step (rubocop) generated a coverage report, that's what would be uploaded, and could explain the discrepancy. If the coverage from the the rspec step is what you would like to upload to Code Climate, can you try re-ordering the steps:

bundle exec rspec --format d
bundle exec codeclimate-test-reporter
bundle exec rubocop

And see if that corrects the issue?

pbrisbin commented 7 years ago

Note: it doesn't make much sense that running rubocop would alter or generate a coverage report. Just trying to rule it out.

glebm commented 7 years ago

@pbrisbin Done, hasn't made a difference

pbrisbin commented 7 years ago

Hmm OK, we'll have to dig a bit deeper. Would it be possible for you to grab the coverage/.resultset.json file, just before you run the reporter? Maybe pushing it to a public pastebin from within the travis job? Showing the output of ls coverage/ at that point might be useful too.

pbrisbin commented 7 years ago

Ah, nevermind that. I see why.

We expect all the coverage data to be under a single RSpec key in the coverage report. But yours has other keys, like ../../bin/i18n-tasks. I'll have to do some investigation to see what causes those other keys and how we should handle it. Sorry for the trouble.

pbrisbin commented 7 years ago

It looks like https://github.com/glebm/i18n-tasks/blob/30525288384efb7e7d6e235712a887cbee10aa2f/spec/bin_simplecov_helper.rb#L5 is causing this. I'm not sure if we should just flatten all the coverage data from all keys in the resultset, or that would cause other problems in other use cases. I may not get to a fix here until after the new year, but that at least explains the behavior.

glebm commented 7 years ago

Would it be possible for you to grab the coverage/.resultset.json file, just before you run the reporter?

@pbrisbin It's attached in the original post (inside of the coverage.zip file).

We expect all the coverage data to be under a single RSpec key in the coverage report. I'm not sure if we should just flatten all the coverage data from all keys in the resultset, or that would cause other problems in other use cases.

This is not a good assumption because most projects that have e2e tests have a different command name for them (binary name, selenium, etc).

I think that this is not how it used to work either: pre-v1.0 the reporter used to merge the results because it let SimpleCov pass the results into its formatter. SimpleCov, by default, merges the coverage results before passing them to the formatter. SimpleCov v1.0+ passes the raw results instead, causing this issue.

If you want to do the merging server-side instead, you can use the merged_results method, defined here.

It looks like https://github.com/glebm/i18n-tasks/blob/30525288384efb7e7d6e235712a887cbee10aa2f/spec/bin_simplecov_helper.rb#L5 is causing this

Yes, changing this to command_name 'RSpec' works around the issue.

maxjacobson commented 7 years ago

@glebm Hey Gleb, thanks for your helpful report here. @pbrisbin made this fixto the test reporter and I've just shipped it as v1.0.4. I believe this should resolve your issue here, but please let me know if you have any additional feedback!

glebm commented 7 years ago

@pbrisbin @maxjacobson Fix confirmed working! Thank you for a quick resolution! 😃