search

adoptium / TKG

TestKitGen (TKG)
Apache License 2.0
18 stars 96 forks source link

KEEP_REPORTDIR does not work as expected #156

Open llxia opened 3 years ago

llxia commented 3 years ago

When KEEP_REPORTDIR is set to false, we should not capture any passed test artifacts. However, this does not work as expected. Please see the build link below: https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_extended.functional_s390x_linux_aot_Personal_testList_0/4/

It looks like all test artifacts are captured.

I think this is due to test pipeline uses boolean https://github.com/AdoptOpenJDK/openjdk-tests/blob/e55242930ee3bd56ceb2cbdcb898df130fbfc9c9/buildenv/jenkins/JenkinsfileBase#L78 https://github.com/AdoptOpenJDK/openjdk-tests/blob/master/buildenv/jenkins/JenkinsfileBase#L264

and TKG uses string https://github.com/AdoptOpenJDK/TKG/blob/b7d19e42ab2c131d71ba037304b6efd8e07d5ad5/settings.mk#L195

llxia commented 3 years ago

We think we should change this line first https://github.com/AdoptOpenJDK/openjdk-tests/blob/e55242930ee3bd56ceb2cbdcb898df130fbfc9c9/buildenv/jenkins/JenkinsfileBase#L78

llxia commented 3 years ago

@OscarQQ Could you help with this issue? Thanks

smlambert commented 3 years ago

Given https://github.com/AdoptOpenJDK/openjdk-tests/pull/2348 is now merged, can this be closed?

llxia commented 3 years ago

Not really. There are missing changes in https://github.com/AdoptOpenJDK/openjdk-tests/blob/master/buildenv/jenkins/testJobTemplate#L37 and build scripts. For example: https://github.com/eclipse/openj9/blob/bb13191b0bebc16d69eebfb72293e904858375da/buildenv/jenkins/common/variables-functions.groovy#L783 https://github.com/AdoptOpenJDK/ci-jenkins-pipelines/blob/a57a15186003e0990d94ceb06102ff2100666be3/pipelines/build/common/openjdk_build_pipeline.groovy#L292

We should not mix string and boolean value.

smlambert commented 3 years ago

Indeed. I was looking through to tag good first issues, ones where there is a clear step by step description of the changes required. I will leave this open, but not tagged as 'good first issue' for now.