KAFKA-17479 Fail the whole pipeline if junit step times out [4/n]

[mumrah]

Follow up from #17106, this patch fixes a bug in the workflow where the tests timeout but no JUnit errors are found. Since the test report is incomplete, this is a false positive for a build being green. We should fail the whole workflow if the JUnit tests timeout.

[mumrah]

I have tested the timeout case on mumrah/kafka here https://github.com/mumrah/kafka/actions/runs/10745303070/job/29804106047?pr=13#step:6:71

This is a perfect example since we do actually see some test reports being processed

Did not find done file 'build/junit-xml/done'. These are partial results!
1057 tests cases run in 31s. 1057 PASSED ✅, 0 FAILED ❌, 0 FLAKY ⚠️ , 0 SKIPPED 🙈, and 0 errors.
Failing this step because done file was missing.

The timeout error is coalesced into the Parse JUnit Tests step:

And, most importantly, the link from the PR takes us to the JUnit summary instead of the actual Test step. This is important because the Test step usually has 10s of thousands of log lines and often gets truncated in the UI. Linking to the summary step is much more user friendly.

Screen.Recording.2024-09-06.at.5.32.07.PM.mp4

[chia7712]

@mumrah thanks for this patch. one small question PTAL

[chia7712]

out of curiosity, why not set default value like build/junit-xml/done?

[mumrah]

I wanted this to be an optional argument so its easier to test locally. I don't feel too strongly about this, either way seems fine

[chia7712]

The "--path" has default value, so that is why I raise this comment 😃

For another trivial reason, we can simplify the command passed to junit parser if it has default value.

[mumrah]

Adding a finalizer to the default "test" task did not work as expected. I can't find a simple way to add task to run after all the subproject ":test" tasks, so I changed this to have the GH workflow explicitly call createDoneFile

[chia7712]

Adding a finalizer to the default "test" task did not work as expected. I can't find a simple way to add task to run after all the subproject ":test" tasks, so I changed this to have the GH workflow explicitly call createDoneFile

Could we check the create time of build/.../xml instead?

1. If there is no xml file -> timeout
2. If any xml's creation time is too old -> timeout

[mumrah]

Could we check the create time of build/.../xml instead?

The trouble is, we may have some of the copied XML files, but not all of them. For example, if all the tests finished except for :core we would have several XML files in build/junit-xml, but not any from core.

---

The whole problem we're working around here is that there's no easy way to distinguish between a timeout and some other failure in a GitHub step (a major limitation, IMO). Another solution could be to use the Linux timeout command to do our own timeout. With this approach, we just watch for a particular exit code from timeout to determine if Gradle had an error or if it timed out.

It would be something like:

timeout 180 ./gradlew ...
RET=$?
if [[ $RET == 124 ]]; then 
  echo "timed out";
elif [[ $RET == 0 ]]; then 
  echo "ok"; 
else 
  echo "failed"; 
fi

Maybe we could capture the exit code and store it as a step output to be read by the Parse JUnit step. WDYT?

[chia7712]

The trouble is, we may have some of the copied XML files, but not all of them. For example, if all the tests finished except for :core we would have several XML files in build/junit-xml, but not any from core.

I meant we can check creation time of xml file (from finished tests) in parsing and throw error if one of them has "too old" creation time.

Maybe we could capture the exit code and store it as a step output to be read by the Parse JUnit step. WDYT?

Sound good. Overall it would be better to avoid complicating build.gradle 😄

[mumrah]

@chia7712 I took advice from this thread https://github.com/orgs/community/discussions/46992#discussioncomment-4961541 to capture the exit code. I tested it with a 5s timeout here https://github.com/apache/kafka/actions/runs/10752630233/job/29821166851, seems to work.

I think this is a better solution than fiddling with files.

[chia7712]

LGTM but it would be better to test timeout again before merging :)