Open Bug 1188021 Opened 4 years ago Updated 4 years ago

Enable our harnesses to be run in production with unicode characters in the profile path

Categories

(Testing :: General, defect)

defect
Not set

Tracking

(Not tracked)

People

(Reporter: jgriffin, Unassigned)

Details

Bug 1184333 discovered that EME was broken on machines in which Chinese characters occurred in the profile path.  We don't catch this in CI because we run only with profiles with ascii chars in the temp directory.

We could easily modify mozprofile to (optionally) generate profiles in a subdir that contains UTF-8 chars like Chinese, in order to catch these types of issues in CI.
Can marionette restart the browser with a new profile within a test? If so, this might be better off as a new test rather than a whole new harness configuration.
It is, but that wouldn't catch any similar problems that might currently be identified only with mochitests.  I don't know that we actually need a new harness configuration for this; is there any reason not to do this by default?
If the proposal here is to force non-ascii into the profile name, that seems very possible, although I wonder how far we should go. For example non-BMP characters seem likely to cause bugs so is it worth making sure we have some of them?
(In reply to Jonathan Griffin (:jgriffin) from comment #2)
> It is, but that wouldn't catch any similar problems that might currently be
> identified only with mochitests.  I don't know that we actually need a new
> harness configuration for this; is there any reason not to do this by
> default?

Doing it by default seems fine.. as long as we don't have M-unicode(1 2) :p
The purpose here is to prevent regressions, rather than to identify every possible character that causes problems, some of which are extremely unlikely.  So I think try runs should be our guide.
(In reply to Andrew Halberstadt [:ahal] from comment #4)
> (In reply to Jonathan Griffin (:jgriffin) from comment #2)
> > It is, but that wouldn't catch any similar problems that might currently be
> > identified only with mochitests.  I don't know that we actually need a new
> > harness configuration for this; is there any reason not to do this by
> > default?
> 
> Doing it by default seems fine.. as long as we don't have M-unicode(1 2) :p

Agreed, I think we explicitly want to avoid mirroring existing runs in a new configuration.
Naive try run just to see what effect injecting unicode chars into the profile path has on our current automation:  https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb8e775085e7

This won't catch things that don't use mozprofile, but on desktop at least that set is pretty limited.
We've hit stuff like this before (bug 396052). I filed a bug for a similar idea back then, bug 396059, but we never went anywhere with it. That was for the actual Firefox install path, not the profile path, but both seem useful.

I think putting non-ASCII in the profile path for all our test suites is not unreasonable and likely to help catch various broken things. Per jgraham's comment 3, I would recommend using a non-BMP character (pick your favorite emoji, I'm fond of 
Apparently bugzilla doesn't like emoji? That was U+1F4A9, and the rest of my comment was that non-BMP characters are likely to cause the most problems, and if they work then the code is probably fully unicode safe.
(In reply to Jonathan Griffin (:jgriffin) from comment #7)
> Naive try run just to see what effect injecting unicode chars into the
> profile path has on our current automation: 
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb8e775085e7
> 
> This won't catch things that don't use mozprofile, but on desktop at least
> that set is pretty limited.

So, the results here are pretty ugly.  There are lots of things going wrong in our automation with a unicode profile path; we don't get to the point of launching Firefox, generally.

There are some problems with certutil, and another problem handling a unicode _PROFILE_PATH in server.js: https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/server.js#172
wpt appears to be relatively easy to fix. It worked, at least somewhat, after ensuring that commands were converted to UTF8 strings before being logged in a couple of places; one inside wptrunner and mozrunner.GeckoRuntimeRunner.command. The latter looks somewhat wasteful since it builds up the whole command on every call…
You need to log in before you can comment on or make changes to this bug.