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

Most upvoted comments

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

  1. I think it’d be better if lines and oneshot_lines had the same basic data structure (aka an array) - they’re eerily similar so having one be an object and the other an array doesn’t make much sense to me. (as mentioned before)
  2. I’m not sure what the individual values in your “branches” array mean, there are 6 values there, the original ruby example has 7 (including the count which it points too). I’m guessing you might have omitted the 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)
  3. this is nitpicky, but I don’t know why we’d need the “Test” top level key - just meta and coverage should be enough as we’re only dealing with test coverage, right? 😃

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:

#!/bin/bash -eo pipefail
./cc-test-reporter format-coverage -t simplecov -o coverage/codeclimate.json
./cc-test-reporter upload-coverage -i coverage/codeclimate.json

Error: json: cannot unmarshal object into Go struct field input.coverage of type []formatters.NullInt
Usage:
  cc-test-reporter format-coverage [coverage file] [flags]

Flags:
      --add-prefix string   add this prefix to file paths
  -t, --input-type string   type of input source to use [clover, cobertura, coverage.py, excoveralls, gcov, gocov, jacoco, lcov, simplecov, xccov]
  -o, --output string       output path (default "coverage/codeclimate.json")
  -p, --prefix string       the root directory where the coverage analysis was performed (default "/home/circleci/project")

Global Flags:
  -d, --debug   run in debug mode

Exited with code exit status 255

@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 😁

WhatsApp Image 2020-12-11 at 12 02 24

@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:

  1. a proposed json format for Tobias’s review
  2. Code Climate engineers will then open a PR on the SimpleCov project.

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!

There 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

  1. You need to set env CC_TEST_REPORTER_ID for both simplecov (i.e. bundle exec rake) AND the codeclimate action.
  2. Explicitly setting SimpleCov.formatter would break things if you don’t include the JSON formatter in your formatters. Explicitly setting the formatter as JSONFormatter should also work, and by doing so you don’t need (1). Ref: https://github.com/simplecov-ruby/simplecov/blob/441d8ca6249c07275202880f3ff604272a4a3b76/lib/simplecov/default_formatter.rb#L11

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 ✌️

IMG_20180317_113323

@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:

Note: SimpleCov 0.18+ not yet supported

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:

require "simplecov_json_formatter"

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.

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.

@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 our simplecov-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:

  • A Regarding to the JSON formatter implementation itself.
    • A.1 Should be part of simplecov’s internals logic.
    • A.2 Should be packaged in it’s own gem, and installed by default within simplecov as with the HTML formatter.
  • B Regarding to the logic that determines whetter to output JSON, HTML or both, or other options.
    • B.1 Seems logical due to the CI related use case of the JSON formatter to be able to config simplecov’s formatter using an environment variable. So we good need to add that logic to simplecov.
      • B.1* Since it would be awesome for the people using codeclimate’s tools to not need to change their current setup, we would like to add a special logic in simplecov to output JSON if the environment variable CC_TEST_REPORTER_ID is present.
    • B.2 Always output JSON along with the HTML.

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:

  1. suggesiton of JSON format
  2. feedback on format, consensus
  3. Implement JSON formatter over in SimpleCov
  4. Implement that format for SimpleCov here in the test reporter
  5. release both a new version of the test reporter and SImpleCov

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:

Coverage report generated for RSpec to /home/runner/work/rspec-pending_for/rspec-pending_for/coverage. 209 / 211 LOC (99.05%) covered.

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

👋 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 (and simplecov 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

       "branches": {
          "5": 0,
          "7": 2
        }

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:

[:if, 3, 6, 4, 13, 10] =>
          {[:then, 4, 7, 6, 7, 10] => 0, [:else, 5, 9, 4, 13, 10] => 1}

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 and else) 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 having undefined and null 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:

{
  "Tests": {
    "coverage": {
      "filename": {
        "lines": [ 1, null, 0, 3, 1 ],
        "methods": {
          "2": 3,
          "6": 0, 
          "17": 1
        }, 
        "branches": {
          "5": 0,
          "7": 2
        },
        "oneshot_lines" : {
          "8": 1
        },
        "ignore_coverage": [4, 20, 60]
      }
    }
  } 

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.

Does Code Climate still have an environment variable like “CODE_CLIMATE_TOKEN” or something that could be checked to only write the JSON report (unless configured in other ways) when that is set?

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.