New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 847948 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS , Fuchsia
Pri: 1
Type: Bug-Regression



Sign in to add a comment

ios_chrome_external_url_egtest testLanguageDetectionHttpContentLanguage failing on device

Project Member Reported by mrefaat@chromium.org, May 30 2018

Issue description

ios_chrome_external_url_egtests have been failing on device for more than a week.

I've tried reproducing on device but i wasn't able to.

For some reason LoadURL for invalid URL doesn't complete in the test on bots.

Paster with error log:
https://paste.googleplex.com/5088197964988416

 
Labels: -Type-Bug -Pri-3 Pri-1 Type-Bug-Regression
It might just be this needs more time to load on devices on the swarm?

This bug is used to disable 2 tests. Can we include the log for the second failure?

Comment 4 by mrefaat@google.com, May 30 2018

https://paste.googleplex.com/5480326331629568 this is the log for the second failure
Project Member

Comment 5 by bugdroid1@chromium.org, May 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/83d66a752c609d0a977364f6ce16cc9c8dd8d000

commit 83d66a752c609d0a977364f6ce16cc9c8dd8d000
Author: mrefaat <mrefaat@chromium.org>
Date: Wed May 30 21:24:29 2018

Disable Failing ios Chrome external url eg tests on device

These tests have been failing for more than a week now. LoadURL doesn't
finish on device on some tests.

Bug:  847948 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I151a3a2d031b4311d17c5c148c635a92e5a1f232
Reviewed-on: https://chromium-review.googlesource.com/1079630
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562999}
[modify] https://crrev.com/83d66a752c609d0a977364f6ce16cc9c8dd8d000/ios/chrome/browser/metrics/external_url_tab_usage_recorder_egtest.mm
[modify] https://crrev.com/83d66a752c609d0a977364f6ce16cc9c8dd8d000/ios/chrome/browser/translate/translate_egtest.mm

Cc: -linds...@chromium.org huangml@chromium.org
Owner: linds...@chromium.org
assigning to lindsayw@ as she is going to own external url tests 
Thanks for owning that.
Cc: kariahda@chromium.org
Hi all - was this possibly fixed by c#2? 
No this is not fixed yet.
CL linked to this bug disabled 2 tests:
 - testLanguageDetectionHttpContentLanguage
 - testEvictedTabReloadFailure

Instead of fixing testEvictedTabReloadFailure, I think we should rewrite it as a unit test. 

Sylvain, do you know if we need testLanguageDetectionHttpContentLanguage? Is there anything that we test in external URL test, that is not covered by different tests?
We should be able to rewrite testEvictedTabReloadFailure to use EmbeddedTestServer:
 https://bugs.chromium.org/p/chromium/issues/detail?id=852341&desc=2#c21

That's easier than writing a unit test from scratch.
Cc: sdefresne@chromium.org
Any update here? Should rewriting these tests block M68? Should this blocking Beta instead?
We should understand if test failures indicate the issue with the product. Blocking M68 seems late.
Cc: -huangml@chromium.org pkl@chromium.org cma...@chromium.org
After comment 10, is testLanguageDetectionHttpContentLanguage the only test that is failing?

Does the failing test indicate anything that is actually broken that would/should block M68 release?
Labels: -M-68 M-69
Punting this as it's not ready for 68.
Cc: olivierrobin@chromium.org stkhapugin@chromium.org noyau@chromium.org
Summary: ios_chrome_external_url_egtest testLanguageDetectionHttpContentLanguage failing on device (was: ios_chrome_external_url_egtests are failing on device)
For testEvictedTabReloadFailure, let's consider  issue 852341  the tracker for that. I'm definitely not the right owner for metrics tests, and this test is indeed failing on device and simulator. Olivier, please let me know if you have any questions.

I'll try to review the translate test case even though I'm also not the owner for translate. in -owners stk@ is mentioned as a potential translate owner, so cc'ing him here. 

Changing title to reflect that this is only about reviewing testLanguageDetectionHttpContentLanguage. 
I'll send out a cl to fix the in code crbug reference for testEvictedTabReloadFailure in  as well such that it will only point to  issue 852341  and I'll update  issue 852341  to be for both simulator and device.
Project Member

Comment 17 by bugdroid1@chromium.org, Jul 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f44a47ef657a9aefcf740c91866038624a1f0e32

commit f44a47ef657a9aefcf740c91866038624a1f0e32
Author: Lindsay Pasricha <lindsayw@google.com>
Date: Wed Jul 25 16:29:02 2018

Fix disable comment to point to only one tracker for both device and simulator failure.

Bug:  847948 , 852341 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I77f63093d642a51193787641568a639f80d5c5e1
Reviewed-on: https://chromium-review.googlesource.com/1148796
Commit-Queue: Lindsay Pasricha <lindsayw@chromium.org>
Reviewed-by: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577925}
[modify] https://crrev.com/f44a47ef657a9aefcf740c91866038624a1f0e32/ios/chrome/browser/metrics/external_url_tab_usage_recorder_egtest.mm

Out of about 50 test runs of testLanguageDetectionHttpContentLanguage on local device, I've only been able to get it to fail once, and it, like the paste above, just hit the timeout waiting for the page to load (waitForPageToFinishLoading) after calling loadURL(). I'll look into the best options for that issue.


Since I've landed https://chrome-internal-review.googlesource.com/c/chrome/ios_internal/+/658155 I'd like to try reenabling this test to see if the flakiness is resolved now.
Project Member

Comment 20 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/032102f00a32d672fb197a99ae9e04af8a81c693

commit 032102f00a32d672fb197a99ae9e04af8a81c693
Author: Lindsay Pasricha <lindsayw@google.com>
Date: Thu Aug 02 01:26:21 2018

Reenable testLanguageDetectionHttpContentLanguage on device.

Bug:  847948 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I1aa8d2b06ccbe326beb9e7832cee69d60882b25c
Reviewed-on: https://chromium-review.googlesource.com/1159193
Commit-Queue: Lindsay Pasricha <lindsayw@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580024}
[modify] https://crrev.com/032102f00a32d672fb197a99ae9e04af8a81c693/ios/chrome/browser/translate/translate_egtest.mm

Status: Fixed (was: Assigned)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-69; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-69 label, otherwise remove Merge-TBD label. Thanks.
Since this is EG test, confirming this doesn't need merge?
No this doesn't need a merge as it was an issue with the devices on the waterfall at most what I could do is merge changes to help fix the beta-device bot but since we are low on devices until the new ones ordered get plugged in I can't even say for sure that merges there will make that bot green. Short answer - no merge needed at this time.
Labels: -Merge-TBD

Sign in to add a comment