368585 - JavaScript Tests - initial move of tests depending upon SpiderMonkey extensions into js/tests/*/extensions/

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Bob Clary [:bc] (inactive)]

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

[Jeff Walden [:Waldo]]

(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.

Comment 2

[Bob Clary [:bc] (inactive)]

(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

[Brendan Eich [:brendan]]

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

Comment 4

[Bob Clary [:bc] (inactive)]

mozilla/js/tests/extensions it is unless someone votes no today.

Comment 5

[Bob Clary [:bc] (inactive)]

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

[Brendan Eich [:brendan]]

Oh, I had assumed we were gonna have ext or extensions subdirs of all the top-level tests subdirs -- so full speed ahead ;-).

/be

Comment 7

[Bob Clary [:bc] (inactive)]

preliminary candidates for moving to an extensions subsuite include uses of 

__proto__, 
__parent__,
__iterator__,
Script(), 
watch(), 
__defineSetter__(), 
__defineGetter__(), 
__lookupGetter__(), 
__lookupSetter__()

others?

Comment 8

[Jeff Walden [:Waldo]]

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

[Brendan Eich [:brendan]]

(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

[Jeff Walden [:Waldo]]

(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.

Comment 11

[Bob Clary [:bc] (inactive)]

Attachment: current list of tests to be "moved"

(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.

Comment 12

[Jeff Walden [:Waldo]]

(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).

Comment 13

[Bob Clary [:bc] (inactive)]

Attachment: list of tests to be moved v2

Thanks. I caught a couple of more as well.

Comment 14

[Bob Clary [:bc] (inactive)]

Attachment: existing /extensions/ tests

this is the list of new tests which I have placed in /extensions/ subdirectories.

Comment 15

[Bob Clary [:bc] (inactive)]

Attachment: list of tests to be moved v3

this is the current list of existing tests which I plan to move to /extensions/ subdirectories.

Comment 16

[Bob Clary [:bc] (inactive)]

Attachment: shell script to cvs move the tests

brendan, if the v3 list meets with your approval, I will use this script to perform the cvs removes|adds for the tests.

Comment 17

[Brendan Eich [:brendan]]

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

Comment 18

[Brendan Eich [:brendan]]

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

Comment 19

[Bob Clary [:bc] (inactive)]

(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.

Comment 20

[Bob Clary [:bc] (inactive)]

Attachment: gzipped cvs log of initial complete moves

also includes updates to *.tests lists. 

I still have ~100 "partial" moves pending.

Comment 21

[Bob Clary [:bc] (inactive)]

Attachment: gzipped cvs log of initial partial moves

turns out the list was much smaller thank I originally thought.

Comment 22

[Bob Clary [:bc] (inactive)]

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

Comment 23

[Bob Clary [:bc] (inactive)]

ditto
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-313938.js,v  <--  regress-313938.js
new revision: 1.2; previous revision: 1.1

Comment 24

[Bob Clary [:bc] (inactive)]

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