Bug 17634 - string-validate-input test in SunSpider uses window properties instead of variables
Summary: string-validate-input test in SunSpider uses window properties instead of var...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-02 00:41 PST by John Resig
Modified: 2011-05-18 21:15 PDT (History)
6 users (show)

See Also:


Attachments
string-validate-input.js patch (1.84 KB, patch)
2008-03-02 00:42 PST, John Resig
no flags Details | Formatted Diff | Diff
string-validate-input.js patch (1.40 KB, patch)
2008-03-02 00:43 PST, John Resig
darin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Resig 2008-03-02 00:41:10 PST
There's a couple minor bugs in the test which allows variables to leak into the global scope. Please see the attached patch.
Comment 1 John Resig 2008-03-02 00:42:08 PST
Created attachment 19481 [details]
string-validate-input.js patch
Comment 2 John Resig 2008-03-02 00:43:23 PST
Created attachment 19482 [details]
string-validate-input.js patch
Comment 3 Oliver Hunt 2008-03-02 01:10:03 PST
Thanks John,
Changes look sane to me, but i'm not sure what the story is with changes to sunspider -- maciej is probably best to look into things like this, but any changes to performance characteristics makes it harder to compare prior results.  Then again we already know of a couple of bug fixes which will have to be fixed at some point anyway :-/
Comment 4 John Resig 2008-03-02 01:17:20 PST
Thanks Oliver - that's perfectly understandable. I figure that when it comes time for the next big update a bunch of these fixes can be grouped together; I just wanted to make sure that they don't get forgotten.
Comment 5 Darin Adler 2008-03-02 18:30:02 PST
Comment on attachment 19482 [details]
string-validate-input.js patch

Not a bug.

This test intentionally testing cases where variables are not declared. This correctly reflects the sloppiness you'll see in lots of real world scripts.

Most JavaScript engines are faster if you declare the variables, but this test is testing the slower code path.
Comment 6 Darin Adler 2008-03-02 18:30:48 PST
Maciej, please correct me if I'm mistaken.
Comment 7 Maciej Stachowiak 2008-04-14 18:43:23 PDT
I don't think this test is intentionally testing variables that are not declared. Some of the tests are, but in this case it is not intentional.
Comment 8 Maciej Stachowiak 2010-02-28 14:25:24 PST
Since this doesn't affect correctness of the test output, I think we'll wait to address this in SunSpider 2.0.
Comment 9 David Kilzer (:ddkilzer) 2011-05-18 21:15:16 PDT
See also Bug 60937.