Enable Codecoverage Upload (Jacoco/Codecov) #1981

daniel-sudz · 2022-03-22T17:09:13Z

verify that it is actually running
codecov.yml file probably good idea
clean up for review
verify that codecov seems reasonable

basically moving changes over from #1922 based on #1977

.github/workflows/CI.yml

daniel-sudz · 2022-03-22T17:12:32Z

scripts/run_test.sh

 TST_EXIT_CODE=$?

 echo "Running mima checks ... "
-echo "time ./sbt ++$TRAVIS_SCALA_VERSION $(withCmd mimaReportBinaryIssues)"


we already have the bash flags that print out commands before they are run so this isn't really needed

daniel-sudz · 2022-03-22T17:12:59Z

scripts/run_test.sh

-COMPILE_DOC_EXIT_CODE=$?

-echo "all done"

-exit $(( $TST_EXIT_CODE || $MIMA_EXIT_CODE || $COMPILE_DOC_EXIT_CODE ))
+echo "all test checks done"


we already have bash flags to fail-fast so we don't explicitly need to collect exit codes

codecov-commenter · 2022-03-22T23:46:28Z

Codecov Report

❗ No coverage uploaded for pull request base (develop@525321c). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             develop    #1981   +/-   ##
==========================================
  Coverage           ?   40.49%           
  Complexity         ?     1353           
==========================================
  Files              ?      356           
  Lines              ?    26221           
  Branches           ?     4579           
==========================================
  Hits               ?    10617           
  Misses             ?    13938           
  Partials           ?     1666

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 525321c...5db2df2. Read the comment docs.

johnynek · 2022-03-23T20:31:48Z

Coverage seems quite low. Are we sure it is aggregating all the projects correctly? Many subprojects wind up testing the others.

daniel-sudz · 2022-03-23T20:48:53Z

Coverage seems quite low. Are we sure it is aggregating all the projects correctly? Many subprojects wind up testing the others.

there's currently one part in the CI that is failing. It should be a bit higher because of it. I'm not 100% sure why it's failing just get a "step failed" in cascading. I suspect it's due to very high memory usage due to coverage. I can currently reproduce the failure locally when switching coverage on/off before running the test. I'm playing with memory settings but currently not much success. See scoverage/sbt-scoverage#50

daniel-sudz · 2022-03-23T21:21:56Z

Coverage seems quite low. Are we sure it is aggregating all the projects correctly? Many subprojects wind up testing the others.

there's currently one part in the CI that is failing. It should be a bit higher because of it. I'm not 100% sure why it's failing just get a "step failed" in cascading. I suspect it's due to very high memory usage due to coverage. I can currently reproduce the failure locally when switching coverage on/off before running the test. I'm playing with memory settings but currently not much success. See scoverage/sbt-scoverage#50

after tuning the JVM opts, jacking the stack frame size to 64M (crazy!) seemed to let the failing test pass. It looks like others have had similar issues: scoverage/sbt-scoverage#276 scoverage/sbt-scoverage#422

edit: I though I had coverage enabled for above but I did not so I still have no resolution for sbt-scoverage

johnynek · 2022-03-23T21:28:55Z

huh:

https://github.com/twitter/scalding/runs/5664208563?check_suite_focus=true#step:7:1367

seems one of the jobs fails (in both 2.11 and 2.12) with coverage... I wonder if somehow the instrumentation is making something non-serializable...

daniel-sudz · 2022-03-23T22:29:41Z

huh:

https://github.com/twitter/scalding/runs/5664208563?check_suite_focus=true#step:7:1367

seems one of the jobs fails (in both 2.11 and 2.12) with coverage... I wonder if somehow the instrumentation is making something non-serializable...

that's completely possible. I've tested https://www.scala-sbt.org/sbt-jacoco/index.html as an alternative on the failing test and it seems to work. I'm going to try it in CI.

The downside of jacoco is that it knows nothing about scala (targets bytecode) so it might be slightly less accurate.

