gradle/foojay-toolchains

Fix flaky tests \ Improve FoojayApiTest

StefMa opened this issue · 3 comments

StefMa commented

Right now the plugin have tests that checks the exact URL returning by the FoojayApi.
See

foojay-toolchains/foojay-resolver/src/test/kotlin/org/gradle/toolchains/foojay/FoojayApiTest.kt

Lines 23 to 27 in fe13a5f

fun `download URI provided correctly`() {
assertDownloadUri(
"https://api.foojay.io/disco/v3.0/ids/c2457a42e8aa37d56d4564214dff9a85/redirect",
8, any(), false, OperatingSystem.MAC_OS, Architecture.AARCH64
) // zulu8.74.0.17-ca-fx-jdk8.0.392-macosx_aarch64.tar.gz

These tests fail quite often because the tests requesting only a major JDK version (8; in this case) but the foojay api returns always the latest exact version of this major version.
So in case there is a new patch or minor bump, the redirect URL is a different one.

Therefore, each time a new JDK version got "accepted"/"available" at foojay, the tests here will fail.

A few pull requests come accross this issue already:

Additionally, the tests contain a comment to the version they expect.
It is likely that these will be outated if not carefully updated together with the redirect url.

However, this should be fixed to have stable tests.
Maybe a simple regex on the URL would make sense, excluding the "hash" in the URL in the assertion part 🤔
So maybe something like this

https://api\.foojay\.io/disco/v3\.0/ids/(?![\da-f]{32}\b)[\w/-]+/redirect

(I used ChatGPT here, not sure if it works. Also didn't tested it and didn't thought about it 😂 But you get the point I guess. Simply checking if the "hash" is alphanumeric and 32 chars long. The rest is hardcoded.)

What do you think? 🤔

ov7a commented

Thanks for looking into this. I think this is a good idea - the tests should not depend on exact hashes.

  • 👍1
octylFractal commented

Yeah, I think this makes sense, and for the version assertion just asserting on the major version as well, e.g. at

foojay-toolchains/foojay-resolver/src/test/kotlin/org/gradle/toolchains/foojay/FoojayApiTest.kt

Line 161 in fe13a5f

assertEquals("11.0.21", p.distribution_version)
assert matches 11.\d+.\d+.

  • 👍1
StefMa commented
  • 👍1