1553515 - Make geckoview-junit pass reliably on x86_64

Status: RESOLVED FIXED

(GeckoView :: General, defect, P2)

Description

[Agi Sferro | :agi | Employee until July 1st | he/him]

Our arm tests run in the emulator which is really slow (about 30-45 times slower than native x86_64 emulator). We should move to x86_64 but right now the tests do not pass. This tracks work to make them pass reliably and switch over.

Comment 1

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Don't use reflection in ContentDelegateTest.

Comment 2

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Wait to load a page in crashContentMultipleSessions.

Comment 3

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Use environment.defaultTimeoutMillis in tests.

Comment 4

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Account for errors in PermissionDelegateTest.geolocation.

Sometime the geolocation test fails because the position is not retrieved
quickly enough, this is OK as we're just trying to make sure that the
permission is correct.

Comment 5

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Use javascript to track scrolling instead of ScrollDelegate.

Comment 6

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Never reuse session.

Reusing sessions doesn't actually save that much time and avoids a lot of
hard-to-debug intermittents.

Comment 7

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Add FOREGROUND_SERVICE for CrashTest.

Comment 8

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Use lambads in GeckoResultTest for readability.

Comment 9

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Add waitForCondition.

Comment 10

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Add GeckoResult.accept.

Comment 11

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Replace loopUntilIdle -> waitForCondition.

Comment 12

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Make sure runtime has started before running tests.

Comment 13

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Call geckoview.startup on DOMContentLoaded.

Believe it or not this helps with getting PageStop events more reliably.

Comment 14

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Always send navigation events for about:blank.

Comment 15

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Use WebExtension to run content javascript in tests.

Comment 16

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Fix for SessionLifecycleTest intermittent.

Comment 17

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Use .equals instead of == for GeckoSession.

Comment 18

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Add GeckoSessionTestRule.createFromParcel.

This method makes so that we don't add a GeckoSession to a HashMap before
changing its hashCode() by calling readFromParcel.

Comment 19

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Fix for ContentDelegateTest.autoFill_navigation.

Comment 20

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Use WebExtension to set prefs in tests.

Comment 21

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Move getLinkColor to WebExtension.

Comment 22

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Remove all remaining bits of RDP for tests.

Comment 23

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Wait for link color in HistoryDelegateTest.getVisited.

This helps making this test more reliable.

Comment 24

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Remove assert in accessibility code.

Comment 25

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Raise default timeout to 30s.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80512ab53f44
Call geckoview.startup on DOMContentLoaded. r=geckoview-reviewers,snorp

Comment 27

[James Willcox (:snorp) ([e-mail]) (he/him)]

Adding 'leave-open', since I only landed one patch in the series.

Comment 28

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/80512ab53f44

Comment 29

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Re-enable geckoview-junit on x86_64.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2889a14d1858
Don't use reflection in ContentDelegateTest. r=snorp
https://hg.mozilla.org/integration/autoland/rev/5435965d4c04
Wait to load a page in crashContentMultipleSessions. r=snorp
https://hg.mozilla.org/integration/autoland/rev/9fca72dc9b74
Use environment.defaultTimeoutMillis in tests. r=snorp
https://hg.mozilla.org/integration/autoland/rev/ac9c129bfbcc
Account for errors in PermissionDelegateTest.geolocation. r=snorp
https://hg.mozilla.org/integration/autoland/rev/316439eec048
Use javascript to track scrolling instead of ScrollDelegate. r=snorp
https://hg.mozilla.org/integration/autoland/rev/b2b2a5d55e93
Never reuse session. r=snorp
https://hg.mozilla.org/integration/autoland/rev/a44d58f58ce4
Add FOREGROUND_SERVICE for CrashTest. r=snorp
https://hg.mozilla.org/integration/autoland/rev/c4287e1c1485
Use lambads in GeckoResultTest for readability. r=snorp
https://hg.mozilla.org/integration/autoland/rev/5a9d7367edb7
Add waitForCondition. r=snorp
https://hg.mozilla.org/integration/autoland/rev/9a058acc45e8
Add GeckoResult.accept. r=snorp
https://hg.mozilla.org/integration/autoland/rev/7a1fb1222e52
Replace loopUntilIdle -> waitForCondition. r=snorp
https://hg.mozilla.org/integration/autoland/rev/879397260d90
Make sure runtime has started before running tests. r=snorp

Comment 31

[Andreea Pavel [:apavel]]

Backed out for android gradle failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=879397260d9090878d5c5586cbadcbbd944a3b13&selectedJob=253760963

