ov7a
commented
9 months ago Thanks for looking into this. I think this is a good idea - the tests should not depend on exact hashes.
Closed StefMa closed 9 months ago
ov7a
commented
9 months ago Thanks for looking into this. I think this is a good idea - the tests should not depend on exact hashes.
octylFractal
commented
9 months ago Yeah, I think this makes sense, and for the version assertion just asserting on the major version as well, e.g. at https://github.com/gradle/foojay-toolchains/blob/fe13a5ff77d2352332c67507b70eb379ecb734ae/foojay-resolver/src/test/kotlin/org/gradle/toolchains/foojay/FoojayApiTest.kt#L161 assert matches 11.\d+.\d+.
StefMa
commented
9 months ago Cool. Thanks for sharing your opinion 👍
I will tackle this if you don't mind. Watch out for an PR 🙃
On Wed, Jan 24, 2024, 5:37 PM Octavia Togami @.***> wrote:
Yeah, I think this makes sense, and for the version assertion just asserting on the major version as well, e.g. at https://github.com/gradle/foojay-toolchains/blob/fe13a5ff77d2352332c67507b70eb379ecb734ae/foojay-resolver/src/test/kotlin/org/gradle/toolchains/foojay/FoojayApiTest.kt#L161 assert matches 11.\d+.\d+.
— Reply to this email directly, view it on GitHub https://github.com/gradle/foojay-toolchains/issues/58#issuecomment-1908502127, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACOBQ62NXE2ET4VR27LSCB3YQE2DLAVCNFSM6AAAAABCIHUWPCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBYGUYDEMJSG4 . You are receiving this because you authored the thread.Message ID: @.***>
Right now the plugin have tests that checks the exact URL returning by the FoojayApi. See https://github.com/gradle/foojay-toolchains/blob/fe13a5ff77d2352332c67507b70eb379ecb734ae/foojay-resolver/src/test/kotlin/org/gradle/toolchains/foojay/FoojayApiTest.kt#L23-L27
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
(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? 🤔