Skip to content

Fix problems between cc-test-reporter and SimpleCov in circleci#5673

Merged
tramuntanal merged 3 commits intomasterfrom
fix/cc_test_reporter-simplecov_version_incompatibility
Jan 30, 2020
Merged

Fix problems between cc-test-reporter and SimpleCov in circleci#5673
tramuntanal merged 3 commits intomasterfrom
fix/cc_test_reporter-simplecov_version_incompatibility

Conversation

@tramuntanal
Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal commented Jan 29, 2020

🎩 What? Why?

In CircleCi, the "Format CodeClimate test coverage" step fails due to an incompatibility between `cc-test-reporter's expected input format and what simplecov outputted:

./cc-test-reporter format-coverage coverage/.resultset.json -t simplecov -o coverage/codeclimate.$CIRCLE_JOB.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 "/app")

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


Exited with code exit status 255

simplecov 0.17.1 version seems to work for other projects: https://github.com/DEFRA/defra-ruby-validators/pull/49/files

Although this is the versions Decidim already has in the Gemfile.lock.

📌 Related Issues

  • Related to #?
  • Fixes #?

📋 Subtasks

  • Add CHANGELOG entry
  • Add documentation regarding the feature
  • Add/modify seeds
  • Add tests
  • Another subtask

@tramuntanal tramuntanal self-assigned this Jan 29, 2020
@tramuntanal tramuntanal changed the title Force simplecov v0.17.1 which seems to be the current compatible version Fix problems between cc-test-reporter and SimpleCov in circleci Jan 29, 2020
@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Jan 30, 2020

Tests passed @tramuntanal!

@tramuntanal
Updgrade simplecov to v0.18
f9dff26 
To check if cc-test-reporter supports simplecov 0.18.
Following what has been said [here](qltysh-archive/test-reporter#418)
it should fail.
@tramuntanal
Revert "Updgrade simplecov to v0.18"
518dc45 
This reverts commit f9dff26.
@tramuntanal
Copy link
Copy Markdown
Contributor Author

tramuntanal commented Jan 30, 2020 •
edited
Loading

There was a strange thing about this fix.
In theory, the problem was that cc-test-reporter still has no support for simplecov v0.18.0. But simplecov was declared in decidim-dev's gemspec as s.add_dependency "simplecov", "~> 0.16" and resolved to v0.17.1. So it should have worked, but failed.

I tried to upgrade to v0.18.0 just in case the problem was the other way around, this is, cc-test-reporter was only supporting v0.18.0. But again it failed.

So... forcing the exact version in decidim-dev gemspec works. We'll merge this then and see how the test reporter solves the incompatibility. See:

@tramuntanal tramuntanal marked this pull request as ready for review January 30, 2020 10:41
@tramuntanal tramuntanal requested a review from mrcasals January 30, 2020 10:42
@tramuntanal tramuntanal merged commit 1ae49f4 into master Jan 30, 2020
@tramuntanal tramuntanal deleted the fix/cc_test_reporter-simplecov_version_incompatibility branch January 30, 2020 12:35
@mrcasals mrcasals mentioned this pull request Jan 31, 2020
Merged
1 task
@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Feb 3, 2020

I rebased #5674 so that it builds on top of this one, but it's still failing due to cc-test-reporter issues, can you check what's going on please? 😕

@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Feb 6, 2020

Update on my previous comment: I upgraded simplecov to 0.18 in my PR before rebasing, so I didn't notice when the version limit was added. I undid the upgrade and everything works as expected now.

@mrcasals mrcasals mentioned this pull request Feb 6, 2020
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@microstudi microstudi microstudi approved these changes

@mrcasals mrcasals Awaiting requested review from mrcasals

Assignees

@tramuntanal tramuntanal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tramuntanal @mrcasals @microstudi