Skip to content

Conversation

JinwooHwang
Copy link
Contributor

Summary

This change restores and stabilizes Gradle execution optimizations for the aggregated test reporting workflow by explicitly declaring all upstream Test tasks as both inputs and dependencies of the :combineReports TestReport task. It eliminates the prior Gradle validation warning caused by implicit consumption of test result outputs (notably from :geode-old-versions:test) and removes a redundant late graph mutation hook.

Background (Problem)

Gradle emitted a validation warning of the form: Task ':combineReports' uses the output of task ':geode-old-versions:test' without declaring an explicit or implicit dependency. Execution optimizations have been disabled.

Implications

  • Execution optimization for that unit was disabled ( reduced scheduling efficiency / potential loss of incremental behavior)
  • Potential nondeterministic ordering if task scheduling shifted
  • Noise in local and CI build logs

Root Cause

combineReports read binary test result directories generated by multiple subproject Test tasks without declaring a formal task dependency edge and input relationship. Gradle 7+ detects this implicit file coupling and defensively disables optimizations.

What Changed

  1. The combineReports task now:

    • Collects all subproject Test tasks.
    • Ensures explicit inclusion of :geode-old-versions:test.
    • Declares:
      • reportOn allTests (inputs)
      • dependsOn allTests (ordering)
    • Emits a location message after completion.
  2. Removed the redundant trailing gradle.taskGraph.whenReady { ... } block that previously re-applied the same reportOn / dependsOn wiring (no functional value, added noise).

  3. Retained the existing whenReady block.

Outcome

  • Gradle validation warning eliminated.
  • Execution optimizations re-enabled for :combineReports.
  • Report aggregation remains deterministic and complete.
  • Combined HTML report continues to appear at: build/reports/combined.

Verification (Performed / Expected)

  • Run ./gradlew clean build
  • Validate
    • No implicit dependency warning in console output.
    • :geode-old-versions:test executes before :combineReports.
    • All module test results appear in the combined report.
    • No new warnings introduced.

Risk Assessment

  • Low Risk: Change is purely declarative build graph augmentation; no alterations to test logic, filtering, or artifact paths.

Reviewer Checklist

  • No implicit dependency warnings in build output
  • combineReports defined only once; no duplicate wiring hook
  • Report generated at build/reports/combined
  • Old versions module results included
  • No unrelated build regressions

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

  • Has your PR been rebased against the latest commit within the target branch (typically develop)?

  • Is your initial contribution a single, squashed commit?

  • Does gradlew build run cleanly?

  • Have you written or updated unit tests to verify your changes?

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

@JinwooHwang JinwooHwang requested a review from raboof September 12, 2025 18:17
@JinwooHwang
Copy link
Contributor Author

JinwooHwang commented Sep 13, 2025

@raboof, all 17 checks passed without issues. Let me know if anything needs clarification or if you'd like to review together. Huge thanks for going through all the PRs. Your effort made a real difference, and we wouldn’t be where we are without your help. Much appreciated!

@JinwooHwang
Copy link
Contributor Author

Hi @raboof. Please don’t hesitate to reach out if anything looks unclear or if you'd prefer to walk through any part of the PR together. I really appreciate the time and care you’ve put into reviewing everything. Your support has been instrumental in getting us this far. Looking forward to your thoughts whenever you're ready.

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.

1 participant