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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dmandelin, Unassigned)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

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.
Summary: TM: Add comparison-based closure test suite → TM: Add dmandelin's ad hoc closure tests to trace-test suite
Attached patch PatchSplinter Review
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 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+
(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
One of the tests fails on 64-bit.  See bug 538314.
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.

Attachment

General

Created:
Updated:
Size: