There is quite a lot of hokeyness in the JS test suite, actually, but a lot of it we can live with. In poking around the past few days, I've found a few things odd enough that I think we should fix them. * ecma/Date/220.127.116.11.js is almost completely bogus. None of that code is doing what it claims to do. * Several tests have a line that says something like: new TestCase( "SECTION", "with MyObject, eval should return square of " ); which is nonsense. ecma/ExecutionContexts/10.1.4-1.js ecma/ExecutionContexts/10.1.4-10.js ecma/ExecutionContexts/10.1.4-2.js ecma/ExecutionContexts/10.1.4-3.js ecma/ExecutionContexts/10.1.4-4.js ecma/ExecutionContexts/10.1.4-5.js ecma/ExecutionContexts/10.1.4-6.js ecma/ExecutionContexts/10.1.4-7.js ecma/ExecutionContexts/10.1.4-8.js ecma/ExecutionContexts/10.1.5-1.js ecma/ExecutionContexts/10.1.5-2.js ecma/ExecutionContexts/10.1.5-3.js ecma/ExecutionContexts/10.1.5-4.js ecma/extensions/10.1.4-9.js * ecma_3/Function/regress-97921.js is asserting that for example innerValue === outerValue is equal to true, when it should be asserting that innerValue and outerValue are equal. regress-85880.js is very similar. * Several tests are named regress-######.js but contain a `var BUGNUMBER = ######;` with a different bug number. This is not necessarily a mistake but I think it probably is in most cases. e4x/Regress/regress-328249.js e4x/decompilation/regress-350531.js js1_5/Array/regress-311515.js js1_5/Regress/regress-306633.js js1_5/Regress/regress-308566.js js1_5/Regress/regress-310993.js js1_5/Regress/regress-407957.js js1_5/Regress/regress-425360.js js1_5/String/regress-157334-01.js js1_5/decompilation/regress-351693.js js1_5/decompilation/regress-353000.js js1_5/extensions/regress-375183.js js1_5/extensions/regress-420869-01.js js1_6/extensions/regress-465443.js js1_7/block/regress-349298.js js1_7/block/regress-351606.js js1_7/decompilation/regress-348904.js js1_7/iterable/regress-355090.js js1_7/regress/regress-355832-01.js js1_7/regress/regress-355832-02.js js1_7/regress/regress-407957.js js1_8/regress/regress-459185.js * ecma_3/RegExp/regress-224676.js has crazy indentation.
(In reply to comment #0) > There is quite a lot of hokeyness in the JS test suite, actually, but a lot of > it we can live with. In poking around the past few days, I've found a few > things odd enough that I think we should fix them. > > * ecma/Date/18.104.22.168.js is almost completely bogus. None of that code > is doing what it claims to do. > I can just add it to the exclusion lists if it is totally bogus. > * Several tests have a line that says something like: > new TestCase( "SECTION", "with MyObject, eval should return square of " ); > which is nonsense. > > ecma/ExecutionContexts/10.1.4-1.js > ecma/ExecutionContexts/10.1.4-10.js > ecma/ExecutionContexts/10.1.4-2.js > ecma/ExecutionContexts/10.1.4-3.js > ecma/ExecutionContexts/10.1.4-4.js > ecma/ExecutionContexts/10.1.4-5.js > ecma/ExecutionContexts/10.1.4-6.js > ecma/ExecutionContexts/10.1.4-7.js > ecma/ExecutionContexts/10.1.4-8.js > ecma/ExecutionContexts/10.1.5-1.js > ecma/ExecutionContexts/10.1.5-2.js > ecma/ExecutionContexts/10.1.5-3.js > ecma/ExecutionContexts/10.1.5-4.js > ecma/extensions/10.1.4-9.js > Not really. I don't have time to look really closely but the new TestCase is creating a TestCase object and putting it in the gTestcases array and then the rest of the test horks with it. The style is screwed but the tests as written require it. > * ecma_3/Function/regress-97921.js is asserting that for example > innerValue === outerValue is equal to true, when it should be asserting > that innerValue and outerValue are equal. > > regress-85880.js is very similar. I'm a little leery of reportCompare here. The tests work as is, right? > > * Several tests are named regress-######.js but contain a > `var BUGNUMBER = ######;` with a different bug number. This is not > necessarily a mistake but I think it probably is in most cases. > > e4x/Regress/regress-328249.js > e4x/decompilation/regress-350531.js > js1_5/Array/regress-311515.js > js1_5/Regress/regress-306633.js > js1_5/Regress/regress-308566.js > js1_5/Regress/regress-310993.js > js1_5/Regress/regress-407957.js > js1_5/Regress/regress-425360.js > js1_5/String/regress-157334-01.js > js1_5/decompilation/regress-351693.js > js1_5/decompilation/regress-353000.js > js1_5/extensions/regress-375183.js > js1_5/extensions/regress-420869-01.js > js1_6/extensions/regress-465443.js > js1_7/block/regress-349298.js > js1_7/block/regress-351606.js > js1_7/decompilation/regress-348904.js > js1_7/iterable/regress-355090.js > js1_7/regress/regress-355832-01.js > js1_7/regress/regress-355832-02.js > js1_7/regress/regress-407957.js > js1_8/regress/regress-459185.js K. > > * ecma_3/RegExp/regress-224676.js has crazy indentation. Looks like autoindent was thrown off by the regexp chars.
Created attachment 363132 [details] [diff] [review] patch v1 addresses the bugnumber and indentation issues. If this is ok, I'll get it in with my current patch queue. The other aspects, I'll deal with later.
Attachment #363132 - Flags: review?(jorendorff)
You need to log in before you can comment on or make changes to this bug.