ti-chi-bot[bot]
commented
3 weeks ago I have already done a preliminary review for you, and I hope to help you do a better job.
Based on the pull request title and description, it seems that the author is trying to implement a feature to collect failure logs after a test timeout. Looking at the diff, it seems that the change is only in the pull_lightning_integration_test.groovy file. The change is in the post section where the failure keyword is replaced with unsuccessful.
The change seems to be in the right direction as it is trying to collect logs after a test timeout. However, the change from failure to unsuccessful needs to be reviewed more carefully. The failure keyword is used to catch all types of failures, including build failures, test failures, and other types of failures. The unsuccessful keyword, on the other hand, is only used to catch test failures.
It is possible that the change from failure to unsuccessful might cause some logs to be missed if the failure is not a test failure. So, it is recommended to test the change thoroughly before merging it.
One possible fix would be to keep the failure keyword and add an additional step to catch test failures. For example, the following code can be added to catch test failures:
post {
// Catch test failures
always {
junit 'target/surefire-reports/*.xml'
}
// Collect logs
failure {
sh label: "collect logs", script: """
ls /tmp/lightning_test
tar -cvzf log-${TEST_GROUP}.tar.gz \$(find /tmp/lightning_test/ -type f -name "*.log")
"""
}
}This way, all types of failures will be caught, and the logs will be collected after any type of failure.
purelind
try to collect failure log after test timeout