Closed Bug 1335429 Opened 3 years ago Closed 2 years ago

auto-hyphenation reftest fails if MOZ_EXCLUDE_HYPHENATION_DICTIONARIES is set

Categories

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

All
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 56
Tracking Status
firefox56 --- fixed

People

(Reporter: sebastian, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Actually the code for that landed (in bug 1280757):
https://hg.mozilla.org/mozilla-central/rev/dc96e5ff02fd

I wonder why this is failing now.
Priority: -- → P2
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...
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.
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.
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.
It's not immediately clear to me what the issue is here, but it reminds me of bug 1197716 (for fonts).
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).
Thanks; I found that earlier and am just experimenting with stuff there.... looks like this may fix things. :)
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
Attachment #8845996 - Flags: review?(s.kaspari)
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.
Attachment #8845996 - Flags: review?(s.kaspari) → review?(gbrown)
Assignee: nobody → jfkthame
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.)
Attachment #8845996 - Flags: review?(gbrown) → review+
(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).
Got it. Thanks.
Jonathan, is that ready to land? :sebastian says that it's the last blocker for hypenation dicts in DLC.
Flags: needinfo?(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.)
Flags: needinfo?(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
https://hg.mozilla.org/mozilla-central/rev/a8df878e675f
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.