Issue metadata
Sign in to add a comment
|
ios_chrome_external_url_egtest testLanguageDetectionHttpContentLanguage failing on device |
||||||||||||||||||||||
Issue descriptionios_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
,
May 30 2018
It might just be this needs more time to load on devices on the swarm?
,
May 30 2018
This bug is used to disable 2 tests. Can we include the log for the second failure?
,
May 30 2018
https://paste.googleplex.com/5480326331629568 this is the log for the second failure
,
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
,
May 30 2018
assigning to lindsayw@ as she is going to own external url tests Thanks for owning that.
,
Jun 7 2018
Hi all - was this possibly fixed by c#2?
,
Jun 7 2018
No this is not fixed yet.
,
Jun 25 2018
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?
,
Jun 25 2018
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.
,
Jun 28 2018
,
Jul 2
Any update here? Should rewriting these tests block M68? Should this blocking Beta instead?
,
Jul 11
We should understand if test failures indicate the issue with the product. Blocking M68 seems late.
,
Jul 16
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?
,
Jul 17
Punting this as it's not ready for 68.
,
Jul 24
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.
,
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
,
Jul 31
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.
,
Aug 1
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.
,
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
,
Aug 2
,
Aug 2
[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.
,
Aug 16
Since this is EG test, confirming this doesn't need merge?
,
Aug 16
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.
,
Aug 16
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by mrefaat@chromium.org
, May 30 2018