Last Comment Bug 746236 - Fix make -C firefox-debug jstestbrowser
: Fix make -C firefox-debug jstestbrowser
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: Terrence Cole [:terrence]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 753316
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-17 10:56 PDT by Terrence Cole [:terrence]
Modified: 2012-05-09 07:24 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v0: Run browser tests out of dist dir. (1.88 KB, patch)
2012-04-17 12:35 PDT, Terrence Cole [:terrence]
ted: review+
Details | Diff | Splinter Review

Description Terrence Cole [:terrence] 2012-04-17 10:56:30 PDT
This got broken by Bug 735549.
Comment 1 Terrence Cole [:terrence] 2012-04-17 12:35:26 PDT
Created attachment 615826 [details] [diff] [review]
v0: Run browser tests out of dist dir.

Now that we generate the jstests.list manifest files automatically at package-tests time, we cannot run the jsbrowsertests directly out of the srcdir.  Instead we need to stage-package the jstests and do the testing in the dist directory.
Comment 2 Ted Mielczarek [:ted.mielczarek] 2012-04-23 11:55:07 PDT
This is pretty unfortunate. :-( This patch will certainly work, but it means that running the tests will take a lot longer.

What was the reasoning behind getting rid of the in-tree manifests? We've been moving towards adding manifests, not removing them, in other test suites.
Comment 3 Terrence Cole [:terrence] 2012-04-23 13:47:23 PDT
(In reply to Ted Mielczarek [:ted] from comment #2)
> What was the reasoning behind getting rid of the in-tree manifests? We've
> been moving towards adding manifests, not removing them, in other test
> suites.

The situation is rather complex, so some history first:

In the long ago before time, there was a perl test runner.  All of the tests were in a single file, it was horrible to use, and the output was terrible.  So some time later around tracemonkey, someone added the trace-tests suite.  Tests were still in a single file, but the output was better.  A bit later, dmandelin wrote the first real JS test suite (the one in question here), which split up the perl tests into individual tests and added a test runner with useful progress-bar styled output, and manually maintained manifests.  Slightly later, around jaegermonkey, trace-tests got rewritten as jit-tests, stealing the nice output and split-up tests, but finding tests through the fs, rather than through manifests.  Since adding a tests to the jit-tests suite is trivial - unlike with jstests - people basically only add tests to jit-tests now, and have been since it was written.

Let me be perfectly clear: this is a far better workflow for JS than manually maintaining a manifest.  Unlike tests for the browser, js tests are only a single file and never access external resources.  Of the ~4000 tests in the jsreftest suite, only 374 use any annotation at all in the manifests, of which ~8 of those are non-trivial.  From the JS team's perspective, the manifest is needless make-work.

The down side of this situation is (1) jit-tests only get run in the shell and (2) they run at `make check` time and are thus not parallelized on tbpl.

> This is pretty unfortunate. :-( This patch will certainly work, but it means
> that running the tests will take a lot longer.
 
Given that the jsreftest suite takes 100's of seconds to run, I don't think the added overhead is all that significant.  The two makes are for trivial programs and run in ~1 second on my machine.  The stage-jstests can take a bit longer on a cold cache, but still only takes a handful of seconds on my slow laptop drive.  The other parts of the new test are identical.

More importantly, even though this workflow is now completely busted and has been for over a week, there has only been one bug filed on it.
Comment 4 Ted Mielczarek [:ted.mielczarek] 2012-04-24 06:04:07 PDT
A depressing indication that nobody is running this test suite at home! I suppose it's not totally damning, though, since JS devs are presumably running the same tests in the shell instead.
Comment 5 Ted Mielczarek [:ted.mielczarek] 2012-04-24 06:14:43 PDT
Comment on attachment 615826 [details] [diff] [review]
v0: Run browser tests out of dist dir.

Review of attachment 615826 [details] [diff] [review]:
-----------------------------------------------------------------

I'm still not a big fan, but it's better than having something that doesn't work.
Comment 6 Terrence Cole [:terrence] 2012-04-24 12:07:24 PDT
(In reply to Ted Mielczarek [:ted] from comment #4)
> A depressing indication that nobody is running this test suite at home! I
> suppose it's not totally damning, though, since JS devs are presumably
> running the same tests in the shell instead.

You are exactly right.  We generally debug and test on the shell (with its ~40 second build times) and rely on try runs to sus out integration errors.  Building a full browser only happens when we are doing API work and generally we aren't running the test suites to debug those.

One of the major goals of this work is to expand our browser test coverage by making the jit-tests run in the browser on TBPL.  I think the "run at home" version is good enough if it works at all, given that none of the people actively hacking on these tests use that target regularly.
Comment 7 Terrence Cole [:terrence] 2012-04-26 10:25:13 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/852320142dce
Comment 8 Ed Morley [:emorley] 2012-04-27 06:58:18 PDT
https://hg.mozilla.org/mozilla-central/rev/852320142dce

Note You need to log in before you can comment on or make changes to this bug.