another miscopy typo typo in dirname feed cobertura.xml into codecov instead of scoverage.xml specify files due to autodetect failure use coverageAggregate instead of coverageReport load default codecov behaviour into codecov.yml typo draft impl

autodetect on codecov uploader seems to work new bash features not supported on CI try using jacoco

daniel-sudz · 2022-03-24T17:47:32Z

scripts/run_test.sh

-export JVM_OPTS="-XX:+CMSClassUnloadingEnabled -XX:+UseConcMarkSweepGC -XX:ReservedCodeCacheSize=96m -XX:+TieredCompilation -XX:MaxPermSize=256m -Xms256m -Xmx512m -Xss2m"
-
-
-INNER_JAVA_OPTS="set javaOptions += \"-Dlog4j.configuration=file://$TRAVIS_BUILD_DIR/project/travis-log4j.properties\""


I see no references to this variable while searching through the codebase. I don't think it's used anymore

daniel-sudz · 2022-03-24T19:35:18Z

@johnynek does 40% sound reasonable? It's recording 0% for maple (there is no test folder) and 0% for scalding-avro (no test folder) and 0% for scalding-thrift-macros (there are some tests but maybe it can't trace macros well).

I think this is similar to what scoverage was reporting.

Report is here: https://codecov.io/gh/twitter/scalding/tree/5db2df2a50b4a92c0fbf934a5dc13b06229624e9

the reference from 2 years ago was 68%: https://codecov.io/gh/twitter/scalding/tree/f1e9670175047724bf23727ab17f807303281150

johnynek · 2022-03-24T20:05:15Z

I don't see how it could have fallen so much, but maybe.

I was looking through, it marks a lot of lines as partially covered, e.g.:

https://app.codecov.io/gh/twitter/scalding/blob/5db2df2a50b4a92c0fbf934a5dc13b06229624e9/scalding-core/src/main/scala/com/twitter/scalding/typed/memory_backend/MemoryBackend.scala

are those being counted?

Anyway, this is an improvement, and we can iterate. Thanks for spending the time on this. We can later see if we can find examples of tests that definitely exercise code, but it doesn't see and add an issue at that time.

johnynek · 2022-03-24T20:05:48Z

README.md

