1335429 - auto-hyphenation reftest fails if MOZ_EXCLUDE_HYPHENATION_DICTIONARIES is set

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect, P2)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

In a last test before enabling the hyphenation dictionary download (DLC) I noticed that auto-hyphenation-* reftests are failing:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d55c84cbba4c0d80635e6b342baebd7001ba2c65&selectedJob=73262023

This shouldn't be happening. In bug 1276560 we added code to add the dictionaries to the test profile. But it looks like we never landed the code to actually load the dictionaries from the profile directory. See snippet in bug 1276560 comment 3.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Actually the code for that landed (in bug 1280757):
https://hg.mozilla.org/mozilla-central/rev/dc96e5ff02fd

I wonder why this is failing now.

Comment 2

[Jonathan Kew [:jfkthame]]

Interestingly, if I apply the patch from bug 1285752 to a local build, and run reftests on my Nexus device, the hyphenation tests work fine; the harness successfully adds the dictionaries to the profile, and Gecko loads them. But pushing the same thing to tryserver still fails.

Not sure why it behaves differently in automation vs locally...

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

It might be worth looking at logcat (grep for "DLC") to see whether our service is downloading the dictionaries in your case (in automation we prevent this, see bug 1334562).

Another possibility might be that you still have downloaded dictionaries from a previous non-test run of the application. We store them in a profile-agnostic directory - so just starting the app might download the dictionaries and then they are still available when running a reftest. Clearing app data and not running the app manually might prevent this.

Comment 4

[Jonathan Kew [:jfkthame]]

I'll do some further testing, but I don't think that's it. I added some logging to runreftest.py, and when I run locally, I see it pushing the dictionaries to the profile:

REFTEST INFO | copying tree /Users/jkew/mozdev/mozilla-central/mobile/android/fonts to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/fonts
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/af/hyphenation/hyph_af.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/bg/hyphenation/hyph_bg.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/ca/hyphenation/hyph_ca.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/cy/hyphenation/hyph_cy.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/da/hyphenation/hyph_da.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/de-1901/hyphenation/hyph_de-1901.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/de-1996/hyphenation/hyph_de-1996.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/de-CH/hyphenation/hyph_de-CH.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/en-US/hyphenation/hyph_en_US.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/eo/hyphenation/hyph_eo.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/es/hyphenation/hyph_es.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/et/hyphenation/hyph_et.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/fi/hyphenation/hyph_fi.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/fr/hyphenation/hyph_fr.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/gl/hyphenation/hyph_gl.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/hr/hyphenation/hyph_hr.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/hsb/hyphenation/hyph_hsb.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/hu/hyphenation/hyph_hu.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/ia/hyphenation/hyph_ia.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/is/hyphenation/hyph_is.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/it/hyphenation/hyph_it.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/kmr/hyphenation/hyph_kmr.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/la/hyphenation/hyph_la.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/lt/hyphenation/hyph_lt.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/mn/hyphenation/hyph_mn.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/nb/hyphenation/hyph_nb.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/nl/hyphenation/hyph_nl.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/nn/hyphenation/hyph_nn.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/pl/hyphenation/hyph_pl.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/pt/hyphenation/hyph_pt.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/ru/hyphenation/hyph_ru.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/sh/hyphenation/hyph_sh.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/sl/hyphenation/hyph_sl.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/sv/hyphenation/hyph_sv.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/tr/hyphenation/hyph_tr.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying file /Users/jkew/mozdev/mozilla-central/intl/locales/uk/hyphenation/hyph_uk.dic to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/hyphenation
REFTEST INFO | copying tree /Users/jkew/mozdev/mozilla-central/obj-arm-linux-androideabi/_tests/reftest/chrome to /var/folders/jq/5dbj29lj10l_thg8t9kq73qc0000gq/T/tmpFY7Sq2.mozrunner/chrome

But when I push it to try, I can see that it fails:

