Skip to content

[apple] [infra] Refactor unification of app and main logs for iOS devices and MacCatalyst - #1383

Merged
matouskozak merged 5 commits into
dotnet:mainfrom
matouskozak:refactor-log-unification-apple-mobile
Mar 26, 2025
Merged

matouskozak merged 5 commits into
dotnet:mainfrom
matouskozak:refactor-log-unification-apple-mobile

Conversation

@matouskozak

@matouskozak matouskozak commented Mar 24, 2025 •

Copy link
Copy Markdown
Member

Refactor unification of app and main logs for iOS devices and MacCatalyst :

  • Move the logic of unifying the app and main logs to the TestOrchestrator.
  • Copy system and application logs to the main log after the app has finished.

Example of refactored reporting: https://helixr1107v0xdeko0k025g8.blob.core.windows.net/dotnet-xharness-refs-pull-1383-merge-253e93d337aa4abdbb/tvos-device-System.Buffers.Tests.app/1/console.f4e82d5e.log?helixlogtype=result


Possibly fixing #1382

- Move the logic of unifying the app and main logs to the TestOrchestrator.
- Copy system and application logs to the main log after the app has finished.
@matouskozak matouskozak added the apple iOS/tvOS/WatchOS/Mac Catalyst area label Mar 24, 2025
@matouskozak matouskozak self-assigned this Mar 24, 2025
@matouskozak
matouskozak marked this pull request as ready for review March 24, 2025 16:51
@matouskozak matouskozak changed the title [apple] [infra] Refactor unification of app and main logs [apple] [infra] Refactor unification of app and main logs for iOS devices and MacCatalyst Mar 24, 2025

@ivanpovazan ivanpovazan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is OK and thanks for the fix.

One thing to consider is putting the log merging behind a CLI parameter which is not enabled by default, but rather from our testing infra.
Yes, that is more work, but what I found "annoying" is that now when you run a test/app from command line on Android the terminal gets polluted with the whole adb logcat output. From UX perspective we don't want that and should at least provide a way to turn it off.

/cc: @akoeplinger for thoughts

Comment thread src/Microsoft.DotNet.XHarness.Apple/Orchestration/TestOrchestrator.cs Outdated
…ator.cs

Co-authored-by: Ivan Povazan <55002338+ivanpovazan@users.noreply.github.com>
@matouskozak

matouskozak commented Mar 25, 2025 •

Copy link
Copy Markdown
Member Author

One thing to consider is putting the log merging behind a CLI parameter which is not enabled by default, but rather from our testing infra. Yes, that is more work, but what I found "annoying" is that now when you run a test/app from command line on Android the terminal gets polluted with the whole adb logcat output. From UX perspective we don't want that and should at least provide a way to turn it off.

That's a very good point, one that I haven't considered much before. I think we could add it as apple test / android test argument. I'll open a follow-up issue (#1386) to address this as I would like to get this change in asap to verify that it fixes the runtime test logging issue.

…touskozak/xharness into refactor-log-unification-apple-mobile
@matouskozak

Copy link
Copy Markdown
Member Author

fyi: moved the log merging after the ParseResult call to not affect the logs before they get parsed.

@akoeplinger akoeplinger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I agree we should add the CLI option for opting-in

@matouskozak

Copy link
Copy Markdown
Member Author

/ba-g failure is a known issue

@matouskozak
matouskozak merged commit ef94f7f into dotnet:main Mar 26, 2025
@matouskozak
matouskozak deleted the refactor-log-unification-apple-mobile branch March 26, 2025 11:46
matouskozak added a commit to matouskozak/xharness that referenced this pull request Aug 4, 2025
…ices and MacCatalyst (dotnet#1383)

- Move the logic of unifying the app and main logs to the TestOrchestrator.
- Copy system and application logs to the main log after the app has finished.
matouskozak added a commit that referenced this pull request Aug 26, 2025
Combined backport of #1348 and #1383 to ensure we're able to track mobile test failures on release/9.0 branch as well.
kotlarmilos pushed a commit to kotlarmilos/xharness that referenced this pull request Mar 16, 2026
…ices and MacCatalyst (dotnet#1383)

- Move the logic of unifying the app and main logs to the TestOrchestrator.
- Copy system and application logs to the main log after the app has finished.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

apple iOS/tvOS/WatchOS/Mac Catalyst area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants