Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tests][mobile] Enable diagnostic_tracing for mobile runtime tests #60879

Merged
merged 7 commits into from
Feb 10, 2022

Conversation

mdh1418
Copy link
Member

@mdh1418 mdh1418 commented Oct 26, 2021

Before merging this, dotnet/diagnostics needs to be updated to accept a string parameter to the DiagnosticsClient constructor and allow for TCPIP connection. After which, MicrosoftDiagnosticsNETCoreClientVersion can be bumped. The tests work locally because a modified manual build of dotnet/diagnostics was used.

This PR looks to stand-up diagnostics tests for mobile, specifically targeting Android. Diagnostics runtime components have been made compatible for mobile platforms, and here, they will be tested and integrated.

The following are changes made:
Includes diagnostics_tracing runtime components in BuildAndroidApp
Sets the DiagnosticsPorts to default to 127.0.0.1:9000,nosuspend,listen
Fix IpcUtils GetStandardTransport to set up TCP/IP connection on Android, connecting to default DiagnosticsPorts
Fixes #52763
Disables JitOptimizationSensitive on Android for eventpipe tests


With a goal of emulating the diagnostics tracing tests on mono desktop, the following tests now pass locally.

eventlistener

eventlistener

eventcounter

eventcounter
pollingcounter
regression-46938
regression-25709
incrementingpollingcounter
runtimecounters
incrementingeventcounter
gh53564

eventactivityidcontrol

eventactivityidcontrol


Of the eventpipe tests, 5 of them will be enabled in a separate PR, and the rest require a significant modification to allow Android to run multiple apps at a time, specifically to allow for RunSubprocess in IpcUtils to work.

name_config_with_pid, reverse, reverseouter, pauseonstart, diagnosticport, processenvironment

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@fanyang-mono
Copy link
Member

With Steve's latest change to reduce the amount of runtime tests running per PR, I believe that all runtime tests running on mobile targets have been moved to rolling build. You would need to modify the runtime.yml and/or runtime-stage.yml files to validate your change here.

@mdh1418
Copy link
Member Author

mdh1418 commented Dec 6, 2021

/azp run runtime

@azure-pipelines
Copy link

Azure Pipelines failed to run 1 pipeline(s).

@mdh1418 mdh1418 force-pushed the enable_mobile_diagnostics_tests branch from ecdf812 to 2f5dce9 Compare December 6, 2021 06:47
Copy link
Member

@lateralusX lateralusX left a comment

Choose a reason for hiding this comment

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

Once this builds and pass, could we also validate that the tests are actually executed on CI as expected?

src/tests/tracing/eventpipe/common/IpcUtils.cs Outdated Show resolved Hide resolved
src/tests/tracing/eventpipe/common/IpcUtils.cs Outdated Show resolved Hide resolved
@mdh1418 mdh1418 marked this pull request as ready for review December 8, 2021 16:24
@mdh1418 mdh1418 force-pushed the enable_mobile_diagnostics_tests branch from cd09c58 to cbf9611 Compare December 9, 2021 16:26
@steveisok
Copy link
Member

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ghost
Copy link

ghost commented Jan 5, 2022

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR looks to stand-up diagnostics tests for mobile, specifically targeting Android. Diagnostics runtime components have been made compatible for mobile platforms, and here, they will be tested and integrated.

The following are changes made:
Includes diagnostics_tracing runtime components in BuildAndroidApp
Sets the DiagnosticsPorts to default to 127.0.0.1:9000,nosuspend,listen
Fix IpcUtils GetStandardTransport to set up TCP/IP connection on Android, connecting to default DiagnosticsPorts
Fixes #52763
Removing AdditionalXHarnessArguments configuration from Android as that broke local Android runtime runs.
Disable COMPlus_TieredCompilation on Android because of SKIPPING EXECUTION BECAUSE COMPlus_TieredCompilation has not been disabled and this test is marked JitOptimizationSensitive
Whitespace cleanup


