RFC: Adopt Codecov at Artsy, starting with Gravity about readme HOT 8 CLOSED

Resolution

We decided codecov might not be the ideal option, but we would like to have coverage data somehow. We'll work on an initial setup for Exchange as a Future Friday project

Level of Support

5: Unclear Resolution.

Additional Context:

Depending on the results of the FF implementation, we'll have more clarity on what would be the best approach for us

Next Steps

Future Friday project

from readme.

I definitely like that it will spare the reviewer the hint that there is a test missing for a new implementation. I like when tools are taking over these kind of simple jobs. In the past I have had different experiences... Sometimes it was useful, and sometimes the tool kept being ignored and in the end was just hindering work. I think it also depends on the configuration and usage. But I am definitely up for a try.

from readme.

I for one would be very interested in coverage data for a project like Gravity, since it has decent coverage but possibly a lot of slow and redundant tests. We need to constantly be consolidating and refining its tests to avoid excessive bloat.

However, I've been burned by the Codecov integration too many times to advocate for adopting it across more projects:

• There was a massive exploit recently in which the Codecov integration allowed malicious code to be executed with access to source code and CI configuration, and necessitated a painstaking review of dozens of repositories' histories for anything sensitive that may have been exposed.
• Exchange's CI is running codecov right now, but the artifacts aren't being exposed correctly.
• Positron's Codecov integration is 2 major versions behind and tests are failing.

I guess all I'm saying is that these integrations tend to require a lot of maintenance, and seeing value in the results is not the same as volunteering to fix the inevitable issues that come up. I'd be interested to find a way to generate a coverage report periodically rather than integrate them into every CI run, especially if we don't plan to respect the output on a PR-by-PR basis.

from readme.

I'm intrigued. We've done this very sporadically for some projects (example — coveralls instead of codecov, technically), but I can't recall conversations about pros & cons myself.

One suggestion is that if we do proceed with this, we might trial it on a smaller Rails project than Gravity to begin with, to get a better idea if we should adopt more widely.

Actually now that I'm poking around I see that Exchange used to have this enabled but it seems like it was turned off at some point? It might be fruitful to ask that team why.

from readme.

Hmm, that's a very good point!! As I understand, Exchange is mostly maintained by @artsy/transact-devs and @artsy/px-devs, right? Would you have some thoughts to be shared on this?

from readme.

Great inputs, thanks a lot, @joeyAghion !!

Based on these points, I would think if it would make sense to discuss setting up just simplecov as a code coverage analysis tool (without necessarily integrating it with codecov). What would you think?

from readme.

I'm not that familiar with simplecov or how that would make the integration simpler. Assuming that's the case, though, I'd be open to it. I second @anandaroop's suggestion to pilot with a smaller Rails project first (like maybe by fixing/replacing Exchange's tooling).

from readme.

In that case, would it make sense to think about this as a Future Friday project?

from readme.