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

[net9.0] Revert "Move HybridWebView platform code to handlers" #24876

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

rmarinho
Copy link
Member

@rmarinho rmarinho commented Sep 23, 2024

Reverts #24479

cc: @Eilon , seems this makes the android device tests for hybrid hang on Android in all apis.

@rmarinho rmarinho requested a review from a team as a code owner September 23, 2024 15:03
@rmarinho rmarinho requested review from jfversluis, jsuarezruiz, PureWeen and Eilon and removed request for jsuarezruiz September 23, 2024 15:03
@Eilon
Copy link
Member

Eilon commented Sep 23, 2024

I've run these locally many times in a row on Android 33 and no hangs... very odd.

@Eilon
Copy link
Member

Eilon commented Sep 23, 2024

Hmmm is this PR suffering the same fate?

https://dev.azure.com/xamarin/public/_build/results?buildId=124121&view=logs&j=e3945674-f282-5725-23bc-c22abe46bc97&t=134b0d27-7c89-584b-9750-46cfc158dd4d&l=233

Android Controls_API_33

warn: Running instrumentation class com.microsoft.maui.controls.devicetests.TestInstrumentation timed out after waiting 900.1171838 seconds

@PureWeen
Copy link
Member

Hmmm is this PR suffering the same fate?

https://dev.azure.com/xamarin/public/_build/results?buildId=124121&view=logs&j=e3945674-f282-5725-23bc-c22abe46bc97&t=134b0d27-7c89-584b-9750-46cfc158dd4d&l=233

Android Controls_API_33

warn: Running instrumentation class com.microsoft.maui.controls.devicetests.TestInstrumentation timed out after waiting 900.1171838 seconds

Yea, I noticed this as well

Looking at the logcat it seems like this one is also hanging once it gets to hybridwebviewtests.
I've completely disabled them to see if that fixes.

It might be a test that's before it locking things up, we'll see how this run goes

image

@Eilon
Copy link
Member

Eilon commented Sep 23, 2024

@PureWeen looks like it passed that test leg now. Maybe the timeout is just too short?

On this successful run the times from the log said:

2024-09-23T18:18:39.2809200Z Task                          Duration            
2024-09-23T18:18:39.2809940Z --------------------------------------------------
2024-09-23T18:18:39.2815090Z Setup                         00:01:48.5090484    
2024-09-23T18:18:39.2815820Z build                         00:00:58.1761111    
2024-09-23T18:18:39.2816310Z test                          00:11:28.2679811    
2024-09-23T18:18:39.2817280Z Teardown                      00:00:08.8453491    
2024-09-23T18:18:39.2817890Z --------------------------------------------------
2024-09-23T18:18:39.2852210Z Total:                        00:14:23.7984897  

So that's almost 14.5 minutes, which seems just shy of the 900sec=15min timeout that the tests are set to. And the HybridWebView tests can easily take a few seconds a piece, times ~7 tests (so 20-30sec), which I think could make it exactly take too long if you include random bad luck/slowness.

So I wonder if the timeout should be increased from 900 to say 1800 or even more to give buffer for another few years? And for now it would help us detect actual hangs that are 'permanent' and not merely being slow.

@rmarinho rmarinho merged commit c6ca951 into net9.0 Sep 24, 2024
119 of 121 checks passed
@rmarinho rmarinho deleted the revert-24479-eilon/hwv-platform-cleanup branch September 24, 2024 10:56
@samhouts samhouts added the fixed-in-net9.0-nightly This may be available in a nightly release! label Oct 1, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixed-in-net9.0-nightly This may be available in a nightly release!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants