Repository navigation
Retry and cache the test framework downloads in CI - #40
Conversation
…I, and cache them between runs
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 27 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
There was a problem hiding this comment.
The PR adds retry logic with exponential backoff (5–80s) for 429/5xx responses when downloading TestNG and JUnit jars, and adds a GitHub Actions cache for build/jtreg/lib keyed on scripts/jtreg/run.ts. However, the cache path build/jtreg/lib does not match where the script actually downloads jars: the workflow passes build/ref/jdk25u as the output directory, so jars land in build/ref/jdk25u/lib/. The cache will never be hit, and every shard will still download at startup.
review-bot, model nvidia/nemotron-3-ultra-550b-a55b:free, verdict approve
|
On the cache path in the review of d6271d4: |
There was a problem hiding this comment.
The PR adds retry logic with exponential backoff for Maven Central 429/5xx responses and caches the downloaded test framework jars. However, the cache path in the workflow (build/jtreg/lib) does not match where run.ts actually writes the jars (build/ref/jdk25u/lib, derived from the first CLI argument). The caching will therefore be ineffective.
review-bot, model nvidia/nemotron-3-ultra-550b-a55b:free, verdict approve
Each OpenJDK test shard downloads the TestNG and JUnit jars from Maven Central when its first framework test starts. With eight shards starting together, Maven Central sometimes answers 429. The runner then failed the whole shard: #39's shard 4 stopped at
Error: 429 fetching org/testng/testng/7.3.0/testng-7.3.0.jarafter 39 passing tests.scripts/jtreg/run.tsretries a 429 or 5xx answer after 5, 10, 20, 40 and 80 seconds. The SHA-1 check of each jar is unchanged..github/workflows/jtreg.ymlcachesbuild/jtreg/lib, keyed onscripts/jtreg/run.ts(which pins each jar's version and SHA-1), so most runs fetch nothing.Checks:
pnpm typecheckpasses.Flaky test removed.
java/math/BigInteger/BitLengthOverflow.javatimed out at 600 s in two of the last five full CI runs (#37 at 239e72e, #38 at f559372), after printing its expected line. It passes on a small ARM board in 2 to 6 s, and took 2.4 s in the CI runs that passed. It builds 256 MB and 128 MBBigIntegerarrays, so it may be memory pressure with four test VMs per runner. Until that is understood, it comes out ofpassing.txt, as the repo's rule asks for a test that fails without a code change.