Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8253424: Add support for running pre-submit testing using GitHub Actions #189

Closed
wants to merge 9 commits into from

Conversation

@RealCLanger
Copy link
Contributor

@RealCLanger RealCLanger commented Aug 4, 2021

This is the backport of the GitHub Actions workflow to JDK11 Updates

It is close to the current state of the workflow in head openjdk/jdk, except for the following modifications:

  • No Windows Aarch64
  • No MacOs Aarch64
  • Used jtreg5.1-b01 vs. jtreg-6+1 (which means a slight modification for the steps that build it). This can/should probably be updated later
  • Use Adopt OpenJDK 11.0.11 as boot JDK (I don't know whether Adoptium 11.0.12 is ready. I guess this can be changed later on as well)
  • Computation of JDK_VERSION used in bundle names: It is different because 11u is an update release (e.g. we have to account for 11.0.13 vs. only 11)
  • Added a step "Fix jtreg permissions" because configure on Linux/Mac would bail out otherwise. I don't know why this is not needed in head. Maybe configure was improved?
  • Use gcc 9 vs. gcc 10 on Linux. I chose to take what we use in SAP's nightly tests for the moment. We could probably spend some effort later to update the code to be able to compile on gcc 10.
  • No gtest support
  • Make target for test runs is "run-test-prebuilt" vs. "test-prebuilt". There was some refactoring between 11 and 18.
  • MacOS bundle names contain 'osx' (vs 'macos' in head)

To make the pipeline work and results look greenish, I proposed 6 other backports to be done before merging this: #180, #181, #182, #185, #186 and #187.

There are probably a few more testfixes needed to further reduce testing noise.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issues

  • JDK-8253424: Add support for running pre-submit testing using GitHub Actions
  • JDK-8253865: Pre-submit testing using GitHub Actions does not detect failures reliably
  • JDK-8254054: Pre-submit testing using GitHub Actions should not use the deprecated set-env command
  • JDK-8254173: Add Zero, Minimal hotspot targets to submit workflow
  • JDK-8254175: Build no-pch configuration in debug mode for submit checks
  • JDK-8254282: Add Linux x86_32 builds to submit workflow
  • JDK-8255373: Submit workflow artifact name is always "test-results_.zip"
  • JDK-8255895: Submit workflow artifacts miss hs_errs/replays due to ZIP include mismatch
  • JDK-8256127: Add cross-compiled foreign architectures builds to submit workflow
  • JDK-8256277: Github Action build on macOS should define OS and Xcode versions
  • JDK-8256354: Github Action build on Windows should define OS and MSVC versions
  • JDK-8256414: add optimized build to submit workflow
  • JDK-8256393: Github Actions build on Linux should define OS and GCC versions
  • JDK-8256747: GitHub Actions: decouple the hotspot build-only jobs from Linux x64 testing
  • JDK-8257056: Submit workflow should apt-get update to avoid package installation errors
  • JDK-8259679: GitHub actions should use MSVC 14.28
  • JDK-8259924: GitHub actions fail on Linux x86_32 with "Could not configure libc6:i386"
  • JDK-8260460: GitHub actions still fail on Linux x86_32 with "Could not configure libc6:i386"
  • JDK-8263667: Avoid running GitHub actions on branches named pr/*
  • JDK-8255305: Add Linux x86_32 tier1 to submit workflow
  • JDK-8255352: Archive important test outputs in submit workflow

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk11u-dev pull/189/head:pull/189
$ git checkout pull/189

Update a local copy of the PR:
$ git checkout pull/189
$ git pull https://git.openjdk.java.net/jdk11u-dev pull/189/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 189

View PR using the GUI difftool:
$ git pr show -t 189

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk11u-dev/pull/189.diff

@RealCLanger RealCLanger changed the title GitHub Actions for JDK11 Updates Backport 8253424 Aug 4, 2021
@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Aug 4, 2021

👋 Welcome back clanger! A progress list of the required criteria for merging this PR into pr/187 will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

Loading

@openjdk openjdk bot changed the title Backport 8253424 8253424: Add support for running pre-submit testing using GitHub Actions Aug 4, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

This backport pull request has now been updated with the original issue, but not the original commit. If you have the original commit hash, please update the pull request title with Backport <hash>.

Loading

@mlbridge
Copy link

@mlbridge mlbridge bot commented Aug 4, 2021

Webrevs

Loading

@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 4, 2021

/issue add 8253865

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

@RealCLanger
Adding additional issue to issue list: 8253865: Pre-submit testing using GitHub Actions does not detect failures reliably.

Loading

@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 4, 2021

/issue add 8254054

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

@RealCLanger
Adding additional issue to issue list: 8254054: Pre-submit testing using GitHub Actions should not use the deprecated set-env command.

Loading

@shipilev
Copy link
Contributor

@shipilev shipilev commented Aug 4, 2021

Thank you for doing this! So we would also need to do followup additions (as separate backports), right?

8254282: Add Linux x86_32 builds to submit workflow
8254175: Build no-pch configuration in debug mode for submit checks
8254173: Add Zero, Minimal hotspot targets to submit workflow
8256414: add optimized build to submit workflow

Loading

@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 4, 2021

/issue add 8254173

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

@RealCLanger
Adding additional issue to issue list: 8254173: Add Zero, Minimal hotspot targets to submit workflow.

Loading

@shipilev
Copy link
Contributor

@shipilev shipilev commented Aug 4, 2021

Also, from the looks of it, "8256127: Add cross-compiled foreign architectures builds to submit workflow" is also included here.

Loading

@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 4, 2021

Thank you for doing this! So we would also need to do followup additions (as separate backports), right?

8254282: Add Linux x86_32 builds to submit workflow
8254175: Build no-pch configuration in debug mode for submit checks
8254173: Add Zero, Minimal hotspot targets to submit workflow
8256414: add optimized build to submit workflow

Yes, the state of the workflow is very close to what we have in head openjdk/jdk. I'll update the description with some details and mark the important JBS items that I find as backported. Stay tuned...

Loading

@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 4, 2021

/issue add 8254175

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

@RealCLanger
Adding additional issue to issue list: 8254175: Build no-pch configuration in debug mode for submit checks.

Loading

@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 4, 2021

/issue add 8254282

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

@RealCLanger
Adding additional issue to issue list: 8254282: Add Linux x86_32 builds to submit workflow.

Loading

@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 4, 2021

/issue add 8255373

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

@RealCLanger
Adding additional issue to issue list: 8255373: Submit workflow artifact name is always "test-results_.zip".

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

@RealCLanger
Adding additional issue to issue list: 8256747: GitHub Actions: decouple the hotspot build-only jobs from Linux x64 testing.

Loading

@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 4, 2021

/issue add 8260460

Loading

@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 4, 2021

/issue add 8263667

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

@RealCLanger
Adding additional issue to issue list: 8257056: Submit workflow should apt-get update to avoid package installation errors.

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

@RealCLanger
Adding additional issue to issue list: 8259679: GitHub actions should use MSVC 14.28.

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

@RealCLanger
Adding additional issue to issue list: 8259924: GitHub actions fail on Linux x86_32 with "Could not configure libc6:i386".

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

@RealCLanger
Adding additional issue to issue list: 8260460: GitHub actions still fail on Linux x86_32 with "Could not configure libc6:i386".

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 4, 2021

@RealCLanger
Adding additional issue to issue list: 8263667: Avoid running GitHub actions on branches named pr/*.

Loading

Copy link
Contributor

@jerboaa jerboaa left a comment

My preference would be to use Temurin binaries.

Loading

make/conf/test-dependencies Outdated Show resolved Hide resolved
Loading
Copy link
Contributor

@shipilev shipilev left a comment

This looks fine to me. I think these fixes are included too, but not linked to this PR:

8255305: Add Linux x86_32 tier1 to submit workflow
8255352: Archive important test outputs in submit workflow

Loading

@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 5, 2021

/issue add 8255305, 8255352

Loading

@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 5, 2021

This looks fine to me. I think these fixes are included too, but not linked to this PR:

8255305: Add Linux x86_32 tier1 to submit workflow
8255352: Archive important test outputs in submit workflow

Thanks, I added those as well.

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 5, 2021

@RealCLanger
Adding additional issue to issue list: 8255305: Add Linux x86_32 tier1 to submit workflow.

Adding additional issue to issue list: 8255352: Archive important test outputs in submit workflow.

Loading

jerboaa
jerboaa approved these changes Aug 5, 2021
Copy link
Contributor

@jerboaa jerboaa left a comment

I cannot comment on the workflow part (know too little about them), but the test-deps look good. Thumbs up!

Loading

@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 5, 2021

I cannot comment on the workflow part (know too little about them), but the test-deps look good. Thumbs up!

Thank you. I've flagged the bug for approval now.

Loading

@openjdk-notifier openjdk-notifier bot changed the base branch from pr/187 to master Aug 5, 2021
@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented Aug 5, 2021

The dependent pull request has now been integrated, and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork:

git checkout 8249095
git fetch https://git.openjdk.java.net/jdk11u-dev master
git merge FETCH_HEAD
# if there are conflicts, follow the instructions given by git merge
git commit -m "Merge master"
git push

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 5, 2021

@RealCLanger This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

8253424: Add support for running pre-submit testing using GitHub Actions
8253865: Pre-submit testing using GitHub Actions does not detect failures reliably
8254054: Pre-submit testing using GitHub Actions should not use the deprecated set-env command
8254173: Add Zero, Minimal hotspot targets to submit workflow
8254175: Build no-pch configuration in debug mode for submit checks
8254282: Add Linux x86_32 builds to submit workflow
8255373: Submit workflow artifact name is always "test-results_.zip"
8255895: Submit workflow artifacts miss hs_errs/replays due to ZIP include mismatch
8256127: Add cross-compiled foreign architectures builds to submit workflow
8256277: Github Action build on macOS should define OS and Xcode versions
8256354: Github Action build on Windows should define OS and MSVC versions
8256414: add optimized build to submit workflow
8256393: Github Actions build on Linux should define OS and GCC versions
8256747: GitHub Actions: decouple the hotspot build-only jobs from Linux x64 testing
8257056: Submit workflow should apt-get update to avoid package installation errors
8259679: GitHub actions should use MSVC 14.28
8259924: GitHub actions fail on Linux x86_32 with "Could not configure libc6:i386"
8260460: GitHub actions still fail on Linux x86_32 with "Could not configure libc6:i386"
8263667: Avoid running GitHub actions on branches named pr/*
8255305: Add Linux x86_32 tier1 to submit workflow
8255352: Archive important test outputs in submit workflow

Reviewed-by: sgehwolf, shade

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

Loading

@openjdk openjdk bot added the ready label Aug 5, 2021
@RealCLanger
Copy link
Contributor Author

@RealCLanger RealCLanger commented Aug 5, 2021

/integrate

Loading

@openjdk
Copy link

@openjdk openjdk bot commented Aug 5, 2021

Going to push as commit 1faefed.

Loading

@openjdk openjdk bot closed this Aug 5, 2021
@openjdk openjdk bot added integrated and removed ready labels Aug 5, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Aug 5, 2021

@RealCLanger Pushed as commit 1faefed.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Loading

@openjdk openjdk bot removed the rfr label Aug 5, 2021
@RealCLanger RealCLanger deleted the gha branch Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants