Closed
Bug 368585
Opened 17 years ago
Closed 17 years ago
JavaScript Tests - initial move of tests depending upon SpiderMonkey extensions into js/tests/*/extensions/
Categories
(Core :: JavaScript Engine, enhancement)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: bc, Assigned: bc)
References
Details
Attachments
(5 files, 2 obsolete files)
Per request from Brendan on a way for other JS implementations to run the suite without running into SM extensions, I will add a new directory js/tests/ext for SpiderMonkey extensions. The first target will be tests which use __proto__. As for how to organize the subsuites, I think just creating a directory based on the extension feature name would be sufficient, e.g. js/tests/ext/__proto__. Unless we really care about the test history being carried over into the new locations, I will just cvs remove the tests from the old location and cvs add them to the new. I'll list the tests here before moving them over. Brendan, ok with you?
Comment 1•17 years ago
|
||
(In reply to comment #0) > I will add a new directory js/tests/ext for SpiderMonkey extensions. Can you name it something slightly more understandable than "ext", like maybe "extensions", "extend", or even "spidermonkey"? Without context, "ext" could mean a lot of things. > Unless we really care about the test history being carried over into the new > locations, I will just cvs remove the tests from the old location and cvs add > them to the new. I'll list the tests here before moving them over. How often are tests actually meaningfully (Gerv doesn't count) changed after the initial commit? You can emulate CVS copies from the client side relatively easily <http://benjamin.smedbergs.us/blog/2006-07-12/cvs-copies-done-by-anyone/>, so maybe it's worthwhile doing that even if meaningful changes are rare. Alternately, putting the original location in the commit message would be sufficient, although you'd probably have to script the commits.
Assignee | ||
Comment 2•17 years ago
|
||
(In reply to comment #1) > Can you name it something slightly more understandable than "ext", like maybe > "extensions", "extend", or even "spidermonkey"? Without context, "ext" could > mean a lot of things. ext was Brendan's sugggestion. I'll let him decide. > How often are tests actually meaningfully (Gerv doesn't count) changed after > the initial commit? You can emulate CVS copies from the client side relatively I do it occasionally but since I stopped adding tests until they the bug is fixed, much less so. > easily > <http://benjamin.smedbergs.us/blog/2006-07-12/cvs-copies-done-by-anyone/>, so > maybe it's worthwhile doing that even if meaningful changes are rare. > Alternately, putting the original location in the commit message would be > sufficient, although you'd probably have to script the commits. Considering his warning and the relative lack of usefulness I would say that cvs remove|add with a comment would be sufficient.
Comment 3•17 years ago
|
||
I don't see a need to preserve cvs histories if you guys don't. On ext vs. longer names: bclary points out how extension(s) is confusing in light of Firefox and Mozilla extensions, but perhaps it's ok in context. We should not call the subdirectory of tests spidermonkey, since some to many of these extensions are also in Rhino. But perhaps we could duplicate the tests for features in common? I fear that any attempt to unify with AS3 tests (say AS4 == ES4 == JS2 and Tamarin and SpiderMonkey are unified) will just want these all under one common subdir. So I'm still in favor of ext or extensions. /be
Assignee | ||
Comment 4•17 years ago
|
||
mozilla/js/tests/extensions it is unless someone votes no today.
Assignee | ||
Comment 5•17 years ago
|
||
hold your horses... I just realized while in the process of preparing the moves that I need to preserve the top level directory name since it is used for enabling e4x by default or for setting the version, e.g. js1_7. So, the best approach with what we have now is to add a subdirectory |extensions| to each suite and move the appropriate tests there. I can make an exclusion list for people to easily exclude the extensions from consideration. Ok?
Comment 6•17 years ago
|
||
Oh, I had assumed we were gonna have ext or extensions subdirs of all the top-level tests subdirs -- so full speed ahead ;-). /be
Assignee | ||
Comment 7•17 years ago
|
||
preliminary candidates for moving to an extensions subsuite include uses of __proto__, __parent__, __iterator__, Script(), watch(), __defineSetter__(), __defineGetter__(), __lookupGetter__(), __lookupSetter__() others?
Summary: JavaScript Tests - move tests SpiderMonkey extensions into js/tests/ext → JavaScript Tests - move tests SpiderMonkey extensions into js/tests/*/extensions/
Comment 8•17 years ago
|
||
Sharp variables (or just remove them completely? ;-) ), get/set foo(), "x" getter (or whatever the syntax is), Mozilla-specific (but less so every day) Array generics and Array.prototype.*, __noSuchMethod__, catch (e if condition), toSource(), uneval() (I think) Also, practically all the decompilation tests are "extensions" in that the spec doesn't require what they assert.
Comment 9•17 years ago
|
||
(In reply to comment #8) > Sharp variables (or just remove them completely? ;-) ), And do what with uneval on a cyclic or multiple-connected graph? :-P Jeff raises a good point about some of these tests, _mutatis mutandis_, being js2 fodder. We can cross that bridge when we come to it, but worth bearing in mind. /be
Comment 10•17 years ago
|
||
(In reply to comment #9) > And do what with uneval on a cyclic or multiple-connected graph? :-P let expression to not overwrite external-scope variables, with the expression being parenthesized and evaluating to the object, using the comma operator to build up the object as needed? :-P Probably hard to do, but possible in principle (assuming let is taken as a given for JS2, at least). It's probably not possible without let unless you were to introduce and evaluate an anonymous closure.
Assignee | ||
Comment 11•17 years ago
|
||
(In reply to comment #8) > Sharp variables (or just remove them completely? ;-) ), not easy to find via grep... I'll leave these to later. > get/set foo(), None? maybe we need some of these. > "x" getter (or whatever the syntax is), k. > Mozilla-specific (but less so every day) Array generics and Array.prototype.*, handled by segregation into js1_6, js1_7 if they care I think. > __noSuchMethod__, > catch (e if condition), > toSource(), > uneval() (I think) doh. > > Also, practically all the decompilation tests are "extensions" in that the spec > doesn't require what they assert. > true, but I'll leave them for the second round since we are getting up there in terms of numbers.
Assignee | ||
Updated•17 years ago
|
Attachment #253770 -
Attachment mime type: application/data → text/plain
Comment 12•17 years ago
|
||
(In reply to comment #11) > > get/set foo(), > > None? maybe we need some of these. At the least there's the test that prompted me to mention it -- <js/tests/js1_7/lexical/destructuring-order.js>. I suspect tests for get/set in object literals are in js1_5, since I think that's when it was added (could be wrong).
Assignee | ||
Comment 13•17 years ago
|
||
Thanks. I caught a couple of more as well.
Attachment #253770 -
Attachment is obsolete: true
Assignee | ||
Comment 14•17 years ago
|
||
this is the list of new tests which I have placed in /extensions/ subdirectories.
Assignee | ||
Comment 15•17 years ago
|
||
this is the current list of existing tests which I plan to move to /extensions/ subdirectories.
Attachment #253820 -
Attachment is obsolete: true
Attachment #254510 -
Flags: review?(brendan)
Assignee | ||
Comment 16•17 years ago
|
||
brendan, if the v3 list meets with your approval, I will use this script to perform the cvs removes|adds for the tests.
Attachment #254511 -
Flags: review?(brendan)
Assignee | ||
Updated•17 years ago
|
Attachment #254510 -
Attachment mime type: application/data → text/plain
Comment 17•17 years ago
|
||
Comment on attachment 254510 [details]
list of tests to be moved v3
I trust you to get this more or less right and fix any followup bugs, so rs=me here and next attachment ;-).
/be
Attachment #254510 -
Flags: review?(brendan) → review+
Comment 18•17 years ago
|
||
Comment on attachment 254511 [details]
shell script to cvs move the tests
This was generated, right? Be easier to review the generating script, but rs=me.
/be
Attachment #254511 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 19•17 years ago
|
||
(In reply to comment #17) > I trust you to get this more or less right and fix any followup bugs, so rs=me > here and next attachment ;-). (In reply to comment #18) > This was generated, right? Be easier to review the generating script, but > rs=me. I will attach such scripts for review in the future. I've taken the liberty of modifying the list a bit to only do complete moves for the tests which totally rely on an extension feature. I will duplicate the others, removing the parts which rely on an extension feature from the existing test while adding a corresponding new test which contains only the extension feature.
Status: NEW → ASSIGNED
Assignee | ||
Comment 20•17 years ago
|
||
also includes updates to *.tests lists. I still have ~100 "partial" moves pending.
Assignee | ||
Comment 21•17 years ago
|
||
turns out the list was much smaller thank I originally thought.
Assignee | ||
Updated•17 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Summary: JavaScript Tests - move tests SpiderMonkey extensions into js/tests/*/extensions/ → JavaScript Tests - initial move of tests depending upon SpiderMonkey extensions into js/tests/*/extensions/
Assignee | ||
Updated•17 years ago
|
Flags: in-testsuite-
Assignee | ||
Comment 22•17 years ago
|
||
Fix the Script object detection to prevent false failures on trunk where Script is dead. /cvsroot/mozilla/js/tests/js1_5/extensions/regress-220584.js,v <-- regress-220584.js new revision: 1.2; previous revision: 1.1
Assignee | ||
Comment 23•17 years ago
|
||
ditto /cvsroot/mozilla/js/tests/js1_5/extensions/regress-313938.js,v <-- regress-313938.js new revision: 1.2; previous revision: 1.1
Assignee | ||
Comment 24•17 years ago
|
||
Fix test to check new path. Checking in regress-50447-1.js; /cvsroot/mozilla/js/tests/js1_5/extensions/regress-50447-1.js,v <-- regress-50447-1.js new revision: 1.2; previous revision: 1.1
You need to log in
before you can comment on or make changes to this bug.
Description
•