@@ -1,7 +1,7 @@
 # Scalding

 [![Build status](https://github.com/twitter/scalding/actions/workflows/CI.yml/badge.svg?branch=develop)](https://github.com/twitter/scalding/actions)
-[![Coverage Status](https://coveralls.io/repos/twitter/scalding/badge.png?branch=develop)](https://coveralls.io/r/twitter/scalding?branch=develop)
+[![Coverage Status](https://img.shields.io/codecov/c/github/twitter/scalding/develop.svg?maxAge=3600)](https://codecov.io/github/twitter/scalding)


max age is 1h? seems like maybe that should be longer no?

I'm fine with whatever. Although github also does their own caching so it's probably a bit irrelevant

daniel-sudz · 2022-03-24T20:52:01Z

I don't see how it could have fallen so much, but maybe.

I was looking through, it marks a lot of lines as partially covered, e.g.:

https://app.codecov.io/gh/twitter/scalding/blob/5db2df2a50b4a92c0fbf934a5dc13b06229624e9/scalding-core/src/main/scala/com/twitter/scalding/typed/memory_backend/MemoryBackend.scala

are those being counted?

Anyway, this is an improvement, and we can iterate. Thanks for spending the time on this. We can later see if we can find examples of tests that definitely exercise code, but it doesn't see and add an issue at that time.

my guess would be that the partial lines are the ones that can throw exceptions. Ie. they are being executed but not all paths covered

johnynek · 2022-03-26T01:54:51Z

Thanks!

daniel-sudz commented Mar 22, 2022

View reviewed changes

.github/workflows/CI.yml Outdated Show resolved Hide resolved

daniel-sudz commented Mar 22, 2022

View reviewed changes

daniel-sudz added 4 commits March 24, 2022 10:39

(squash) draft implementation

d25e68e

another miscopy typo typo in dirname feed cobertura.xml into codecov instead of scoverage.xml specify files due to autodetect failure use coverageAggregate instead of coverageReport load default codecov behaviour into codecov.yml typo draft impl

add changes to codecov.yml

f2dcece

(squash) don't upload to codecov when there is nothing to upload

b79dad1

(squash) use jacoco instead of scoverage

dd8e9a6

autodetect on codecov uploader seems to work new bash features not supported on CI try using jacoco

daniel-sudz force-pushed the coverage_report branch from 3b12057 to dd8e9a6 Compare March 24, 2022 17:45

daniel-sudz commented Mar 24, 2022

View reviewed changes

daniel-sudz added 2 commits March 24, 2022 10:53

update README with codecov link

a7d58c7

try adding some retry to the codecov uploader

5db2df2

daniel-sudz changed the title ~~[WIP] enable codecoverage upload~~ Enable Codecoverage Upload (Jacoco/Codecov) Mar 24, 2022

daniel-sudz marked this pull request as ready for review March 24, 2022 19:37

johnynek reviewed Mar 24, 2022

View reviewed changes

johnynek merged commit 3d63170 into twitter:develop Mar 26, 2022

johnynek mentioned this pull request Apr 9, 2022

Re-enable code coverage in CI and switch from Coveralls to Codecov #1922

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Codecoverage Upload (Jacoco/Codecov) #1981

Enable Codecoverage Upload (Jacoco/Codecov) #1981

daniel-sudz commented Mar 22, 2022 •

edited

Loading

daniel-sudz Mar 22, 2022

daniel-sudz Mar 22, 2022

codecov-commenter commented Mar 22, 2022 •

edited

Loading

johnynek commented Mar 23, 2022

daniel-sudz commented Mar 23, 2022 •

edited

Loading

daniel-sudz commented Mar 23, 2022 •

edited

Loading

johnynek commented Mar 23, 2022

daniel-sudz commented Mar 23, 2022

daniel-sudz Mar 24, 2022

daniel-sudz commented Mar 24, 2022

johnynek commented Mar 24, 2022

johnynek Mar 24, 2022

daniel-sudz Mar 24, 2022

daniel-sudz commented Mar 24, 2022

johnynek commented Mar 26, 2022

		export JVM_OPTS="-XX:+CMSClassUnloadingEnabled -XX:+UseConcMarkSweepGC -XX:ReservedCodeCacheSize=96m -XX:+TieredCompilation -XX:MaxPermSize=256m -Xms256m -Xmx512m -Xss2m"


		INNER_JAVA_OPTS="set javaOptions += \"-Dlog4j.configuration=file://$TRAVIS_BUILD_DIR/project/travis-log4j.properties\""

Enable Codecoverage Upload (Jacoco/Codecov) #1981

Enable Codecoverage Upload (Jacoco/Codecov) #1981

Conversation

daniel-sudz commented Mar 22, 2022 • edited Loading

daniel-sudz Mar 22, 2022

Choose a reason for hiding this comment

daniel-sudz Mar 22, 2022

Choose a reason for hiding this comment

codecov-commenter commented Mar 22, 2022 • edited Loading

Codecov Report

johnynek commented Mar 23, 2022

daniel-sudz commented Mar 23, 2022 • edited Loading

daniel-sudz commented Mar 23, 2022 • edited Loading

johnynek commented Mar 23, 2022

daniel-sudz commented Mar 23, 2022

daniel-sudz Mar 24, 2022

Choose a reason for hiding this comment

daniel-sudz commented Mar 24, 2022

johnynek commented Mar 24, 2022

johnynek Mar 24, 2022

Choose a reason for hiding this comment

daniel-sudz Mar 24, 2022

Choose a reason for hiding this comment

daniel-sudz commented Mar 24, 2022

johnynek commented Mar 26, 2022

daniel-sudz commented Mar 22, 2022 •

edited

Loading

codecov-commenter commented Mar 22, 2022 •

edited

Loading

daniel-sudz commented Mar 23, 2022 •

edited

Loading

daniel-sudz commented Mar 23, 2022 •

edited

Loading