From 991dad67049582c6ac0108d9ec5107a14243907c Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Fri, 20 Jan 2023 08:32:32 +0200 Subject: [PATCH 1/8] [improve][ci] Improve code coverage reporting - use test scope classpath to collect correct dependencies - exclude test projects so that coverage doesn't get reported for them --- build/pulsar_ci_tool.sh | 52 +++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/build/pulsar_ci_tool.sh b/build/pulsar_ci_tool.sh index 613b58bcb582b..73449ad43356e 100755 --- a/build/pulsar_ci_tool.sh +++ b/build/pulsar_ci_tool.sh @@ -363,14 +363,8 @@ _ci_upload_coverage_files() { for execFile in $execFiles; do local project="${execFile/%"/target/jacoco.exec"}" local artifactId=$(xmlstarlet sel -t -m _:project -v _:artifactId -n $project/pom.xml) - local scope=runtime - # for integration tests, there's no dependencies in the runtime scope - # detect a plain test project based on missing src/main/java - if [ ! -d $project/src/main/java ]; then - scope=test - fi - # find the runtime classpath for the project to ensure that only production classes get covered - mvn -f $project/pom.xml -DincludeScope=$scope -Dscan=false dependency:build-classpath -B | { grep 'Dependencies classpath:' -A1 || true; } | tail -1 \ + # find the test scope classpath for the project + mvn -f $project/pom.xml -DincludeScope=test -Dscan=false dependency:build-classpath -B | { grep 'Dependencies classpath:' -A1 || true; } | tail -1 \ | sed 's/:/\n/g' | { grep 'org/apache/pulsar' || true; } \ | { tee -a $completeClasspathFile || true; } > target/classpath_$artifactId || true done @@ -468,7 +462,8 @@ ci_create_test_coverage_report() { else cd "$SCRIPT_DIR/.." fi - local execFiles=$(find . '(' -path "*/target/jacoco.exec" -or -path "*/target/jacoco_*.exec" ')' -printf "%P\n") + + local execFiles=$(find . '(' -path "*/target/jacoco.exec" -or -path "*/target/jacoco_*.exec" ')' -printf "%P\n" ) if [[ -n "$execFiles" ]]; then mkdir -p /tmp/jacocoDir if [ ! -f /tmp/jacocoDir/jacococli.jar ]; then @@ -492,41 +487,42 @@ ci_create_test_coverage_report() { done } | sort | uniq) + # projects that aren't considered as production code and their own src/main/java source code shouldn't be analysed + local excludeProjectsPattern="testmocks|testclient|buildtools" + # iterate projects for project in $projects; do local artifactId="$(printf "%s" "$projectToArtifactIdMapping" | grep -F "$project " | cut -d' ' -f2)" - if [ -d "$project/target/classes" ]; then + if [[ -d "$project/target/classes" && -d "$project/src/main/java" ]]; then mkdir -p "$classesDir/$project" cp -Rl "$project/target/classes" "$classesDir/$project" - echo "/$artifactId/" >> $filterArtifactsFile - fi - local scope=runtime - if [ -d $project/src/main/java ]; then echo "$project/src/main/java" >> $sourcefilesFile - else - # for integration tests, there's no dependencies in the runtime scope - scope=test fi - if [ -f "target/classpath_$artifactId" ]; then - echo "Found cached classpath for $artifactId." - cat "target/classpath_$artifactId" >> $completeClasspathFile + echo "/$artifactId/" >> $filterArtifactsFile + if [[ -n "$(echo "$project" | grep -v -E "$excludeProjectsPattern")" ]]; then + if [ -f "target/classpath_$artifactId" ]; then + echo "Found cached classpath for $artifactId." + cat "target/classpath_$artifactId" >> $completeClasspathFile + else + echo "Resolving classpath for $project..." + # find the test scope classpath for the project + mvn -f $project/pom.xml -DincludeScope=test -Dscan=false dependency:build-classpath -B | { grep 'Dependencies classpath:' -A1 || true; } | tail -1 \ + | sed 's/:/\n/g' | { grep 'org/apache/pulsar' || true; } \ + >> $completeClasspathFile || true + fi else - echo "Resolving classpath for $project..." - # find the runtime classpath for the project to ensure that only production classes get covered - mvn -f $project/pom.xml -DincludeScope=$scope -Dscan=false dependency:build-classpath -B | { grep 'Dependencies classpath:' -A1 || true; } | tail -1 \ - | sed 's/:/\n/g' | { grep 'org/apache/pulsar' || true; } \ - >> $completeClasspathFile || true + echo "Skipping analysing of $project" fi done # delete any possible embedded jar files in the classes directory find "$classesDir" -name "*.jar" -print -delete - filterJarsPattern="bouncy-castle-bc|tests|/buildtools/" + local excludeJarsPattern="bouncy-castle-bc|tests|$excludeProjectsPattern" local classfilesArgs="--classfiles $({ { - for classpathEntry in $(cat $completeClasspathFile | { grep -v -f $filterArtifactsFile || true; } | sort | uniq | { grep -v -E $filterJarsPattern || true; }); do + for classpathEntry in $(cat $completeClasspathFile | { grep -v -f $filterArtifactsFile || true; } | sort | uniq | { grep -v -E "$excludeJarsPattern" || true; }); do if [[ -f $classpathEntry && -n "$(unzip -Z1C $classpathEntry 'META-INF/bundled-dependencies/*' 2>/dev/null)" ]]; then # file must be processed by removing META-INF/bundled-dependencies local jartempfile=$(mktemp -t jarfile.XXXX --suffix=.jar) @@ -543,7 +539,7 @@ ci_create_test_coverage_report() { local sourcefilesArgs="--sourcefiles $({ # find the source file folders for the pulsar .jar files that are on the classpath - for artifactId in $(cat $completeClasspathFile | sort | uniq | { grep -v -E $filterJarsPattern || true; } | perl -p -e 's|.*/org/apache/pulsar/([^/]*)/.*|$1|'); do + for artifactId in $(cat $completeClasspathFile | sort | uniq | { grep -v -E "$excludeJarsPattern" || true; } | perl -p -e 's|.*/org/apache/pulsar/([^/]*)/.*|$1|'); do local project="$(printf "%s" "$projectToArtifactIdMapping" | { grep $artifactId || true; } | cut -d' ' -f1)" if [[ -n "$project" && -d "$project/src/main/java" ]]; then echo "$project/src/main/java" From 5823d21e918b359197c0f5fd26cb99049d5c433f Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Fri, 20 Jan 2023 08:44:11 +0200 Subject: [PATCH 2/8] Show link to Codecov commit level report --- .github/actions/upload-coverage/action.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/actions/upload-coverage/action.yml b/.github/actions/upload-coverage/action.yml index 97d311caf5016..53e1feba3d833 100644 --- a/.github/actions/upload-coverage/action.yml +++ b/.github/actions/upload-coverage/action.yml @@ -83,4 +83,13 @@ runs: with: flags: ${{ inputs.flags }} fail_ci_if_error: true - verbose: true \ No newline at end of file + verbose: true + - name: "Show link to Codecov report" + shell: bash + run: | + tee -a "$GITHUB_STEP_SUMMARY" < Date: Fri, 20 Jan 2023 08:51:31 +0200 Subject: [PATCH 3/8] Notify as soon as 4 uploads are completed for the commit --- codecov.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/codecov.yml b/codecov.yml index d8330c2ef3e81..e9f9593658680 100644 --- a/codecov.yml +++ b/codecov.yml @@ -18,12 +18,15 @@ # codecov: - require_ci_to_pass: yes + # the state in Codecov for a PR status in GitHub is often wrong. Just ignore this condition and + # rely on after_n_builds for notifying about the coverage + require_ci_to_pass: no notify: # should match the number of coverage report uploads # pulsar-ci.yaml contains 3 uploads (unittests, inttests, systests) # pulsar-ci-flaky.yaml contains 1 upload after_n_builds: 4 + wait_for_ci: no comment: # should match the number of builds sending coverage reports From 4954a9b7799be337218d06a5a0c034538186b1b8 Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Fri, 20 Jan 2023 09:15:12 +0200 Subject: [PATCH 4/8] Exclude shaded classes --- pom.xml | 5 +++++ .../pulsar/tests/integration/containers/PulsarContainer.java | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f40e79dd48b85..2ddbbb6e1d126 100644 --- a/pom.xml +++ b/pom.xml @@ -1970,6 +1970,11 @@ flexible messaging model and an intuitive client API. org.apache.pulsar.* org.apache.bookkeeper.mledger.* + + *.proto.* + *.shade.* + *.shaded.* + diff --git a/tests/integration/src/test/java/org/apache/pulsar/tests/integration/containers/PulsarContainer.java b/tests/integration/src/test/java/org/apache/pulsar/tests/integration/containers/PulsarContainer.java index b3d6747bf8646..dc11acd00c3f2 100644 --- a/tests/integration/src/test/java/org/apache/pulsar/tests/integration/containers/PulsarContainer.java +++ b/tests/integration/src/test/java/org/apache/pulsar/tests/integration/containers/PulsarContainer.java @@ -281,7 +281,8 @@ protected void configureCodeCoverage() { } withEnv("OPTS", "-javaagent:/jacocoDir/" + jacocoAgentJar.getName() + "=destfile=/jacocoDir/jacoco_" + getContainerName() + "_" + System.currentTimeMillis() + ".exec" - + ",includes=org.apache.pulsar.*:org.apache.bookkeeper.mledger.*"); + + ",includes=org.apache.pulsar.*:org.apache.bookkeeper.mledger.*" + + ",excludes=*.proto.*:*.shade.*:*.shaded.*"); } else { log.error("Cannot find jacoco agent jar from '" + jacocoAgentJar.getAbsolutePath() + "'"); } From ff4ccea303289a7c5d5a979dbc371a36d1d2e842 Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Fri, 20 Jan 2023 09:25:50 +0200 Subject: [PATCH 5/8] Adjust inttest coverage report: exclude test project sources --- build/pulsar_ci_tool.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/build/pulsar_ci_tool.sh b/build/pulsar_ci_tool.sh index 73449ad43356e..64baa6359e115 100755 --- a/build/pulsar_ci_tool.sh +++ b/build/pulsar_ci_tool.sh @@ -591,10 +591,12 @@ ci_create_inttest_coverage_report() { # remove any bundled dependencies as part of .jar/.nar files find /tmp/jacocoDir/pulsar_lib '(' -name "*.jar" -or -name "*.nar" ')' -exec echo "Processing {}" \; -exec zip -q -d {} 'META-INF/bundled-dependencies/*' \; |grep -E -v "Nothing to do|^$" || true fi + # projects that aren't considered as production code and their own src/main/java source code shouldn't be analysed + local excludeProjectsPattern="testmocks|testclient|buildtools" # produce jacoco XML coverage report from the exec files and using the extracted jar files java -jar /tmp/jacocoDir/jacococli.jar report /tmp/jacocoDir/*.exec \ --classfiles /tmp/jacocoDir/pulsar_lib --encoding UTF-8 --name "Pulsar Integration Tests - coverage in containers" \ - $(find -path "*/src/main/java" -printf "--sourcefiles %P ") \ + $(find -path "*/src/main/java" -printf "--sourcefiles %P " | grep -v -E "$excludeProjectsPattern") \ --xml target/jacoco_inttest_coverage_report/jacoco.xml \ --html target/jacoco_inttest_coverage_report/html \ --csv target/jacoco_inttest_coverage_report/jacoco.csv From e6232bd4954f799bfd15672d384a0591b7b75c9a Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Fri, 20 Jan 2023 11:01:26 +0200 Subject: [PATCH 6/8] Fix commit id in Codecov url --- .github/actions/upload-coverage/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/upload-coverage/action.yml b/.github/actions/upload-coverage/action.yml index 53e1feba3d833..273d50856d0c1 100644 --- a/.github/actions/upload-coverage/action.yml +++ b/.github/actions/upload-coverage/action.yml @@ -90,6 +90,6 @@ runs: tee -a "$GITHUB_STEP_SUMMARY" < Date: Fri, 20 Jan 2023 11:10:54 +0200 Subject: [PATCH 7/8] Ignore test code in Codecov --- codecov.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/codecov.yml b/codecov.yml index e9f9593658680..1ee8e26e68a4e 100644 --- a/codecov.yml +++ b/codecov.yml @@ -47,3 +47,12 @@ coverage: default: target: auto informational: true + +# ignore test code +ignore: + - tests + - testmocks + - buildtools + - pulsar-testclient + - pulsar-client-tools-customcommand-example + - pulsar-functions/java-examples \ No newline at end of file From 602f60f8cbf49dd4e5fd5ff3faf2758976ed9795 Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Fri, 20 Jan 2023 12:39:09 +0200 Subject: [PATCH 8/8] Fix Codecov link once more --- .github/actions/upload-coverage/action.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/actions/upload-coverage/action.yml b/.github/actions/upload-coverage/action.yml index 273d50856d0c1..a9706e77333cb 100644 --- a/.github/actions/upload-coverage/action.yml +++ b/.github/actions/upload-coverage/action.yml @@ -87,9 +87,14 @@ runs: - name: "Show link to Codecov report" shell: bash run: | + if [[ "${GITHUB_EVENT_NAME}" == "pull_request" ]]; then + head_sha=$(jq -r '.pull_request.head.sha' "${GITHUB_EVENT_PATH}") + else + head_sha=$(git rev-parse HEAD) + fi tee -a "$GITHUB_STEP_SUMMARY" <