Conversation
There was a problem hiding this comment.
💡 Harness Review
The change gates Android log collection on Yamato’s command-block result while preserving the existing log destination and artifact collection behavior; I examined the batch control flow, invocation context, and failure paths.
Reviewed commit fbc5476
🤖 Helpful? 👍/👎
todi1856
left a comment
There was a problem hiding this comment.
Honestly I don't think it's good idea. In case of failure, you sometimes want to compare failed logcat against succesful logcat, thus is important to have logcat even in succesful builds too.
|
p.s did you consider clearing logcat before test run, that would make the log smaller, and might be the reason why it sometimes gets stuck |
The problem is that the fail rate is unacceptably high. Something like every other PR has at least one failure due to this. |
|
I cannot stop you from pushing such change, but in my opinion this is wrong, at the very least try clearing the logcat before test run, maybe it's simple as that for fixing this |
There have been multiple instances lately where capturing logcat gets stuck.
As a mitigation, capture logcat only if execution fails. If all tests are successful, skip it.