1433322 - [geckoview] Move GeckoView unit tests to under geckoview/

Status: RESOLVED FIXED

(GeckoView :: General, enhancement)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

Right now we have some unit tests under Fennec that really belong in GeckoView because they test GeckoView code.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1433322 - 1. Move publicsuffix utils to under Fennec;

The publicsuffix utils are not used in GeckoView and more importantly,
their accompanying asset is only included in Fennec and not GeckoView.
Therefore, it makes sense to move the utils to Fennec.

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

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1433322 - 2. Mass move GeckoView unit tests to under geckoview/;

These tests involve GeckoView classes, so move them to under geckoview/.
We use a custom test runner for Fennec unit tests, but I didn't notice
any problems when using standard test runners (e.g. MockitoJUnitRunner),
so I changed these tests to use standard runners.

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

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1433322 - 3. Run GeckoView unit tests with 'mach android test';

Run unit tests under geckoview/ when running 'mach android test'. This
also lets us run those tests on Taskcluster.

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

Comment 4

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8945640 [details]
Bug 1433322 - 1. Move publicsuffix utils to under Fennec;

https://reviewboard.mozilla.org/r/215770/#review221498

This will fail, since moz.build still exists -- so that needs to be addressed.  But I think this is the right home for this functionality!

Comment 5

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8945641 [details]
Bug 1433322 - 2. Mass move GeckoView unit tests to under geckoview/;

https://reviewboard.mozilla.org/r/215772/#review221500

I don't quite follow the test runner details, but I'm happy for you to own this!

Comment 6

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8945642 [details]
Bug 1433322 - 3. Run GeckoView unit tests with 'mach android test';

https://reviewboard.mozilla.org/r/215774/#review221502

As usual, r- is f+.

Three things:

1) my guess is that test failures don't print anything right now, since https://searchfox.org/mozilla-central/source/mobile/android/mach_commands.py#86 needs to be enlarged.  You can test this by introducing failures and running |mach android test| locally.
2) https://searchfox.org/mozilla-central/source/taskcluster/ci/build/android-stuff.yml#19 needs to be enlarged, so that reports are shared.
3) this will conflict a little with my patches for https://bugzilla.mozilla.org/show_bug.cgi?id=1411654, which I'm working hard to get landed.  In particular, those patches change some of the variant names, and update the Robolectric bits.  Can you try to build on top of that series?

Thanks for digging in!

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8945640 [details]
Bug 1433322 - 1. Move publicsuffix utils to under Fennec;

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

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8945641 [details]
Bug 1433322 - 2. Mass move GeckoView unit tests to under geckoview/;

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

Comment 9

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8945642 [details]
Bug 1433322 - 3. Run GeckoView unit tests with 'mach android test';

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

Comment 10

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8945642 [details]
Bug 1433322 - 3. Run GeckoView unit tests with 'mach android test';

https://reviewboard.mozilla.org/r/215774/#review223122

Thanks for catching some errors, which I no doubt introduced in my 3.0 patch!  I'm going to be more careful about empty input sets in the future, 'cuz you make a good point.  Bombs away!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f7862b1b5e9b
1. Move publicsuffix utils to under Fennec; r=nalexander
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a74b6699ff3
2. Mass move GeckoView unit tests to under geckoview/; r=nalexander
https://hg.mozilla.org/integration/mozilla-inbound/rev/e91adf8f9294
3. Run GeckoView unit tests with 'mach android test'; r=nalexander

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/f7862b1b5e9b
https://hg.mozilla.org/mozilla-central/rev/0a74b6699ff3
https://hg.mozilla.org/mozilla-central/rev/e91adf8f9294