codeclimate / ruby-test-reporter

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

Lock simplecov between 0.10 and 0.13 version #181

Closed bliof closed 7 years ago

bliof commented 7 years ago

Done because the 0.14.0 version introduces some changes that break the CodeClimate::TestReporter::Formatter#merge_results https://github.com/colszowka/simplecov/commit/a7747c1391b7f8d0b7081c23df79b151ebbc95bd

Check also:

bliof commented 7 years ago

@GordonDiggs follow-up from https://github.com/codeclimate/ruby-test-reporter/issues/180

FranklinYu commented 7 years ago

Will v1.1.x branch in the future support SimpleCov v0.14.x?

colszowka commented 7 years ago

Hey there, It'd be very nice if this bug could be actually fixed instead of pinning to an outdated version of SimpleCov :heart:

The underlying change in SimpleCov is the replacement of Hash and Array core extensions with utility methods that do this, see the related PR that introduced the change It should be fairly easy to change this gem's code to use the new API, swapping the old call to original_result.merge_resultset (although I am not quite sure why this gem needs to deal with resultset merging in the first place, as this is a low-level functionality of simplecov and I'd expect code climate would only need to consume the final (merged) result)

As a side note, this issue only arises when running multiple test suites (i.e. rspec and cucumber). On an app I am currently working on, we had no problems running codeclimate with SimpleCov 0.14.

ale7714 commented 7 years ago

Hi @colszowka, thank you for your feedback! Currently we're working on a new unified test reporter that will support multiple coverage report formats and parallelized reporting. This new reporter don't rely on low-level functionality of SimpleCov and instead consumes the final result.

We're focusing most of our development efforts on this new tool. It's currently on alpha and we're currently using it with the last version of SimpleCov successfully. I recommend users of SimpleCov v0.14.x to review and try our new test reporter.

FranklinYu commented 7 years ago

@ale7714 The manual page indicates that the unified reporter gets the data from ./coverage/.resultset.json in Ruby projects. Would that JSON format change over time? Should we pin the SimpleCov version? We don't need to worry about that if we're using a gem.

ale7714 commented 7 years ago

@franklinyu that's correct. We're parsing .resulset.json from SimpleCov to a Code Climate payload. For now, it's not needed to pin SimpleCov to a particular version. If we find some issue it require to use a particular version, we will update our docs. Let me know if you have any other question, also feel free to open issues on the test reporter repo if you find any because we're still on an alpha stage.

colszowka commented 7 years ago

Thanks for the pointer @ale7714, I will try out the new reporter.

I would not advise towards using the resultset.json file since it's an internal cache only meant for simplecov's merging mechanism and you're therefore betting yourselves on an internal data structure. I also would really not like pinning to a particular version of simplecov again later down the road when the approach of using an internal cache file turns out to fail.

I would strongly recommend that you build your own simplecov formatter (it's easy). I don't know if it's fully functional, but https://github.com/vicentllongo/simplecov-json might be a good starting point. This way you can vendorize your dependency on json output from simplecov and guard your customers against pinning potentially buggy and outdated versions.

Disclaimer: I'm the author of simplecov.

FranklinYu commented 7 years ago

I agree with @colszowka. Depending on private interface is always fragile, and I don't think customers would be happy about that. There may be a time gap between SimpleCov release a new version and your team update the documentation; if a customer updates his SimpleCov during this gap, he/she is under risk that he/she can't be aware of. Since the unified reporter is still in early stage (there's only a single alpha release), it's time to make the switch before it's too late.

ale7714 commented 7 years ago

@colszowka thank you for your input! I appreciate it. I will review more about implementing our own simplecov formatter. I will bring your recommendations to the team.

FranklinYu commented 7 years ago

The current (low-level) workflow of the unified reporter, for non-parallel build, is

./cc-test-reporter format-coverage
./cc-test-reporter upload-coverage

It seems easy to just replace the first step with a Rake task provided by cc-simplecov-json (the imaginary formatter name):

bundle exec rake cc:coverage:format
./cc-test-reporter upload-coverage