mozilla-mobile / firefox-tv

Firefox for Amazon's Fire TV
https://blog.mozilla.org/blog/2017/12/20/firefox-is-now-on-amazon-fire-tv-happy-holiday-watching/
Mozilla Public License 2.0
255 stars 109 forks source link

Support UTF-8+ in TC docker images' unit tests #594

Closed mcomella closed 6 years ago

mcomella commented 6 years ago

In #582, I added some tests which run on robolectric and compare UTF-8 strings (TestPublicSuffix). These passed locally and failed on TC: https://tools.taskcluster.net/groups/aqFyQHWoR3yHMi3736WvDg/tasks/aqFyQHWoR3yHMi3736WvDg/details

I was able to reproduce by downloading the docker container. I was unable to reproduce in the image when I set LC_ALL: docker run -e LC_ALL=C.UTF-8 -it mozillamobile/firefox-tv /bin/bash

iiuc, the docker image is forcing the test JVM to run in a non-UTF-8 compatible mode and it causes the test to fail. We should fix that. I don't know if LC_ALL is the correct answer (it seems like it overrides everything).

fwiw, to set env var in TC/docker, it looks like "there's an env property to docker-worker payloads that lets you set environment variables a peer property to command" (via irc)

Due to time constraints, I disabled the failing tests in #582: please re-enable them (uncomment the lines I commented out).

fwiw, here is the docker command to pull down the tests:

git fetch https://github.com/mcomella/firefox-tv.git i511-titles && git config advice.detachedHead false && git checkout 3c78731d5422ed0b33e32b8c4de4fc9083fd907e && echo \"--\" > .adjust_token && ./gradlew clean assemble lint checkstyle ktlint pmd test
mcomella commented 6 years ago

Inside the docker image (without the env var specified), this also appears to pass the tests:

LANG=en_US.UTF-8 ./gradlew testAmazonWebviewDebugUnitTest --tests "*TestPublicSuffix"

Note that you'll have uncomment the tests for these commands to start failing.

mcomella commented 6 years ago

Looks like we localgen UTF-8 in the docker image: https://github.com/mozilla-mobile/firefox-tv/blob/b17738d3722244fd654460207907704d028e7980/tools/docker/Dockerfile#L31

I wonder:

@pocmo Do you have any ideas?

mcomella commented 6 years ago

Setting -e LANG=en_US.UTF-8 also seems to work (which is how macOS appears to be configured).

mcomella commented 6 years ago

Thinking: it'd be ideal to set the LANG in the image so the build environment is reproducible as to when someone pulls down the image. Failing that, we should make it part of the command that TC runs when it runs the unit tests. I think we should avoid setting the env var in the task cluster config file otherwise (since it won't be reproducible).

mcomella commented 6 years ago

Due to time constraints, I disabled the failing tests in #582: please re-enable them (uncomment the lines I commented out).

I lied: I just decided to run the gradle test command with LANG=en_US.UTF-8 instead: https://github.com/mozilla-mobile/firefox-tv/pull/582/commits/0f4a1660d4657ae670745a296ecbf422dd16e94f

@pocmo Can you think of a better solution or should we call this fixed and roll it out to our other TC commands and project repos?

pocmo commented 6 years ago

Looks like we localgen UTF-8 in the docker image:

Yes, we needed to do that because otherwise fastlane would fail. In retrospect this might be the same problem you see here. I remember that even though we generated the locale it wasn't available in the Docker image. However it worked with fastlane because we were running it inside a shell script (take-screenshots.sh).

Anyhow. If settings LANG is all that is needed to fix this then let's do this in the Docker image directly, like we set ANDROID_SDK_HOME etc.: https://github.com/mcomella/firefox-tv/blob/master/tools/docker/Dockerfile#L39

mcomella commented 6 years ago

To update the docker image, we can use this guide Sebastian wrote: https://github.com/mozilla-mobile/focus-android/wiki/Automation:-Docker-guide#updating-the-docker-image :)

mcomella commented 6 years ago

[triage] I fixed this with a hack, this bug is to fix it properly. If our team is small, we'll probably ask each other if we run into issues setting up new tests so this isn't urgent.