Failure logs: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=253760963&repo=autoland&lineNumber=3105
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=253762820&repo=autoland&lineNumber=2984

Backout: https://hg.mozilla.org/integration/autoland/rev/1c2e073c6478def06e986c01fa93653ac518f879

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c29929cc67c
Don't use reflection in ContentDelegateTest. r=snorp
https://hg.mozilla.org/integration/autoland/rev/2b1f78e255ce
Wait to load a page in crashContentMultipleSessions. r=snorp
https://hg.mozilla.org/integration/autoland/rev/3d1a0d412dfd
Use environment.defaultTimeoutMillis in tests. r=snorp
https://hg.mozilla.org/integration/autoland/rev/9e9dc0e7afb3
Account for errors in PermissionDelegateTest.geolocation. r=snorp
https://hg.mozilla.org/integration/autoland/rev/d3734e6fe22a
Use javascript to track scrolling instead of ScrollDelegate. r=snorp
https://hg.mozilla.org/integration/autoland/rev/84414ac99629
Never reuse session. r=snorp
https://hg.mozilla.org/integration/autoland/rev/dbbb8fe1ecfb
Add FOREGROUND_SERVICE for CrashTest. r=snorp
https://hg.mozilla.org/integration/autoland/rev/98cf5b0cadf1
Use lambads in GeckoResultTest for readability. r=snorp
https://hg.mozilla.org/integration/autoland/rev/faa333bb64e2
Add waitForCondition. r=snorp
https://hg.mozilla.org/integration/autoland/rev/8fe9f5d7fef9
Add GeckoResult.accept. r=snorp
https://hg.mozilla.org/integration/autoland/rev/bbc184402f36
Replace loopUntilIdle -> waitForCondition. r=snorp
https://hg.mozilla.org/integration/autoland/rev/080d770c7b35
Make sure runtime has started before running tests. r=snorp

Comment 33

[Agi Sferro | :agi | Employee until July 1st | he/him]

My bad sorry. Fixed and relanded.

Comment 34

[Andreea Pavel [:apavel]]

Backed out for increasing gv-junit intermittent failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=080d770c7b35ebbb52c48a025c60fc74ce599dfe&selectedJob=253790197

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=253790197&repo=autoland&lineNumber=1913

Backout: https://hg.mozilla.org/integration/autoland/rev/b6e6ade7d550efa0de6a1b73919e785f69887768

*** bug 1546297 was backed out because it was causing conflicts, then relanded in the same push.

Comment 35

[Agi Sferro | :agi | Employee until July 1st | he/him]

Looks like we need to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1533888 before landing this.

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/604f9f7fa587
Don't use reflection in ContentDelegateTest. r=snorp
https://hg.mozilla.org/integration/autoland/rev/36e8ca1b4b70
Wait to load a page in crashContentMultipleSessions. r=snorp
https://hg.mozilla.org/integration/autoland/rev/2f2d90fccda9
Use environment.defaultTimeoutMillis in tests. r=snorp
https://hg.mozilla.org/integration/autoland/rev/27c75ff5fd96
Account for errors in PermissionDelegateTest.geolocation. r=snorp
https://hg.mozilla.org/integration/autoland/rev/f48fe89dbedd
Use javascript to track scrolling instead of ScrollDelegate. r=snorp
https://hg.mozilla.org/integration/autoland/rev/b53f9d993a20
Never reuse session. r=snorp
https://hg.mozilla.org/integration/autoland/rev/f753025bb55b
Add FOREGROUND_SERVICE for CrashTest. r=snorp
https://hg.mozilla.org/integration/autoland/rev/6054b4310a45
Use lambads in GeckoResultTest for readability. r=snorp
https://hg.mozilla.org/integration/autoland/rev/ce691a380b23
Add waitForCondition. r=snorp
https://hg.mozilla.org/integration/autoland/rev/42a44c814b6f
Add GeckoResult.accept. r=snorp
https://hg.mozilla.org/integration/autoland/rev/547ff97b194e
Replace loopUntilIdle -> waitForCondition. r=snorp
https://hg.mozilla.org/integration/autoland/rev/c5039a265a46
Make sure runtime has started before running tests. r=snorp
https://hg.mozilla.org/integration/autoland/rev/67f2e46bdb98
Use WebExtension to run content javascript in tests. r=snorp
https://hg.mozilla.org/integration/autoland/rev/03f9dfc360de
Use .equals instead of == for GeckoSession. r=snorp
https://hg.mozilla.org/integration/autoland/rev/94a6bc1d5443
Add GeckoSessionTestRule.createFromParcel. r=snorp
https://hg.mozilla.org/integration/autoland/rev/67dbdef6e00a
Fix for ContentDelegateTest.autoFill_navigation. r=snorp
https://hg.mozilla.org/integration/autoland/rev/b03a8aee7acc
Use WebExtension to set prefs in tests. r=snorp
https://hg.mozilla.org/integration/autoland/rev/56f413baf041
Move getLinkColor to WebExtension. r=snorp
https://hg.mozilla.org/integration/autoland/rev/fa0cb607fe0d
Remove all remaining bits of RDP for tests. r=snorp
https://hg.mozilla.org/integration/autoland/rev/99faa668dfa0
Wait for link color in HistoryDelegateTest.getVisited. r=snorp
https://hg.mozilla.org/integration/autoland/rev/059dc56460e0
Re-enable geckoview-junit on x86_64. r=gbrown
https://hg.mozilla.org/integration/autoland/rev/cabf177300f4
Always send navigation events for about:blank. r=snorp

