Closed Bug 1452913 Opened 2 years ago Closed 2 years ago
Missing error handling for reading test list in reftest
.jsm can cause infinite hangs
59 bytes, text/x-review-board-request
I noticed a permanent hang of the reftest harness when running the tests by using a profile with a Unicode character in its path. Further investigation has been shown that the problem here is mainly because there is no error handling code for the following call to `OS.File.read()`: https://dxr.mozilla.org/mozilla-central/rev/83de58ddda2057f1cb949537f6b111e3b115ea3d/layout/tools/reftest/reftest.jsm#367-371 Whenever that fails `StartTests()` is not being called, and the extension performs no single test which then causes the browser to not output anything. As result the job times out after 370s without receiving any output. See bug 1441580. What we have to do here is to handle a possible exception, and early exit.
The exact failure is something like: > REFTEST ERROR | ***** failed to load test: Unix error 2 during operation open on file /var/folders/4k/sf4gz5fn3kl9hr3nd7pzbvhc0000gn/T/tmpP_Ga7H.mozrunnerðª/reftests.json (No such file or directory) I feel there is a problem with `OS.File` which is kinda broken when Unicode characters are in the path. But that should actually be a different bug. A patch to prevent the hang should be up soon.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
The problem is not with `Os.File` but with `prefs.getCharPref("reftest.tests", null)` which doesn't have Unicode support. To fix that the reftest harness has to use `prefs.getStringPref()`. I will file a separate bug for this change.
Comment on attachment 8966552 [details] Bug 1452913 - Prevent hang of reftests when reading test objects list fails. https://reviewboard.mozilla.org/r/235272/#review240998
Attachment #8966552 - Flags: review?(gbrown) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/f079b11f50b7 Prevent hang of reftests when reading test objects list fails. r=gbrown
This patch makes reftests more stable. It would be good to see this test-only patch uplifted to beta.
You need to log in before you can comment on or make changes to this bug.