Closed
Bug 538314
Opened 14 years ago
Closed 13 years ago
TM: Add dmandelin's ad hoc closure tests to trace-test suite
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: dmandelin, Unassigned)
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file)
106.40 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
I have a personal test suite of about 120 tests for use of closures and the |arguments| keyword in JS. This should be added to trace-tests. These tests use a different setup: they are simply JS programs that call 'print'. The harness runs them with and without -j and compares the output. I talked to dvander, and we thought the easiest and best way to get these tests in would be to replace the calls to |print| with calls to a different function that concatenates the 'output' to a global string. We would then |assertEq| at the end with a reference output. The advantages are that this requires only one run of the shell, can't be fooled by an interp bug (where the output is the same with and without -j), requires minimal changes to the harness, and the tests can be updated with a script. The disadvantage is that it's slightly more work to create a test case, because you need to get the reference output.
Reporter | ||
Updated•14 years ago
|
Summary: TM: Add comparison-based closure test suite → TM: Add dmandelin's ad hoc closure tests to trace-test suite
Reporter | ||
Comment 1•14 years ago
|
||
I pretty much followed the plan I gave in comment 0. I did find out that a few tests throw JS exceptions, which was intended (they are testing that we don't crash in certain situations that do throw an exception). I added a metadata directive to support that.
Attachment #420604 -
Flags: review?(jorendorff)
Comment 2•14 years ago
|
||
Comment on attachment 420604 [details] [diff] [review] Patch So, more tests are good. >+ name ::= "TMFLAGS" | "expect-error" Just "error" I guess. >+def check_output(out, err, rc, allow_oom, error): It would be nice to rename these params to errorMsg and expectedError or something.
Attachment #420604 -
Flags: review?(jorendorff) → review+
Reporter | ||
Comment 3•14 years ago
|
||
(In reply to comment #2) > >+def check_output(out, err, rc, allow_oom, error): > > It would be nice to rename these params to errorMsg and expectedError or > something. I made the latter change, but skipped the former because 'out' and 'err' mean the text of stdout and stderr by convention (in this program). http://hg.mozilla.org/tracemonkey/rev/1ad8a92636b2
Whiteboard: fixed-in-tracemonkey
Comment 4•14 years ago
|
||
One of the tests fails on 64-bit. See bug 538314.
Comment 6•13 years ago
|
||
Resolving as fixed -- reopen and remove fixed-in-tracemonkey if that's not the case.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•