601245 - Stop packaging test files which are now in tests.jar

Status: VERIFIED WONTFIX

(Testing :: General, defect)

Description

[Serge Gautherie (:sgautherie)]

Now, the unjarred files are bloat (ftp, ...) and confusing (not the place to look at nor hack).

Comment 1

[Serge Gautherie (:sgautherie)]

Hum, I think some of these tests are still accessing their unjarred resources:
we probably need to replace more 'mochi.test:8888' in
http://mxr.mozilla.org/comm-central/search?string=mochi.test%3A8888&case=1&find=%2Ftest.*%2Fbrowser_.*%5C.js
etc

Comment 2

[cmtalbert]

Joel, can you look into this?

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

thanks for bringing this up.  It was not as easy as I would have though to do this.  There are some hard coded tests (in chrome) which need updating, then we can adjust the makefile to remove extra files during the packaging step.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

So I have been looking into this and there are a few tests I have found so far that need http access (vs file:/// or chrome://):
 - toolkit/mozapps/downloads/tests/chrome/test_unknownContentType_dialog_layout.xul (requires the .sjs for the content type)
 - toolkit/mozapps/updates/* (relies heavily on .sjs)

So this brings up a question about whether we should remove everything or not?  One middle of the road solution is to fix all tests that rely on http://mochi.test:8888/... which don't need http for the test and then leave all the rest of the tests alone.  This could be done by moving stuff to a utility directory.

for a11y, we are ok to remove the directory and for browser-chrome there are a lot of tests which depend on mochi.test or other http:// paths.

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)
> fix all tests that rely on
> http://mochi.test:8888/... which don't need http for the test

Let's do that first, then we'll see.

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: fix up tests that depend on webserver

this patch fixes all tests in mochitest-chrome (a11y doesn't need any work) but not browser-chrome.  There is a small list of tests which are not fixed by this:
 * browser/components/feeds/tests/chrome/* (this is a chrome test - 1 test) 
 * layout/base/tests/chrome/test_bug551434.html
 * toolkit/components/places/tests/chrome/* (7 tests)
 * toolkit/mozapps/downloads/tests/chrome/test_unknownContentType_dialog_layout.xul (requires .sjs)
 * toolkit/mozapps/update/test/chrome/* (lots of tests here)

Next up is browser-chrome tests, and figuring out how we want to handle these tests.  I also need to ensure we are making a tests.jar for osx opt builds as I suspect it isn't doing that for some reason.

Comment 7

[Serge Gautherie (:sgautherie)]

Comment on attachment 485596 [details] [diff] [review]
fix up tests that depend on webserver

>diff --git a/docshell/test/chrome/bug293235_window.xul b/docshell/test/chrome/bug293235_window.xul
>@@ -107,18 +105,17 @@
>-
>-      // Load the page that the link on the previous page points to.
>+      // Load the page that the link on the previous page point to.

's' shouldn't be removed, should it?


>diff --git a/docshell/test/chrome/docshell_helpers.js b/docshell/test/chrome/docshell_helpers.js
>@@ -401,22 +403,46 @@ function enableBFCache(enable) {
>+  if (jar != null) {
>+    if (gExtractedPath == null) {
>+      var resolved = extractJarToTmp(jar);
>+      gExtractedPath = resolved.path;
>+    }
>+  } else {
>+    return null;
>+  }

Return early:
if (jar == null)
  return null;

I'm not familiar with extractJarToTmp(): Tmp is garanteed to be cleaned up after use, isn't it?

Comment 8

[cmtalbert]

Comment on attachment 485596 [details] [diff] [review]
fix up tests that depend on webserver

This all looks pretty straight-forward as the middle of the road approach.  It of course doesn't remove anything so it doesn't address teh core problem here, just makes sure that all the existing tests are referencing the right place.  Also, as Serge noted, don't remove the 's' on the comment.

Serge, yes, the extractJarToTmp(http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/chrome-harness.js#282) will extract to the current profile location, which if you're running using the default command line options will be a location that the harness will clean up after the test run completes.  (In fact, given the default options, the profile directory is created in the system temp directory).

r+ so far (with reverting the change to the comment).

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: fix up tests that depend on webserver (1.1) [Checked in: Comment 11]

updated accidental type in comment string.  Rerunning through try server as it has been a couple weeks.

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

runs clean on try (2 runs)

Comment 11

[cmtalbert]

http://hg.mozilla.org/mozilla-central/rev/315b3a3dda0f

Comment 12

[Serge Gautherie (:sgautherie)]

(In reply to comment #11)

This patch was just a first step, was it not?

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

this patch fixes 90% of the chrome tests and doesn't touch browser-chrome.

Comment 14

[Serge Gautherie (:sgautherie)]

(In reply to Joel Maher (:jmaher) from comment #3)
> we can adjust the makefile to remove extra files during the packaging step.

It would help to attach such a patch: Try server could then be used to find out if/what issues (= test failures) remain.

***

Ftr, currently, simply renaming tests.jar lets tests run from their unjarred files, which is very practical to develop/test from a packaged build.
Fixing this bug should retain this "feature": unzipping+renaming tests.jar should still let tests work.

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

ok, tests.jar is no  longer built, marking this as wontfix.

Comment 16

[Serge Gautherie (:sgautherie)]

(In reply to Joel Maher (:jmaher) from comment #15)
> tests.jar is no  longer built

Ftr, that change was done in "Bug 939719 - stop packaging tests.jar up during a build".

V.WontFix then.