Skip to content

Add CodeCov - #28

Merged
Vladyslav-Kuksiuk merged 7 commits into
masterfrom
add-codecov
Jul 28, 2026
Merged

Add CodeCov#28
Vladyslav-Kuksiuk merged 7 commits into
masterfrom
add-codecov

Conversation

@Vladyslav-Kuksiuk

Copy link
Copy Markdown
Collaborator

This PR adds CodeCov to verify the test coverage.

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk self-assigned this Jul 28, 2026
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

Base automatically changed from clean-up to master July 28, 2026 12:03
@Vladyslav-Kuksiuk
Vladyslav-Kuksiuk marked this pull request as ready for review July 28, 2026 12:44
Comment thread codecov.yml
informational: true
patch:
default:
target: 90%

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must fix — the patch target is not met by this PR itself.

codecov/patch on this head commit reports 69.07% of diff hit (target 90.00%), so the check this file introduces is red on the very change that adds it, and it will stay red for anyone rebasing on it.

Reproduced locally with ./gradlew --no-configuration-cache :gradle-plugin:jacocoTestReport. The uncovered new lines are all error-handling wrappers plus one synthetic overload:

  • InstallationDirectory.kt 66–69, 142–147, 182–185 — the IOExceptionGradleException wrappers in prepare(), createDirectoriesSafely(), and isRedirectingFileSystemEntry().
  • Checksum.kt 191–192 — the IOException catch in readChecksumMetadata.
  • Checksum.kt 152, 155 — the synthetic default-argument overload of readGitHubReleaseMetadata (see the separate comment there).

Smallest correction: either cover the reachable ones (a read-only installation root exercises the prepare() wrapper; a closed loopback socket exercises the readChecksumMetadata catch) and drop the default argument, or set a target this repository can actually sustain and raise it later. Merging a permanently failing status check is the outcome to avoid.

internal fun readGitHubReleaseMetadata(
source: URI,
githubToken: String?,
readMetadata: (URI, String?) -> String = ::readChecksumMetadata,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should fix — the defaulted test seam leaves the production call path uncovered and deviates from the neighbouring function.

resolveExpectedAssetSha256 (line 211 in this same file) takes readMetadata: (URI) -> String as a required parameter, and InstallEmbedCodeTask passes the reader explicitly. This new function adds a default instead, which has two consequences:

  1. Kotlin emits a synthetic readGitHubReleaseMetadata$default for the two-argument production call. ChecksumSpec only ever calls the three-argument form, and the TestKit builds configure sha256 explicitly so the metadata reader is never reached — lines 152 and 155 are fully uncovered in the generated JaCoCo report, and they count against the 90% patch target added in codecov.yml.
  2. The reader that production actually uses is no longer visible at the call site.

Smallest correction: drop = ::readChecksumMetadata and pass ::readChecksumMetadata from InstallEmbedCodeTask, matching resolveExpectedAssetSha256.

Separately, the KDoc above does not mention readMetadata at all. Per the KDoc policy, an internal declaration needs KDoc where it explains a non-obvious contract — an injected reader qualifies.

Comment thread .github/workflows/coverage.yml Outdated
uses: codecov/codecov-action@v7
with:
files: ./gradle-plugin/build/reports/jacoco/test/jacocoTestReport.xml
fail_ci_if_error: true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should fix — fail_ci_if_error: true combined with a repository secret makes fork pull requests fail.

The workflow triggers on pull_request. GitHub does not expose secrets.* to workflow runs from forked repositories, so token resolves to an empty string there. codecov-action then falls back to a tokenless upload, which is rate-limited, and fail_ci_if_error: true turns any upload hiccup into a red check on a contribution the author cannot fix.

Smallest correction: either drop fail_ci_if_error: true (coverage reporting is advisory here — project is already informational: true), or gate the upload step with if: github.event.pull_request.head.repo.full_name == github.repository.

}

@Test
fun `reject coverage collection with the configuration cache`() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should fix — this test verifies the spec's own helper, not the plugin.

