test-reporter: Possible Incompatibility with SimpleCov 0.18+
Hi there,
SimpleCov maintainer here.
To the best of my knowledge you’re using resultset.json
to do the code coverage reporting to code climate. Due to the introduction of branch coverage the format of this file will change slightly in the next release (beta release just released) - namely there are now top level “lines” and “branches” keys.
However, the file format might be changed further in the future due to problems with the standard branch format (we might even switch to YML format or something else). See https://github.com/colszowka/simplecov/issues/801 for some information.
I don’t want to break code climate test reporting, however:
a.) we need these changes to work correctly b.) resultset.json was never intended or communicated as an integration point - it’s merely the way in which we do merging results from different processes
As a result of this, I can’t guarantee stability for resultset.json
, furthermore as it pretty much just dumps and merges the standard coverage data it also doesn’t support a lot of features of simplecov (off the top of my hat):
- ignoring coverage through nocov #246
- filtering out/ignoring files (all the different ways in which simplecov allows you to do this)
- tracking files/adding files that weren’t loaded
- I’m not 100% sure if we write the file again after the last process finished writing, might be related to #385
My suggested approach would be to write a gem/formatter that writes a file in a format that you control and can continue working. (I know this is against the general “no integration needed” idea of this project) I’m happy to work with you to ensure that the APIs you need are stable. I also still have hopes (🤞) to release a 1.0 version soon-ish (couple of months) and to make another pass through the code to make sure that public vs. private APIs in the code are accurately documented. Happy to provide you with more public APIs if you need them.
Cheers and happy new year, Tobi
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Reactions: 54
- Comments: 84 (21 by maintainers)
Commits related to this issue
- Pin to older simplecov https://github.com/codeclimate/test-reporter/issues/413 — committed to fnordfish/teckel by fnordfish 4 years ago
- Pin simplecov to 0.17.1 Because codeclimate doesn't support simplecov 0.18 https://github.com/codeclimate/test-reporter/issues/413 — committed to sul-dlss/workflow-server-rails by jcoyne 4 years ago
- Pin simplecov to 0.17.1 Because codeclimate doesn't support simplecov 0.18 codeclimate/test-reporter#413 — committed to sul-dlss/sdr-api by jcoyne 4 years ago
- Lock simplecov to last supported cc version https://github.com/codeclimate/test-reporter/issues/413 — committed to hashrocket/devise_token_auth by briandunn 4 years ago
- Pin simplecov version https://github.com/codeclimate/test-reporter/issues/413 — committed to hakanensari/peddler by hakanensari 4 years ago
- Amend 0.18 changelog with notice about code climate test reporter I didn't add this information as to my mind we didn't change any API documented anywhere and hence it shouldn't appear in a Changelog... — committed to simplecov-ruby/simplecov by PragTob 4 years ago
- Amend 0.18 changelog with notice about code climate test reporter I didn't add this information as to my mind we didn't change any API documented anywhere and hence it shouldn't appear in a Changelog... — committed to simplecov-ruby/simplecov by PragTob 4 years ago
- Pin simplecov https://github.com/codeclimate/test-reporter/issues/413 — committed to hakanensari/vacuum by hakanensari 4 years ago
- Rollback simplecov due to issues on codeclimate report tool https://github.com/codeclimate/test-reporter/issues/413 — committed to gmmcal/gmmcal.com.br by gmmcal 4 years ago
- Fix the simplecov version because cc-test-reporter does not support simplecov v0.18.x now. See: https://github.com/codeclimate/test-reporter/issues/413 — committed to ryz310/pr_comet by deleted user 4 years ago
- Fix the simplecov version because cc-test-reporter does not support simplecov v0.18.x now. See: https://github.com/codeclimate/test-reporter/issues/413 — committed to ryz310/gem_comet by deleted user 4 years ago
- downgrades simplecov because of https://github.com/codeclimate/test-reporter/issues/413 — committed to tansengming/stripe-rails by tansengming 4 years ago
- Downgrade simplecov SimpleCov 0.18 is incompatible with the Code Climate reporter For more info: https://github.com/codeclimate/test-reporter/issues/413 — committed to rafasoares/human_enum by rafasoares 4 years ago
- Setting up GitHub workflows (#7) * Add CI workflow * Fix CI workflow triggers * Update ci.yml * Fix CI steps * Install proper bundler version * Update Gemfile.lock * Merge jobs *... — committed to rafasoares/human_enum by rafasoares 4 years ago
- Downgrade simplecov This is imcompatible with the latests code climate reporter. The version is capped until this is fixed. https://github.com/codeclimate/test-reporter/issues/413 — committed to colinpetruno/portunus by colinpetruno 4 years ago
- Fixing code climate version But this is because of https://github.com/codeclimate/test-reporter/issues/413 — committed to agile-alliance-brazil/aa-service-bus by hugocorbucci 4 years ago
- [Snyk] Security upgrade simplecov from 0.16.1 to 0.16.1 (#15) * fix: Gemfile & Gemfile.lock to reduce vulnerabilities The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vu... — committed to agile-alliance-brazil/aa-service-bus by snyk-bot 4 years ago
- Rollback simplecov to fix CC incompatiblity CodeClimate isn't compatible with simplecov 0.18.x: https://github.com/codeclimate/test-reporter/issues/413 Even 0.17 was causing errors: https://jenkin... — committed to cyberark/conjur by jonahx 4 years ago
Hi y’all,
a lot of the delay (since late September) is my fault, been going through a rough time and barely touched OSS save for one small bugfix release.
I just released SimpleCov 0.20.0 which includes the default JSON formatter that should also be triggered automatically when run in a CodeClimate environment thanks to @fede-moya . https://github.com/simplecov-ruby/simplecov/blob/main/CHANGELOG.md#0200-2020-11-29
I’m not sure if the test reporter has already been updated to deal with the new format. In theory the work on the SimpleCov side is done now and all that needs updating is the test reporter and potentially the JSON formatter. Or so I hope, the way these things go I expect at least one bugfix release 😛
@efueger can you say if it’s already fixed/supposed to be working?
So sorry everyone, hope it works now - all the best to y’all, Tobi
Yo, sorry I had a couple of… interesting months with almost no OSS work done. Sorry y’all this time the delay is entirely my fault. Also, thanks for bumping.
@ale7714
id
as it doesn’t have much value, but it’s hard to tell. It might be better to use a JSON object there so we have keys that give names. Would significantly increase the readability of the format but alas require much more space 😃 (although if gzipped that would likely mostly go away)rest looks good as far as I can see, thanks!
Hi there,
Simplecov 0.18.0 is released and we could not use it as it leads to an error with
cc-test-reporter
:@tvdeyen thanks, and sorry it took so long. Also thanks for sharing that it really does work for people now that makes me very happy 💚 Thanks y’all for the patience, and @fede-moya and @ale7714 for the work! Hope this will keep working 😁
@PragTob no worries very understandable with the current world situation. Thank you for the feedback! Next steps is for us to open up a PR on SimpleCov to add support for this JSON formatter. We’re doing our planning for July and we will prioritize opening the PR on SimpleCov and updating the
test-reporter
.Hi @PragTob,
we imagined that you were busy. Thank you so much for still keeping this in the queue and finding some time to tackle it, it will be awesome to see it working before the end of the year.
So, we do need to update test reporter version but changes needed have already been coded. It would just be a new release in theory. The plan is for that new release to occur ASAP.
Thanks again, and we will came back here after we release a new version of the test reporter.
Quick update from Code Climate
We’ve mapped out a plan to have SimpleCov output a json file that works with the Test Reporter.
We’ll start with:
Thanks to @PragTob 👍
Bump any updates?
@sergioisidoro 👋
That is one of my suggested resolutions in the initial post in this issue. It used to work this way: https://github.com/codeclimate/ruby-test-reporter
However, at a point in time Code Climate did a survey of tools where they determined all of them write out some kind of format and that it’d be better to just read this format and no we’re here with the tool we’re reporting on. See this comment in this issue.
It’d still be my preferred solution, but Code Climate doesn’t want to go that route (which I partly understand, once you made that decision and “everything just works” it’d be weird for ruby to be the only one with custom setup instructions).
What we have now is almost similar though, just that the formatter will live inside simplecov and will automatically be run when inside a CodeClimate environment. This has the drawbacks that in the end maintenance will fall to SimpleCov/me, it’s more code for everyone else to download and more complexity for simplecov. On the other hand you can say, it’d be nice to have an official JSON formatter so 🤷♂️
At the end of the day, I just want things to work for everyone and I know a lot of people rely on CodeClimate.
I wrote a new gem that uses the latest
gem "simplecov", "~> 0.21"
, successfully with CodeClimate… and CodeCov, and Coveralls, and CodeCoverageSummary.Here’s how I did it!
setup-code-climate
Github Action I useThere is no longer a compatibility issue.
I wrote a more complete article about the setup on dev.to
For my project this issue got fixed by this PR: https://github.com/david942j/one_gadget/pull/191
bundle exec rake
) AND the codeclimate action.Just read about the update, great news. Would it be possible to get a shorter feedback loop on this, as we have roughly a few hundred projects in our paid setup that depend on this and can’t be upgraded to use the branching features in SimpleCov 0.18.
Hi folks,
I think we have solved this generally. I know it doesn’t work for some folks, I think you can still ask here or open other issues but I’ll mark this closed as form my POV it’s out and it seems to work generally ✌️
@PragTob Thank you so much for working on this! 👏🏻
I added bare support for simplecov 0.18 in #420 in case someone wants to give it a try.
@rhymes we know. It doesn’t. If it did it’d be in the changelog and I’d post it here 😃
@sgnn7 that is correct. See my initial post. The problem though is that the current “integration” also already suffers from multiple problems (such as ignored lines not working) and that the file used is internal and it’s format might change soon - even drastically (it’s an option that it becomes a YAML due to problems encountered with the branch format in JSON).
Sure CodeClimate could patch it up, but it’s not the most long term stable solution. We’ll see what we can up with, if anything.
@fede-moya I just want to add my two cents as an user of both tools:
I wouldn’t mind setting the formatter via environment variable myself in my CI environment (but not a CC-specific one, something more generic like
SIMPLECOV_FORMATTER
or similar), especially considering that at the moment we need to do additional setup or workarounds so the integration works at all.B.1* seems unnecessary to me and I’m with @PragTob on that.
As a side note: In my use case, I run the tests and upload the coverage results as two separate jobs, sharing the coverage report as an artifact between them. So I wouldn’t have any CC-related variables in the environment during my test run anyway.
Thank you for the note:
It’s exactly what I did. Creating some simple rake task that fetches
lines
keys from json and makes a legacy json file for cc reporter to consume is fairly trivial. One could easily do that while support is added if using pre 0.18 version is not an option.Hi there,
2 weeks have gone by. Would it be possible to get an update on the progress? Is there anything we can help with? (eg, testing)
@pboling
simplecov
now includes a JSON formatter, imported with:From my experience, CodeClimate basically stopped development on their Quality product a couple years ago. Last I heard from them, about a year ago, I had sent in an email basically saying “What’s going on? Everything’s broken. Will it be fixed?” and they never got back to me, even with followups. So I wouldn’t count on this ever being fixed.
@pboling What was the decision for you? I also still have this issue with simplecov 0.21.2 and Code Climate Test Reporter 0.9.0,
CC_TEST_REPORTER_ID
is set for all the steps.if anyone’s interested, I coded up a GitHub Action amancevice/setup-code-climate that I find slightly easier to use than the one above, especially if you’re coming from something like Travis.
Oh I see. Thanks! It worked!
https://github.com/pboling/rspec-pending_for/runs/1598076801?check_suite_focus=true
I’m going to try switching back to a packaged
action
now.Hi @fede-moya The current version of the CodeClimate test reporter says it is compatible with Simplecov 0.20.0. But the documentation still says
SimpleCov 0.18+ not yet supported
with a link to this issue.What is still missing?
@PragTob Thanks for the super fast feedback !. 👏🏼
A.2. Seems like you are thinking on increasing the decoupling degree of
simplecov
’s internals. Guess it’s something we would need to keep an eye on in the future so oursimplecov-json
gem stays up to date.Regarding B.1*, yes I’m aware of your position about having codeclimate’s specific code in your gem, we think it’s a very kind gesture of yours to allow that. Ideally the code would be minimal like
|| ["CC_TEST_REPORTER_ID"].present?
.Thanks again, we will be opening a PR on
simplecov
’s repo soon.@PragTob Hello there !, Federico from CodeClimate’s staff. Started to work on this last week, I think there are two points that haven’t reached consensus yet, being:
CC_TEST_REPORTER_ID
is present.Having internal discussions here at codeclimate we agree that going with A.2 and B.1 along with B.1* would be a great solution for us. What do you think ?
I’m already working on the JSON formatter, I hope to end with that soon, then work on integrating that new JSON formatter into simplecov. Let us know if you have feedback.
What’s the timeline for delivering #420 for general use?
@coding-bunny what @deivid-rodriguez said but that’s restoring functionality, not providing branch coverage statistics for code climate (which is totally different and I left out).
Aside from what Deivid wrote, the different between the files is very little. Theoretically you could run a script after your tests that converts the
resultset.json
back into the old format and thereby have code climate work.I’m sorry but that’s not really an ideal solution. We are paying to run Codeclimate to run for our 100+ repositories and right now we can use the full capabilities because of this. I’d really expect more urgency to have been put behind this from the side of Codeclimate.
Is there at least a work around we could use to make v0.18 work, be it with different format output or anything?
@sergioisidoro Emily sent me an email the other day indicating they haven’t forgotten about this but still need to get to it. So it should happen, but I’d imagine other things like coordinating remote work or what not are a priority right now.
Also for everyone, this will likely still take some time with the proposed plan of action:
I guess after both 3 & 4 are done you could run both from forks/branches/master.
Point being, even if we got the proposal of a JSON format and I’d just agree to it, it’d be unrealistic that this would be fixed by next week.
Can only add my voice to @bmulholland; We canceled our subscription with CodeClimate because of this issue and build our own code analyzer and tooling to run the things like RuboCop, Code Coverage etc.
@pboling In your “Run tests” step of your GitHub Action, the final log entry is:
What is missing here is another line that says “JSON Coverage report generated…” (I can’t recall the exact text).
It looks like this step doesn’t have the
CC_TEST_REPORTER_ID
environment variable set. It looks like you’re only setting this in the “Install CodeClimate” and “Upload test coverage” steps. So SimpleCov isn’t aware that it needs to also generate a JSON file that is compatible with CodeClimate.Hi @fede-moya! As I mentioned in the update of the last post, I only had to add the JSON formatter to the list of formatters I was using. This change would produce a
coverage.json
file that is well parsed by the test-reporter@PragTob @runephilosof-abtion @fede-moya I can confirm it is working
https://github.com/AlchemyCMS/alchemy_cms/pull/1971
👋 Hi @fede-moya
I thought we had consensus on those (although somewhat grudgingly in parts ;P ) and I think it’s mostly as you summarized so:
A.2: yes, just a note here that I tentatively want to change the repo structure to a more mono repo style (as the tests are really interdependent, sadly the html tests run inside rimplsecov proper but hey) and also the gem structure so that there’s a
simplecov
meta gem that does what it currently does but also a new “simplecov-core” gem without any of the formatters that formatters could then rely on (andsimplecov
relies on this one). No big implications for you just a heads up. B.1* this one I’m not too happy about as it puts CodeClimate specific code into the general simplecov code base (basically before printing results it needs to be like “Am I simplecov? Ok add JSONFormatter to formatters”), it shouldn’t be a lot of code but it still feels wrong. Nevertheless, I understood how important and “normal” this is to the CodeClimate organization so it’s fine although I’m not exactly excited about it 😉I think using ENV variables as in B.1 would be nice but I’m also fine with people coding that in Ruby that themselves (which imo needs to be there). Configuring it via ENV variables is a nice added feature imo.
Cheers! 🎉
Also a blocking issue in my workflow (using the new
collate
API of SimpleCov 0.18+, so no easy way to pin version< 0.18
).So can’t use CodeClimate for coverage at the moment 😦Merging PR #420 as a temporary fix would be very nice.Temporary dirty fix, see comment below.Hi all,
thanks for the suggestion @ale7714 - I don’t think this format of coverage will work though, for whatever the defintion of “work” is let me detail what I mean below:
Branch Coverage
The proposed format isn’t aware that one line might have multiple branches. So one branch covered and the other not covered in a line would not be detectable. Also, for calculating total coverage right now I don’t see a difference between a line where a branch was hit 1 times and a line with 2 branches where just one was hit one time and the other not.
A branch can also span multiple lines, which might be nice to show for representation. Plus, there’s value in showing what type a branch is.
For comparisons sake here’s the data returned by ruby:
which roughly is
type, _id, start_line, _start_col, end_line, _end_col
- you can see from the_ignored
which one SimpleCov right now actually works with. I’d suggest keeping these. So far SimpleCov only keeps track of the branches themselves (example above:then
andelse
) not the condition above (example:if
). Which should also work for coverage. Hence an array of branch json objects might work.It’s also worth mentioning that some branches exist in the branch coverage but not in code. E.g.
else
branches are optional, but always reported as a branch that was missed or hit.Methods
I’d prefer not to talk about method coverage right now, mostly because I see complexities as the ones described above incoming for them and I can’t know these until I implemented them.
Oneshot Lines
I’d highly prefer to keep the format of oneshot lines and lines similar, because they are the same with the mapping that
0 => false
and>= 1 => true
. So, more like[ true, null, false, true, true ]
Lines
Ok so far (my interpretation being that
null
means no coverage available) but we have to talk about…Ignored lines
Having ignored lines as an extra property is clever as it is cross cutting. However, that’d mean that every “client” of this JSON has to calculate what line is really skipped themselves (also as an overlap with branches and methods as they span multiple lines) - while SImpleCov already computed this. Having clients compute this might also introduce potential differences.
Therefore my proposal would be more to introduce a new coverage value
"ignored"
or something. Yes that’s mixing types in the JSON, but I think I’d prefer it over the alternatives I see right now which would be havingundefined
andnull
but with different meetings or introducing a value like-1
to show this.Hope that’s all half-way clear.
Cheers and thanks, Tobi
Hi all!
Thank you for your patience with this! Apologies for the slow response. Thank you @deivid-rodriguez for opening #420. We really appreciate it.
@PragTob here’s our first proposal for the JSON format:
We’re including the 4 possible methods of coverage. We have also added the
ignore_coverage
key which will solve the existing issue where we’re unable to take into account:nocov:
. Let us know what you think.Hope everyone is staying safe 🛡 ✨
Just so you know, the binary generated by https://github.com/codeclimate/test-reporter/pull/420 worked just fine for me, in case someone has a lot of urgency for using codeclimate with simplecov 0.18.
Hi @PragTob,
Thank you for your response. I appreciate you’re taking the time to discuss this with us. I understand that other tools might do things differently. A couple of years ago, we took the direction of building a test coverage reporter tool that could parse output from different coverage tools with the main goal of having a consistent way to setup coverage reporting with Code Climate across different languages.
Before writing the tool, we performed some research and we were able to identify that most coverage tools offer a way to output a file (in JSON, XML, or similar) with the test coverage data that we would be able to parse.
Yes, users are setting CC_TEST_REPORTER_ID on their environment.
We would love to setup a time to chat with you to discuss how can we partner and support you on this. Please, let us know your availability over email at hello@codeclimate.com or a DM us in Twitter. Looking forward to hearing back from you.