Comment 37

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/604f9f7fa587
https://hg.mozilla.org/mozilla-central/rev/36e8ca1b4b70
https://hg.mozilla.org/mozilla-central/rev/2f2d90fccda9
https://hg.mozilla.org/mozilla-central/rev/27c75ff5fd96
https://hg.mozilla.org/mozilla-central/rev/f48fe89dbedd
https://hg.mozilla.org/mozilla-central/rev/b53f9d993a20
https://hg.mozilla.org/mozilla-central/rev/f753025bb55b
https://hg.mozilla.org/mozilla-central/rev/6054b4310a45
https://hg.mozilla.org/mozilla-central/rev/ce691a380b23
https://hg.mozilla.org/mozilla-central/rev/42a44c814b6f
https://hg.mozilla.org/mozilla-central/rev/547ff97b194e
https://hg.mozilla.org/mozilla-central/rev/c5039a265a46
https://hg.mozilla.org/mozilla-central/rev/67f2e46bdb98
https://hg.mozilla.org/mozilla-central/rev/03f9dfc360de
https://hg.mozilla.org/mozilla-central/rev/94a6bc1d5443
https://hg.mozilla.org/mozilla-central/rev/67dbdef6e00a
https://hg.mozilla.org/mozilla-central/rev/b03a8aee7acc
https://hg.mozilla.org/mozilla-central/rev/56f413baf041
https://hg.mozilla.org/mozilla-central/rev/fa0cb607fe0d
https://hg.mozilla.org/mozilla-central/rev/99faa668dfa0
https://hg.mozilla.org/mozilla-central/rev/059dc56460e0
https://hg.mozilla.org/mozilla-central/rev/cabf177300f4

Comment 38

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Don't run geckoview-junit on arm opt.

geckoview-junit now runs on x86_64 opt so we don't need to run the tests on arm
too.

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c3c0710eb53
Don't run geckoview-junit on arm opt. r=gbrown

Comment 40

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/8c3c0710eb53

Comment 42

[Chris Peterson [:cpeterson]]

I'm editing a bunch of GeckoView bugs. If you'd like to filter all this bugmail, search and destroy emails containing this UUID:

e88a5094-0fc0-4b7c-b7c5-aef00a11dbc9

Comment 43

[Agi Sferro | :agi | Employee until July 1st | he/him]

Attachment: Bug 1553515 - Enable geckoview-test on debug x86_64.

Comment 44

[Agi Sferro | :agi | Employee until July 1st | he/him]

This is the last patch in this bug \o/

Comment 45

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0a70ad667f71
Enable geckoview-test on debug x86_64. r=snorp

Comment 46

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/0a70ad667f71

Comment 47

[Chris Peterson [:cpeterson]]

69=wontfix because we don't want to uplift all these test changes to Beta.

Comment 48

[Geoff Brown [:gbrown]]

:Agi -- https://hg.mozilla.org/integration/autoland/rev/a44d58f58ce4 added FOREGROUND_SERVICE to the test app's AndroidManifest.xml; should that also be added to the list of 'pm grant' permissions used by test harnesses at:

https://searchfox.org/mozilla-central/rev/42c2ecdc429115c32e6bcb78bf087a228a051044/testing/mozbase/mozdevice/mozdevice/adb.py#3116-3121

?

Comment 49

[Agi Sferro | :agi | Employee until July 1st | he/him]

Yeah, it looks like we should do that too.