Skip to content

Conversation

matouskozak
Copy link
Member

@matouskozak matouskozak commented Mar 24, 2025

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
Copy link
Member

@ivanpovazan ivanpovazan left a comment

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

@matouskozak
Copy link
Member Author

matouskozak commented Mar 25, 2025

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
Member Author

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

Copy link
Member

@akoeplinger akoeplinger left a comment

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
Member Author

/ba-g failure is a known issue

@matouskozak matouskozak merged commit ef94f7f into dotnet:main Mar 26, 2025
15 of 17 checks passed
@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.
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