review-fixes-java-run-fail

name	review-fixes-java-run-fail
description	Review pull requests with the `fixes-java-run-fail` label in graalvm-reachability-metadata. Use when asked to review or triage a PR described as `fixes-java-run-fail` or one that fixes Java/JVM runtime test failures for an existing library version update. Focus on validating the Java runtime fix, keeping the diff scoped, and ensuring dynamic-access coverage does not drop between the previously tested version and the new version.
argument-hint	[pr-number-or-url]

Review fixes-java-run-fail PRs

These PRs repair tests, dependencies, or narrow runtime setup after an existing library version compiles, but the JVM-based test fails when it runs. Review them more lightly than library-new-request PRs: the library already exists, so the goal is to restore Java runtime behavior for the new version while preserving meaningful test and dynamic-access coverage.

The PR number or URL can be passed as an optional argument (for example, 1234, https://github.com/oracle/graalvm-reachability-metadata/pull/1234). If the user says "review this PR" without an argument, infer the PR from the surrounding conversation or gh pr status; only ask the user when it cannot be inferred. Use gh pr view <pr>, gh pr diff <pr>, and gh pr checks <pr> against the resolved PR throughout the workflow below.

Review Principles

• Confirm the PR has label fixes-java-run-fail.
• Expect runtime-focused changes plus the normal generated support files for the newly tested version: test source updates, dependency adjustments, runtime configuration, a new metadata-version directory, stats, and metadata index changes.
• Be more relaxed than library-new-request: do not reject only because a test resembles older coverage, stays in an existing package layout, or contains compatibility branches for multiple supported versions.
• Do not accept changes that remove meaningful test coverage just to make java pass.
• Native-image execution should remain enabled by default when the repaired test reaches the native lane. Do not accept changes that skip or short-circuit native-image behavior. When a test exercises behavior that fundamentally relies on open-ended dynamic class loading that native image cannot support, such as loading classes, JARs, generated bytecode, plugin implementations, or other class definitions that are only discovered after the native executable is built, accept the NativeImageSupport.isUnsupportedFeatureError(e) catch pattern from org.graalvm.internal.tck. Reject raw native-only skips, bare catch (Error) blocks, and use of this pattern for ordinary reflection, resources, serialization, dynamic proxies, JNI, or missing reachability metadata.
• Treat dynamic-access coverage preservation as the main quality gate. The new version should not report lower dynamic-access coverage than the previously tested version unless the PR gives a concrete, credible reason.
• For numeric gates, compare the reported evidence as-is. Do not inspect generation filters, agent configuration, or metadata contents to second-guess why dynamic-access or metadata-count numbers are what they are.
• Compare total metadata entry counts between the previous metadata version and the new metadata version only as a severe-drop guardrail, using the counts reported in the PR description. When the PR reports both library metadata entries and test-only metadata entries, sum them for that version's total. Report metadata entry count issues only when the PR-reported new total metadata entry count has fewer than 25% as many entries as the PR-reported original count.
• Accept only reachability-metadata.json files as metadata files. Reject legacy native-image metadata config files such as reflect-config.json, resource-config.json, proxy-config.json, serialization-config.json, jni-config.json, or predefined-classes-config.json.
• Prefer small, targeted review comments. This label is for JVM runtime repair work, not a full redesign of historical tests.

Workflow

1. Inspect the PR summary.

   • Resolve the target PR from the optional argument, or infer it from context when possible.
   • Confirm the PR has label fixes-java-run-fail.
   • Identify the target coordinate, the previous tested version, and the new tested version from the PR body, title, changed index.json, and changed test path.
   • Gather files, reviews, inline comments, and CI checks.

2. Validate the diff scope.

   • Expected files are usually limited to the target coordinate's generated new-version support:
     • metadata/<group>/<artifact>/<new-version>/reachability-metadata.json
     • metadata/<group>/<artifact>/index.json
     • stats/<group>/<artifact>/<new-version>/stats.json
     • tests/src/<group>/<artifact>/<new-version>/**
   • Accept compatibility edits that keep one test source working across multiple tested versions.
   • Treat a new reachability-metadata.json for the tested version as normal for this label, including {} when validation and stats are coherent.
   • Treat generated test project files such as .gitignore, build.gradle, gradle.properties, settings.gradle, and user-code-filter.json as normal when they live under the target version's test directory.
   • Accept narrow runtime setup changes when they are necessary for the JVM test to exercise the same library behavior, such as test resources, dependency updates, system properties, service loading, or initialization ordering.
   • Be suspicious of unrelated build logic, workflows, generated sources, other libraries, or broad refactors.
   • Reject legacy native-image metadata config files. Metadata for generated support and test-only metadata must use reachability-metadata.json.
   • Reject or request changes if the PR removes tests, disables test classes, catches and ignores the failing exception, or disables native-image behavior.
   • Accept the NativeImageSupport.isUnsupportedFeatureError(e) catch pattern for tests that exercise behavior fundamentally requiring unsupported open-ended dynamic class loading. Reject bare catch (Error) blocks without the isUnsupportedFeatureError verification.

3. Review the Java runtime fix.

   • Confirm the edit addresses the actual JVM runtime failure, such as changed runtime behavior, missing test resources, service loading changes, dependency conflicts, classpath/module issues, initialization order, changed exception types, or changed API semantics that only fail during execution.
   • Compatibility branches are acceptable when they keep older and newer tested versions covered by the same test.
   • Version-specific logic is acceptable when the upstream runtime behavior genuinely changed, but it should be narrow and documented by the code structure or assertions.
   • Test expectation changes are acceptable only when they still assert meaningful behavior for the new version instead of accepting any output or exception.
   • Do not require the stricter library-new-request rules about scaffold-only tests or test package placement unless the PR is also adding a new library.

4. Check dynamic-access coverage across versions.

   • Compare stats/<group>/<artifact>/<old-metadata-version>/stats.json and stats/<group>/<artifact>/<new-metadata-version>/stats.json when stats files are present in the PR or available on the branch.
   • Compare dynamicAccess.coverageRatio, coveredCalls, totalCalls, and the dynamicAccess.breakdown entries for reflection, resources, proxies, serialization, JNI, or any other present report type.
   • Do not use user-code-filter.json, agent configuration, or metadata file contents to argue that the reported dynamic-access values are not comparable. Use the stats values as reported unless the stats are missing or stale.
   • A lower coverageRatio or lower coveredCalls for the new version is blocking unless the total dynamic-access surface changed and the PR explains why the reduction is expected.
   • If totalCalls increases while coveredCalls stays flat or the ratio drops, ask for additional test coverage or metadata unless the uncovered calls are clearly outside the library behavior under test.
   • If stats are missing or stale, ask for generateLibraryStats or the relevant CI stats job before approving.

5. Compare metadata entry counts.

   • Compare total metadata entry counts for the previous metadata version and the new metadata version from the PR description, matching summary fields such as Metadata entries, legacy Entries or Entries found, Test-only metadata entries, Metadata entries (new ...), Test-only metadata entries (new ...), Previous library version metadata entries, and Previous library version test-only metadata entries.
   • Treat each version's total as library metadata entries plus test-only metadata entries when both are reported.
   • Do not manually count entries from metadata files.
   • Do not use metadata file contents to argue that passing reported entry counts are incomplete.
   • Do not require an exact match. Differences are normal when upstream APIs move, generated metadata is cleaned up, or dynamic-access totals change.
   • Do not report metadata entry count issues unless the PR-reported new total metadata entry count is lower than 25% of the PR-reported original total metadata entry count.
   • When the new total is below 25% of the original and the tests and dynamic-access stats still claim comparable coverage, ask for restored metadata or a concrete explanation of the API/runtime-surface change.
   • If the PR description does not report usable old and new metadata entry counts, do not infer them from metadata files; ask for refreshed PR summary evidence when the comparison is needed.

6. Check CI before deciding.

   • Expected minimum: javaTest or equivalent changed-metadata JVM test checks are green for the target coordinate.
   • Compile and metadata validation checks should also be green; a Java runtime fix should not introduce a compile or metadata validation regression.
   • Prefer seeing the full target test lane green because a Java runtime fix can still reduce native-image coverage.
   • If current-defaults and future-defaults lanes both run, both should pass unless the PR clearly targets only one failing lane and the other failure is unrelated infrastructure noise.
   • If CI is flaky but the diff and coverage comparison are sound, ask for a rerun instead of blocking on speculation.

Decision Rules

Approve when all of these are true:

• The PR is scoped to the target existing library and the Java runtime failure it fixes.
• Tests still execute and assert the same meaningful library behavior after the runtime repair.
• Dynamic-access coverage does not drop between the previous and new tested versions, or any apparent drop is convincingly explained by a changed upstream API/runtime surface.
• Total metadata entry counts are not below the 25% severe-drop threshold, or the reduction is convincingly explained by a changed upstream API/runtime surface.
• Required metadata, compile, and Java runtime test checks are green.

Request changes when any of these are true:

• The fix makes JVM tests pass by deleting tests, skipping execution, swallowing the failing exception.
• The fix disables native-image behavior instead of using the NativeImageSupport.isUnsupportedFeatureError(e) catch pattern for unsupported open-ended dynamic class loading.
• Dynamic-access coverage drops without a credible explanation and replacement coverage.
• Total metadata entry count drops below 25% of the original without a credible explanation.
• CI failures indicate the Java runtime problem is not actually fixed.

Ask for follow-up instead of rejecting when:

• Stats needed for the old/new version comparison are missing or stale.
• CI failed in a way that looks like infrastructure noise.
• The API or runtime behavior change is plausible but the PR does not explain why a coverage drop is expected.
• Total metadata entry count drops below 25% of the original, but the changed upstream API/runtime surface makes the reduction plausible.

Output Style

Keep comments short and factual:

• For coverage drops: say that dynamic-access coverage must not regress between tested versions, cite the old and new values, and ask for either restored coverage or a concrete explanation.
• For metadata entry drops: report only drops where the new total metadata has fewer than 25% as many entries as the previous version's total metadata; cite the old and new counts, and ask for either restored metadata or a concrete explanation of the API/runtime-surface change.
• For deleted or bypassed coverage: say that the PR fixes Java runtime execution by removing coverage and should instead adapt the test to the new runtime behavior.
• For swallowed exceptions: say that catching or ignoring the failing exception hides the runtime failure instead of proving the library behavior works.
• For native skips that does not depend on the open-ended dynamic class loading: say that the PR avoids the failing native path instead of fixing it, so it does not demonstrate native-image runtime coverage.
• For unverified catch (Error): say that dynamic class loading tests should verify Native Image failures with NativeImageSupport.isUnsupportedFeatureError(e) and re-throw any other error.
• For version-pinned tests: say that tests should not reference the exact library version because the same test should support multiple library versions.
• For unrelated changes: say the PR should stay scoped to the fixes-java-run-fail repair and remove unrelated files.
• For legacy metadata files: say that metadata must use reachability-metadata.json and ask for old config files such as reflect-config.json or resource-config.json to be replaced.
• For missing stats: ask for regenerated library stats or CI evidence before approval.

Examples

Use these examples as representative patterns, not exhaustive matchers.

• Bad runtime repair that swallows the failure:

@Test
void readsConfiguration() {
    try {
        Configuration config = Configuration.load("name=value");
        assertThat(config.get("name")).isEqualTo("value");
    } catch (RuntimeException ignored) {
    }
}

• Bad native-image skip:

@Test
void readsConfiguration() {
    assumeFalse("runtime".equals(System.getProperty("org.graalvm.nativeimage.imagecode")));

    Configuration config = Configuration.load("name=value");
    assertThat(config.get("name")).isEqualTo("value");
}

• Approved dynamic-class-loading pattern:

@Test
void loadsRuntimePluginJar() throws Exception {
    Path pluginJar = createPluginJar();
    try {
        Plugin plugin = PluginLoader.load(pluginJar, "example.Plugin");
        assertThat(plugin.name()).isEqualTo("example");
    } catch (Error e) {
        if (!NativeImageSupport.isUnsupportedFeatureError(e)) {
            throw e;
        }
        // Expected: Native Image cannot load plugin JARs discovered after build.
    }
}

• Bad version-pinned assertion:

@Test
void reportsVersion() {
    assertThat(LibraryVersion.current()).isEqualTo("1.2.3");
}