Unify main and app output logs to improve the UX of failure investigation #1348
Conversation
| @@ -361,11 +361,15 @@ private async Task RunDeviceTests( | |||
| // We need to check for MT1111 (which means that mlaunch won't wait for the app to exit) | |||
| IFileBackedLog aggregatedLog = Log.CreateReadableAggregatedLog(_mainLog, testReporter.CallbackLog); | |||
|
|
|||
| // The app output log is not directly accessible. | |||
| // Hence, we duplicate the log to the main console log to simplify the UX of failure investigation | |||
| IFileBackedLog aggregatedLogAppOutput = Log.CreateReadableAggregatedLog(_mainLog, appOutputLog); | |||
There was a problem hiding this comment.
Does it make sense to keep the appOutputLog as a separate log file if its content is duplicated to the main log (console log)? We might scratch it out completely, but it would be a bigger change to xharness then.
There was a problem hiding this comment.
I would keep the file for now.
I don't think it is that expensive to keep it around and not sure if there is a dependency on its existence (I might be wrong).
As for the changes, I think they are looking good.
There was a problem hiding this comment.
I'd remove it eventually, it's a bit confusing having two files with the same data.
|
@ivanpovazan @akoeplinger What do you think of this solution? I would like to know your opinion on duplicating the app output log to the main (console) log before I proceed with finishing the other scenarios. |
|
sounds good to me. the console logs aren't particularly big right? (think hundreds of MBs) |
|
Thank you @ivanpovazan and @akoeplinger for the feedback. I'll proceed with the other platforms.
In general, I think the logs are < 1MB. We are already including the test outputs in simulators console logs, so we should be fine. |
matouskozak
force-pushed
the
unify-main-and-appOutputLog
branch
from
6ec42a1 to
9022ea7
Compare
matouskozak
force-pushed
the
unify-main-and-appOutputLog
branch
from
9022ea7 to
d0c1bda
Compare
matouskozak
force-pushed
the
unify-main-and-appOutputLog
branch
from
d0c1bda to
ac5d1a1
Compare
|
@ivanpovazan @akoeplinger I implemented the log unification for Android (emulator/device), iOS device and MacCatalyst. I don't really like that I had to do it 3 times (once for each platform) but I didn't find a single point where this could be achieved. An alternative solution could be to wait for the xharness to finish the test execution and then invoke a job that would collect the test output logs and copy them to the main console log. Please let me know what you think about this. |
I don't see a problem with this, I think this solution is good enough. |
|
FWIW, from the provided logs, one thing I noticed on Android is that for emulator logs we seem to enable GC logging while on the device we don't. Not sure why it is configured differently, just thinking that GC logging could introduce a lot of noise in the logs. |
There was a problem hiding this comment.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
matouskozak
changed the title
|
Let's backport this, so that we have better build analysis on servicing branches |
Background
Tests run on mobile platforms, don't report all outputs into the console log. This behavior renders using Build Analysis un-reliable for these scenarios because console log doesn't contain all information about the completed test. The full test output can be found inside the app output logs.
E.g., console log might contain message "ERROR: WORKLOAD TIMED OUT - Killing user command" but the device log can show more specific errors for the given test.
Proposed Solution
Duplicate the output from app log to the main log. This will increase the size of the console log but will allow Build Analysis to match greater variaty of patterns that are only available as part of app output logs + it speeds up the investigation experience as most of the information will be contained in the single log.
TODO
Future work
Remove the need for separate app output logs for android and iOS if we are duplicating their content to the main (console) log anyways.
Contributes to #1188