Closed Bug 601245 Opened 10 years ago Closed 6 years ago

Stop packaging test files which are now in tests.jar

Categories

(Testing :: General, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED WONTFIX

People

(Reporter: sgautherie, Assigned: jmaher)

References

Details

Attachments

(1 file, 1 obsolete file)

Now, the unjarred files are bloat (ftp, ...) and confusing (not the place to look at nor hack).
Flags: in-testsuite-
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
Joel, can you look into this?
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.
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.
(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.
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.
Assignee: nobody → jmaher
Attachment #485596 - Flags: review?(ctalbert)
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 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).
Attachment #485596 - Flags: review?(ctalbert) → review+
updated accidental type in comment string.  Rerunning through try server as it has been a couple weeks.
Attachment #485596 - Attachment is obsolete: true
Attachment #489506 - Flags: review+
runs clean on try (2 runs)
http://hg.mozilla.org/mozilla-central/rev/315b3a3dda0f
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
(In reply to comment #11)

This patch was just a first step, was it not?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #489506 - Attachment description: fix twinopen to work with remote testing (1.1) → fix twinopen to work with remote testing (1.1) [Checked in: Comment 11]
Attachment #489506 - Attachment description: fix twinopen to work with remote testing (1.1) [Checked in: Comment 11] → fix up tests that depend on webserver (1.1) [Checked in: Comment 11]
this patch fixes 90% of the chrome tests and doesn't touch browser-chrome.
(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.
ok, tests.jar is no  longer built, marking this as wontfix.
Status: REOPENED → RESOLVED
Closed: 9 years ago6 years ago
Resolution: --- → WONTFIX
(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.
Status: RESOLVED → VERIFIED
Depends on: 939719
Component: New Frameworks → General
You need to log in before you can comment on or make changes to this bug.