assertThrows(IllegalArgumentException::class.java) { runner(...) } asserts on the require(...) at line 1200 of this same file. No Gradle build runs, no plugin code executes, and the assertion breaks the moment the helper's message is reworded.

This is a TestKit consumer-build suite; per the test-engineer skill each spec should prove a consuming-build contract. The invariant here is a private precondition — the two callers that need it already pass collectCoverage = false explicitly, and a misuse fails loudly at development time anyway.

Smallest correction: delete the test and keep the require.

/**
* Verifies that [directory] resolves below the real installation root.
*/
internal fun requireRealPathInside(directory: Path) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should fix — visibility widened purely for test access.

requireRealPathInside was private before the move out of InstallEmbedCodeTask; it is now internal so that InstallationDirectorySpec can call it directly. Reaching it through the public entry point is impossible by design: createDirectoriesSafely already rejects every redirecting component before this final check runs, which is why the spec has to bypass it. Widening the boundary to test an unreachable defence-in-depth branch trades a real invariant (this method is an internal postcondition of createDirectoriesSafely) for a coverage line.

Smallest correction: restore private and drop the second test in InstallationDirectorySpec, or keep the test and note in KDoc why the method is separately reachable.

Minor, same file: every member is marked internal inside an already-internal class, so the modifier adds nothing. The original nested class relied on the enclosing visibility.

private companion object {
const val TEST_KIT_COVERAGE_JVM_ARGUMENT_PROPERTY =
"io.spine.embedcode.gradle.testkit.coverage.jvm-argument"
const val DEFAULT_GRADLE_DAEMON_JVM_ARGUMENTS =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit — the hardcoded daemon arguments silently restate Gradle's defaults.

-Dorg.gradle.jvmargs=... replaces the daemon JVM arguments rather than appending to them, which is why the JaCoCo agent argument has to be prefixed with these values. That reason is not recorded anywhere, and -Xmx512m -XX:MaxMetaspaceSize=384m pins today's Gradle defaults: if a future Gradle raises them, coverageFunctionalTest will quietly run its daemons with less memory than functionalTest does, and the two suites will diverge under load.

Smallest correction: add a short comment stating that the constant exists because org.gradle.jvmargs is replaced, not appended.

import java.net.HttpURLConnection
import java.net.URI
import java.net.URLEncoder
import java.net.URLConnection

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit — import out of order.

java.net.URLConnection sorts before java.net.URLEncoder (C < E), but it is added after it. Every other import block in this file is lexicographic.

import java.util.HexFormat

private const val SHA256_LENGTH = 64
private const val CHECKSUM_METADATA_CONNECT_TIMEOUT_MILLIS = 30_000

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit — duplicated timeout values.

These two constants carry exactly the values of InstallEmbedCodeTask.CONNECT_TIMEOUT_MILLIS and READ_TIMEOUT_MILLIS (still present and still used for the asset download at InstallEmbedCodeTask.kt:520-521). The move split one HTTP timeout policy into two independent sources of truth that will drift.

Smallest correction: keep one pair of constants and reference it from both call sites.

Comment thread codecov.yml Outdated
threshold: 0%
only_pulls: true

ignore:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit — these ignore entries have no effect.

The uploaded report is gradle-plugin/build/reports/jacoco/test/jacocoTestReport.xml, whose classDirectories come from the plugin module's main source set only. Neither buildSrc/** (a separate build, never in this report) nor scripts/** (Python) can appear in it, so Codecov has nothing to ignore.

Harmless, but it reads as if buildSrc coverage were being deliberately suppressed. Worth either removing, or keeping with a comment that it guards against a future report that does include them.

@Oleg-Melnik Oleg-Melnik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Vladyslav-Kuksiuk LGTM with comments to address.

@Vladyslav-Kuksiuk
Vladyslav-Kuksiuk merged commit 6fe6627 into master Jul 28, 2026
4 checks passed
@Vladyslav-Kuksiuk
Vladyslav-Kuksiuk deleted the add-codecov branch July 28, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants