1253381 - Create JavascriptBridgeTest and port existing tests

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

We have some code duplication in tests that use JavascriptBridge where they set up JavascriptBridge in `setUp` & `tearDown` – it'd be great to unify that.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1253381 - Add JavascriptBridgeTest. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/37975/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37975/

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1253381 - Add JavascriptTest javadoc to explain test. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/37977/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37977/

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1253381 - Move existing JavascriptBridge tests to JavascriptBridgeTest. r=sebastian

Review commit: https://reviewboard.mozilla.org/r/37979/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37979/

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8726408 [details]
MozReview Request: Bug 1253381 - Add JavascriptBridgeTest. r=sebastian

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37975/diff/1-2/

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8726409 [details]
MozReview Request: Bug 1253381 - Add JavascriptTest javadoc to explain test. r=sebastian

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37977/diff/1-2/

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8726410 [details]
MozReview Request: Bug 1253381 - Move existing JavascriptBridge tests to JavascriptBridgeTest. r=sebastian

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37979/diff/1-2/

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7edde4fa1fbb

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8726408 [details]
MozReview Request: Bug 1253381 - Add JavascriptBridgeTest. r=sebastian

https://reviewboard.mozilla.org/r/37975/#review35009

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/JavascriptBridgeTest.java:25
(Diff revision 2)
> +    // A protected field for easy access by sub-classes and a private field so
> +    // we can close the JS bridge if the sub-classes re-assign the protected field.
> +    // Not very Java-like but simpler than a method.
> +    private JavascriptBridge _js;
> +    protected JavascriptBridge js;

Wouldn't it be simpler to just expose this via a getter method? This would prevent re-assigning.

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8726409 [details]
MozReview Request: Bug 1253381 - Add JavascriptTest javadoc to explain test. r=sebastian

https://reviewboard.mozilla.org/r/37977/#review35011

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8726410 [details]
MozReview Request: Bug 1253381 - Move existing JavascriptBridge tests to JavascriptBridgeTest. r=sebastian

https://reviewboard.mozilla.org/r/37979/#review35015

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://reviewboard.mozilla.org/r/37975/#review35009

> Wouldn't it be simpler to just expose this via a getter method? This would prevent re-assigning.

I was hoping to make accessing the `JavascriptBridge` easier via `js` rather than `getJS()`, but it's confusing and breaks convention – I'll add the getter.

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/97f90224e529b0f3d9828a860ff9114b21c8971a
Bug 1253381 - Add JavascriptBridgeTest. r=sebastian

https://hg.mozilla.org/integration/fx-team/rev/6d663a8715cf1441e237b2a0569e4426ac76bde3
Bug 1253381 - Add JavascriptTest javadoc to explain test. r=sebastian

https://hg.mozilla.org/integration/fx-team/rev/41f8ef49436f3bd46b2f858c254f9c821a523b1d
Bug 1253381 - Move existing JavascriptBridge tests to JavascriptBridgeTest. r=sebastian

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/97f90224e529
https://hg.mozilla.org/mozilla-central/rev/6d663a8715cf
https://hg.mozilla.org/mozilla-central/rev/41f8ef49436f