390539 - honor --test-path option for browser chrome tests as well

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: add support for --test-path in browser chrome harness

The --test-path command line option to MochiTest's runtests.pl script currently only works for content and chrome tests, but it would be useful to use that option for browser chrome tests as well.

Here's a patch that adds support for it to the browser chrome harness.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 274835 [details] [diff] [review]
patch v1: add support for --test-path in browser chrome harness

>Index: testing/mochitest/browser-harness.xul

>+      if (gConfig.testPath)
>+        testsDir.appendRelativePath(gConfig.testPath);

>+      var requestPath = "chrome://mochikit/content/browser";
>+      if (gConfig.testPath)
>+        requestPath += "/" + gConfig.testPath;

To get this to work on windows, I had to use --test-path="browser\fuel\test", because appendRelativePath doesn't accept "/" as a path delimiter. This leads to a somewhat strange requestPath of "chrome://mochikit/content/browser/browser\fuel\test", which in turn leads to browserTestFiles with paths like "chrome://mochikit/content/browser/browser\fuel\test/browser_Browser.js". This doesn't cause any visible problems since it appears that loadSubscript (or whatever lower-level API it uses) treats any kind of slash equally, but it is a bit odd. I don't know if similar problems exist with Mochitest on Windows, but it might be a good idea to make a note of this on one of the MDC pages about writing tests.

>Index: testing/mochitest/runtests.pl.in

>   $log_path = $log_path || "";
>   $log_path =~ s/\\/\\\\/;
>+  $test_path = $test_path || "";
>+  $test_path =~ s/\\/\\\\/;

Looks like I forgot to add a "g" to that regexp-replace, so only the first slash is being replaced. Add it to both before checking in?

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: consistent slashes

> To get this to work on windows, I had to use --test-path="browser\fuel\test",
> because appendRelativePath doesn't accept "/" as a path delimiter. This leads
> to a somewhat strange requestPath of
> "chrome://mochikit/content/browser/browser\fuel\test", which in turn leads to
> browserTestFiles with paths like
> "chrome://mochikit/content/browser/browser\fuel\test/browser_Browser.js". 

Hmm, indeed.  I guess that's ok with documentation, but here's a better approach that lets you write browser/fuel/test on all three platforms.  It utilizes IO service methods to convert the forward slashes into backslashes on Windows.

Re-requesting review since this is a material change from the last patch.

> Looks like I forgot to add a "g" to that regexp-replace, so only the first
> slash is being replaced. Add it to both before checking in?

Yup, I have included that change in this patch.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Checking in testing/mochitest/browser-harness.xul;
/cvsroot/mozilla/testing/mochitest/browser-harness.xul,v  <--  browser-harness.xul
new revision: 1.4; previous revision: 1.3
done
Checking in testing/mochitest/runtests.pl.in;
/cvsroot/mozilla/testing/mochitest/runtests.pl.in,v  <--  runtests.pl.in
new revision: 1.31; previous revision: 1.30
done