Fix MarshalMethods startup logcat race in device test - #12195
Merged
Conversation
MarshalMethodsGCHangTests previously launched the app via RunProjectAndAssert and only then began logcat monitoring, which made the startup marker check racey under CI log volume. This change starts adb logcat monitoring first, then launches MainActivity via a callback once monitoring is attached, and clears logcat immediately before startup monitoring to keep the buffer focused on startup lines. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c491d761-523c-40ad-9374-4897fc12d51b
Contributor
There was a problem hiding this comment.
Pull request overview
Improves reliability of the MarshalMethodsAppRuns(CoreCLR) device test by eliminating a logcat timing race where the test could miss a one-shot startup marker on busy CI devices.
Changes:
- Clear logcat before the startup assertion and start logcat monitoring before launching
MainActivity. - Extend
MonitorAdbLogcatto accept anonMonitoringStartedcallback so callers can trigger actions only after monitoring begins.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/MSBuildDeviceIntegration/Tests/MarshalMethodsGCHangTests.cs | Clears logcat and launches the app via a callback after logcat monitoring starts to avoid missing startup output. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/DeviceTest.cs | Adds onMonitoringStarted callback support to MonitorAdbLogcat to allow “attach-before-run” monitoring patterns. |
Ensure MonitorAdbLogcat always stops and waits for the adb logcat process even when onMonitoringStarted throws. Capture callback exceptions, run process cleanup in a finally path, then rethrow with original stack trace. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c491d761-523c-40ad-9374-4897fc12d51b
Use a single try/finally in MonitorAdbLogcat so onMonitoringStarted exceptions propagate naturally while adb logcat cleanup is always executed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c491d761-523c-40ad-9374-4897fc12d51b
jonathanpeppers
added
the
ready-to-review
rolfbjarne
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
This test flake was caused by a timing race:
MarshalMethodsAppRuns(CoreCLR)launched the app first and only then began logcat monitoring forXXX:OnStart done. On busy CI devices, that one-shot startup marker could be missed even when startup was successful.This change starts logcat monitoring first, then launches
MainActivityvia anonMonitoringStartedcallback so the listener is attached before startup logs are emitted. It also clears logcat immediately before this startup check to keep the buffer focused on current startup output.dotnet build src\Xamarin.Android.Build.Tasks\Xamarin.Android.Build.Tasks.csproj -v:minimaldotnet build tests\MSBuildDeviceIntegration\MSBuildDeviceIntegration.csproj -v:minimal -p:BuildProjectReferences=falsedotnet test bin\TestDebug\MSBuildDeviceIntegration\net10.0\MSBuildDeviceIntegration.dll --filter Name~MarshalMethodsAppRuns(ran on device; failed earlier in app Build/Install before reaching the updated startup assertion path)