[task 2017-03-08T22:57:33.135712Z] 22:57:33     INFO -  REFTEST INFO | copying tree /home/worker/workspace/build/tests/reftest/fonts to /tmp/tmp5cRo_7.mozrunner/fonts
[task 2017-03-08T22:57:33.148623Z] 22:57:33     INFO -  REFTEST WARNING | runreftest.py | Failed to copy /home/worker/workspace/build/tests/reftest/hyphenation to profile
[task 2017-03-08T22:57:33.148941Z] 22:57:33     INFO -  REFTEST INFO | copying tree /home/worker/workspace/build/tests/reftest/chrome to /tmp/tmp5cRo_7.mozrunner/chrome

Which looks to me like the hyphenation dictionaries didn't get added to the list in options.extraProfileFiles, so we just had an empty directory at /home/worker/workspace/build/tests/reftest/hyphenation. Something missing from how tests get packaged, maybe? I don't know my way around these harnesses, packaging, etc., but in my local case, I see that it's copying the dics directly from my source tree, whereas I think automation is working with a tests package of some kind, and they're missing from that.

Comment 5

[Jonathan Kew [:jfkthame]]

And indeed, if I download the target.reftest.tests.zip artifact from the try job, it includes the /fonts/ directory and the /chrome/ directory that we add to the profile; but it doesn't have a /hyphenation/ directory. So IIUC, we just need to add that to whatever command is responsible for packaging up this target.reftest.tests.zip thing.

Comment 6

[Geoff Brown [:gbrown]]

It's not immediately clear to me what the issue is here, but it reminds me of bug 1197716 (for fonts).

Comment 7

[Geoff Brown [:gbrown]]

You might need something like https://dxr.mozilla.org/mozilla-central/rev/58753259bfeb3b818eac7870871b0aae1f8de64a/testing/testsuite-targets.mk#232, for hyphenation (I'm a little hazy on this, but it's worth a try).

Comment 8

[Jonathan Kew [:jfkthame]]

Thanks; I found that earlier and am just experimenting with stuff there.... looks like this may fix things. :)

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: Ensure hyphenation dictionaries are included in the reftest artifact for packaged tests

This fixes the problem according to my testing, by ensuring the hyphenation dictionaries are included in the packaged reftest artifact. Try run with this patch plus bug 1285752: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b80aad6ebb2bb4fd35d5f4f842c9c072b61bc1e

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8845996 [details] [diff] [review]
Ensure hyphenation dictionaries are included in the reftest artifact for packaged tests

Review of attachment 8845996 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome. Thank you so much! I'll redirect this review to gbrown.

Comment 11

[Geoff Brown [:gbrown]]

Comment on attachment 8845996 [details] [diff] [review]
Ensure hyphenation dictionaries are included in the reftest artifact for packaged tests

Review of attachment 8845996 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/tools/reftest/runreftest.py
@@ +708,5 @@
>                  elif os.path.basename(abspath).endswith('.dic'):
> +                    hyphDir = os.path.join(profileDir, "hyphenation")
> +                    if not os.path.exists(hyphDir):
> +                        os.makedirs(hyphDir)
> +                    shutil.copy2(abspath, hyphDir)

