besu icon indicating copy to clipboard operation
besu copied to clipboard

build and run testDocker on PRs

Open macfarla opened this issue 5 months ago • 1 comments

PR description

Previously testDocker task was only run on merge to main

This PR starts with a commit to reproduce the issue with errorprone check not being picked up until buildDocker (merge to main), and then modifies the workflows so that those checks run at PR stage.

  • change was made to BlobType (remove final keyword) to trigger the original issue
  • extra workflow file develop-pr.yml was added to run the relevant checks (build, testDocker but not publish) on PR
  • fixes to build.gradle to ensure testDocker can work without docker env which was also supplying docker org and artifact name (in addition to docker creds which are not needed since we're not publishing)

For some reason the errorprone check only fails on besu-arm runner not the ubuntu-arm one or ubuntu-22-04

eg on this run you can see the errorprone check fails the build for 1 of 2 runners https://github.com/hyperledger/besu/actions/runs/15864950425?pr=8846

and once the final keyword is fixed, both runners succeed

Also errorprone test added to confirm this case (final keyword within enum constructor) is covered https://github.com/besu-eth/besu-errorprone-checks/pull/13

Fixed Issue(s)

fixes #8730

Thanks for sending a pull request! Have you done the following?

  • [ ] Checked out our contribution guidelines?
  • [ ] Considered documentation and added the doc-change-required label to this PR if updates are required.
  • [ ] Considered the changelog and included an update if required.
  • [ ] For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • [ ] spotless: ./gradlew spotlessApply
  • [ ] unit tests: ./gradlew build
  • [ ] acceptance tests: ./gradlew acceptanceTest
  • [ ] integration tests: ./gradlew integrationTest
  • [ ] reference tests: ./gradlew ethereum:referenceTests:referenceTests

macfarla avatar Jun 24 '25 05:06 macfarla

so the error prone PR https://github.com/besu-eth/besu-errorprone-checks/pull/13 fixes the root cause that was flagged initially here - so this besu PR would be just if we want to run build & test docker on PRs rather than just on merge to main. I would argue it's worth it if it doesn't slow down the PR build time. It takes 5 min and it's run in parallel. Maybe we would get enough signal from running on 1 of 2 runners

macfarla avatar Jun 25 '25 21:06 macfarla