With a goal of emulating the diagnostics tracing tests on mono desktop, the following tests now pass locally.

eventlistener

eventlistener

eventcounter

eventcounter
pollingcounter
regression-46938
regression-25709
incrementingpollingcounter
runtimecounters
incrementingeventcounter
gh53564

eventactivityidcontrol

eventactivityidcontrol

eventpipe

processinfo
processinfo2


The rest of the eventpipe tests require a little more effort to replace legacy RuntimeClient code and will be tackled in a follow up PR.

buffersize, eventsourceerror, bigevent, name_config_with_pid, reverse, reverseouter, pauseonstart, diagnosticport, processenvironment

Author: mdh1418
Assignees: -
Labels:

area-System.Diagnostics, os-android

Milestone: -

Mitchell Hwang added 2 commits February 4, 2022 15:37
Reenables eventactivityidcontrol now that diagnostics_tracing runtime component is included
@mdh1418 mdh1418 force-pushed the enable_mobile_diagnostics_tests branch from ec86c65 to 56907eb Compare February 4, 2022 20:40
@mdh1418
Copy link
Member Author

mdh1418 commented Feb 4, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mdh1418 mdh1418 force-pushed the enable_mobile_diagnostics_tests branch from 4ae52d5 to 3b8ef23 Compare February 7, 2022 17:03
@mdh1418
Copy link
Member Author

mdh1418 commented Feb 7, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mdh1418
Copy link
Member Author

mdh1418 commented Feb 8, 2022

Can I get another look over this PR @lateralusX, all the tests in the updated PR description are passing on CI.

@lateralusX
Copy link
Member

@mdh1418 How did you verify that the enabled tests now runs on Android on CI? Are we running the tests on all Android platforms currently tested on CI?

I believe this looks overall good, great to get the test running on mobile platforms.

@mdh1418
Copy link
Member Author

mdh1418 commented Feb 8, 2022

@mdh1418 How did you verify that the enabled tests now runs on Android on CI? Are we running the tests on all Android platforms currently tested on CI?

I believe this looks overall good, great to get the test running on mobile platforms.

I looked through the Android runtime tests lane on this PR's CI checks runtime-extra-platforms and in PayloadGroup0, I was able to find all of the tests in https://helix.dot.net/api/jobs/ed8baac3-e67c-47aa-95d8-0376b284f7a3/workitems/PayloadGroup0?api-version=2019-06-17 and looked through the adb log for a successful output of 100.

eventlistener
eventcounter
pollingcounter
regression-46938
regression-25709
incrementingpollingcounter
runtimecounters
incrementingeventcounter
gh53564
eventactivityidcontrol

However, I only checked that one lane so far Build Android x64 Release AllSubsets_Mono_RuntimeTests

@fanyang-mono
Copy link
Member

LGTM!

@mdh1418
Copy link
Member Author

mdh1418 commented Feb 8, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mdh1418
Copy link
Member Author

mdh1418 commented Feb 9, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mdh1418
Copy link
Member Author

mdh1418 commented Feb 9, 2022

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lateralusX
Copy link
Member

@mdh1418 Can you verify that the last config in this PR now runs the test on Android? If so I believe this is good to go!

@mdh1418
Copy link
Member Author

mdh1418 commented Feb 10, 2022

@lateralusX Just checked the lane Build Android x64 Release AllSubsets_Mono_RuntimeTests again, all the respective tests have a successful exit code of 100.

@mdh1418 mdh1418 merged commit 1c80769 into dotnet:main Feb 10, 2022
@mdh1418 mdh1418 deleted the enable_mobile_diagnostics_tests branch February 10, 2022 20:36
@ghost ghost locked as resolved and limited conversation to collaborators Mar 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Mono Android][Test] tracing/eventactivityidcontrol/eventactivityidcontrol fails on Android
6 participants