Why is this change needed? I read it as "do not create a hyphenation directory unless a dictionary is present"...just an optimization? (I'm just curious.)

Comment 12

[Jonathan Kew [:jfkthame]]

(In reply to Geoff Brown [:gbrown] from comment #11)
> Why is this change needed? I read it as "do not create a hyphenation
> directory unless a dictionary is present"...just an optimization? (I'm just
> curious.)

Without that, I get failures like this on try:

[task 2017-03-10T10:32:05.308079Z] 10:32:05     INFO -  REFTEST INFO | copying tree /home/worker/workspace/build/tests/reftest/fonts to /tmp/tmpQVmgXz.mozrunner/fonts
[task 2017-03-10T10:32:05.323522Z] 10:32:05     INFO -  REFTEST INFO | copying tree /home/worker/workspace/build/tests/reftest/hyphenation to /tmp/tmpQVmgXz.mozrunner/hyphenation
[task 2017-03-10T10:32:05.441793Z] 10:32:05     INFO -  WARNING: Unable to retrieve log file (/storage/sdcard/tests/reftest/reftest.log) from remote device
[task 2017-03-10T10:32:05.812490Z] 10:32:05     INFO -  Automation Error: Exception caught while running tests
[task 2017-03-10T10:32:05.814669Z] 10:32:05     INFO -  Traceback (most recent call last):
[task 2017-03-10T10:32:05.814759Z] 10:32:05     INFO -    File "/home/worker/workspace/build/tests/reftest/remotereftest.py", line 407, in run_test_harness
[task 2017-03-10T10:32:05.814822Z] 10:32:05     INFO -      retVal = reftest.runTests(options.tests, options)
[task 2017-03-10T10:32:05.814898Z] 10:32:05     INFO -    File "/home/worker/workspace/build/tests/reftest/runreftest.py", line 436, in runTests
[task 2017-03-10T10:32:05.814963Z] 10:32:05     INFO -      return self.runSerialTests(manifests, options, cmdargs)
[task 2017-03-10T10:32:05.815195Z] 10:32:05     INFO -    File "/home/worker/workspace/build/tests/reftest/runreftest.py", line 673, in runSerialTests
[task 2017-03-10T10:32:05.815802Z] 10:32:05     INFO -      profile = self.createReftestProfile(options, manifests)
[task 2017-03-10T10:32:05.816139Z] 10:32:05     INFO -    File "/home/worker/workspace/build/tests/reftest/remotereftest.py", line 234, in createReftestProfile
[task 2017-03-10T10:32:05.816415Z] 10:32:05     INFO -      port=options.httpPort)
[task 2017-03-10T10:32:05.816740Z] 10:32:05     INFO -    File "/home/worker/workspace/build/tests/reftest/runreftest.py", line 352, in createReftestProfile
[task 2017-03-10T10:32:05.817040Z] 10:32:05     INFO -      self.copyExtraFilesToProfile(options, profile)
[task 2017-03-10T10:32:05.817389Z] 10:32:05     INFO -    File "/home/worker/workspace/build/tests/reftest/remotereftest.py", line 262, in copyExtraFilesToProfile
[task 2017-03-10T10:32:05.817764Z] 10:32:05     INFO -      RefTest.copyExtraFilesToProfile(self, options, profile)
[task 2017-03-10T10:32:05.818094Z] 10:32:05     INFO -    File "/home/worker/workspace/build/tests/reftest/runreftest.py", line 721, in copyExtraFilesToProfile
[task 2017-03-10T10:32:05.818416Z] 10:32:05     INFO -      shutil.copytree(abspath, dest)
[task 2017-03-10T10:32:05.818696Z] 10:32:05     INFO -    File "/usr/lib/python2.7/shutil.py", line 177, in copytree
[task 2017-03-10T10:32:05.819017Z] 10:32:05     INFO -      os.makedirs(dst)
[task 2017-03-10T10:32:05.819395Z] 10:32:05     INFO -    File "/home/worker/workspace/build/venv/lib/python2.7/os.py", line 157, in makedirs
[task 2017-03-10T10:32:05.819607Z] 10:32:05     INFO -      mkdir(name, mode)
[task 2017-03-10T10:32:05.819958Z] 10:32:05     INFO -  OSError: [Errno 17] File exists: '/tmp/tmpQVmgXz.mozrunner/hyphenation'

(This is from a job that had some extra logging there, see https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1cc17b7f97842cbb86f4d8d12bc8e352ecff11a.)

AFAICS, in the packaged-tests case, the hyphenation directory gets copied from the test package by a single shutil.copytree call; but that apparently fails if the target directory has already been created in the profile.

So this ensures we don't create it explicitly unless we're actually doing the per-file copying of the dictionaries (which is what happens when running locally from the tree, rather than using the packaged reftest artifact).

Comment 13

[Geoff Brown [:gbrown]]

Got it. Thanks.

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

Jonathan, is that ready to land? :sebastian says that it's the last blocker for hypenation dicts in DLC.

Comment 15

[Jonathan Kew [:jfkthame]]

Yes, I think we can land this; I'll push it to inbound.

(I'd be interested to know a bit more about the current state of the DLC service, particularly how we handle failures, re-trying, etc., but that's a separate issue; this is just about making our reftest setup work ok.)

Comment 16

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8df878e675f7d53e3fbfc37d0c74764de29ecdd
Bug 1335429 - Ensure hyphenation dictionaries are included in the reftest artifact for packaged tests. r=gbrown

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a8df878e675f