Skip to content

Cache the maven wrapper when building on GHI - #51445

Open
ahus1 wants to merge 1 commit into
keycloak:mainfrom
ahus1:is-51444-cache-maven-wrapper
Open

Cache the maven wrapper when building on GHI#51445
ahus1 wants to merge 1 commit into
keycloak:mainfrom
ahus1:is-51444-cache-maven-wrapper

Conversation

@ahus1

@ahus1 ahus1 commented Aug 5, 2026

Copy link
Copy Markdown
Member

Closes #51444

This also removes the cross-OS settings, as the archive is now OS specific

@ahus1 ahus1 self-assigned this Aug 5, 2026
@ahus1
ahus1 force-pushed the is-51444-cache-maven-wrapper branch 3 times, most recently from 59c39f8 to 3879563 Compare August 5, 2026 06:24
@ahus1
ahus1 requested a balanced review from Copilot August 5, 2026 07:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Caches Maven wrapper distributions to reduce CI download failures.

Changes:

  • Adds Maven wrapper distributions to the OS-specific Maven cache.
  • Enables Windows cache creation and removes cross-OS restoration.
  • Adds batch mode to Windows-capable Quarkus unit tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/ci.yml Documents Windows wrapper behavior and adds Maven batch mode.
.github/actions/maven-cache/action.yml Caches Maven wrapper distributions using OS-specific keys.
Suppressed comments (1)

.github/actions/maven-cache/action.yml:35

  • Include the wrapper properties hash in this key. Because caches are immutable, changing distributionUrl during the same weekly window still hits the old cache and prevents the newly downloaded Maven distribution from being saved, so every job must download it again and can reproduce the failure this change targets.
        key: mvn-and-wrapper-${{ runner.os }}-${{ steps.weekly-cache-key.outputs.key }}

Comment thread .github/actions/maven-cache/action.yml
@ahus1
ahus1 marked this pull request as ready for review August 5, 2026 07:30
@ahus1
ahus1 requested a review from a team as a code owner August 5, 2026 07:30
@ahus1
ahus1 requested a review from Pepo48 August 5, 2026 07:30
@ahus1

ahus1 commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

@Pepo48 - I hope this additional caching and cleanup makes this more robust. Please let me know if you agree with this approach. Thanks!

Copilot AI review requested due to automatic review settings August 6, 2026 00:29
@ahus1
ahus1 force-pushed the is-51444-cache-maven-wrapper branch from 3879563 to d8c35b7 Compare August 6, 2026 00:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/actions/maven-cache/action.yml:48

  • The comment above this restore path still says the hybrid cache strategy is used, although this step now restores only the exact OS-specific key. Please align the comment with the new behavior.
        key: mvn-and-wrapper-${{ runner.os }}-${{ steps.weekly-cache-key.outputs.key }}

.github/actions/maven-cache/action.yml:30

  • The adjacent comment still describes a hybrid cross-OS fallback, but this key is now OS-specific and the fallback settings were removed. Update it to avoid documenting behavior that no longer exists.

This issue also appears on line 48 of the same file.

        key: mvn-and-wrapper-${{ runner.os }}-${{ steps.weekly-cache-key.outputs.key }}

Closes keycloak#51444

Signed-off-by: Alexander Schwartz <alexander.schwartz@gmx.net>
Copilot AI review requested due to automatic review settings August 6, 2026 01:16
@ahus1
ahus1 force-pushed the is-51444-cache-maven-wrapper branch from d8c35b7 to 6b387a6 Compare August 6, 2026 01:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@keycloak-github-bot

Copy link
Copy Markdown

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.webauthn.passwordless.WebAuthnPwdLessPropertyTest#timeout

Keycloak CI - WebAuthn IT

org.openqa.selenium.WebDriverException: 
unknown error: unhandled inspector error: {"code":-32000,"message":"Node with given id does not belong to the document"}
  (Session info: chrome=150.0.7871.128)
Build info: version: '4.28.1', revision: '73f5ad48a2'
System info: os.name: 'Linux', os.arch: 'amd64', os.version: '6.17.0-1020-azure', java.version: '25.0.3'
...

Report flaky test

@keycloak-github-bot keycloak-github-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@Pepo48 Pepo48 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like a net improvement. I haven't found any problems with the implementation.

Therefore, approving. Thanks, @ahus1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cache the maven wrapper when building on GHI

4 participants