Convert argument toObject() in Object.getOwnPropertyNames/Descriptors

Convert argument toObject() in Object.getOwnPropertyNames/Descriptors

BUG=v8:3443
LOG=Y
R=arv@chromium.org, dslomov@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=24638

[caitp (gmail), 2014-09-30 15:53:37]

2nd half of fix for https://code.google.com/p/v8/issues/detail?id=3443

PTAL

[arv (Not doing code reviews), 2014-09-30 16:02:21]

LGTM

https://codereview.chromium.org/613283002/diff/1/test/mjsunit/es6/symbols.js
File test/mjsunit/es6/symbols.js (right):

https://codereview.chromium.org/613283002/diff/1/test/mjsunit/es6/symbols.js#...
test/mjsunit/es6/symbols.js:505: // Object.getOwnPropertySymbols() does not
throw when passed primitive values.
Wrap this in a named function

function TestGetOwnPropertySymbolsOnPrimitives() {

then the comment can go away as well.

[caitp (gmail), 2014-10-09 14:51:35]

https://codereview.chromium.org/613283002/diff/1/test/mjsunit/es6/symbols.js
File test/mjsunit/es6/symbols.js (right):

https://codereview.chromium.org/613283002/diff/1/test/mjsunit/es6/symbols.js#...
test/mjsunit/es6/symbols.js:505: // Object.getOwnPropertySymbols() does not
throw when passed primitive values.
On 2014/09/30 16:02:20, arv wrote:
> Wrap this in a named function
> 
> function TestGetOwnPropertySymbolsOnPrimitives() {
> 
> then the comment can go away as well.

done

[caitp (gmail), 2014-10-09 14:53:32]

Patchset #2 (id:20001) has been deleted

[caitp (gmail), 2014-10-09 14:54:31]

Patchset #1 (id:1) has been deleted

[arv (Not doing code reviews), 2014-10-14 15:29:40]

https://codereview.chromium.org/613283002/diff/40001/test/test262/test262.status
File test/test262/test262.status (right):

https://codereview.chromium.org/613283002/diff/40001/test/test262/test262.sta...
test/test262/test262.status:97: '15.2.3.4-1-4': [SKIP],
Based on the comments in other reviews this can be PASS FAIL_OK. That way the
tests are run at least.

[caitp (gmail), 2014-10-14 17:27:20]

On 2014/10/14 15:29:40, arv wrote:
>
https://codereview.chromium.org/613283002/diff/40001/test/test262/test262.status
> File test/test262/test262.status (right):
> 
>
https://codereview.chromium.org/613283002/diff/40001/test/test262/test262.sta...
> test/test262/test262.status:97: '15.2.3.4-1-4': [SKIP],
> Based on the comments in other reviews this can be PASS FAIL_OK. That way the
> tests are run at least.

r24555 changed the [SKIP] to [FAIL_OK] rather than [PASS, FAIL_OK] --- I'm
guessing this is the right way to mark the tests as known failures.

I admittedly don't fully understand what having a PASS outcome adds to this ---
does that mean that it doesn't matter if the test starts passing some day?
Glancing at statusfile.py and testsuite.py isn't really answering this for me.

Anyways, I'll change this to [FAIL_OK] for now since that's what's in the tree
already.

[caitp (gmail), 2014-10-14 17:30:16]

Patchset #1 (id:40001) has been deleted

[arv (Not doing code reviews), 2014-10-14 19:58:01]

On 2014/10/14 17:27:20, caitp wrote:
> On 2014/10/14 15:29:40, arv wrote:
> >
>
https://codereview.chromium.org/613283002/diff/40001/test/test262/test262.status
> > File test/test262/test262.status (right):
> > 
> >
>
https://codereview.chromium.org/613283002/diff/40001/test/test262/test262.sta...
> > test/test262/test262.status:97: '15.2.3.4-1-4': [SKIP],
> > Based on the comments in other reviews this can be PASS FAIL_OK. That way
the
> > tests are run at least.
> 
> r24555 changed the [SKIP] to [FAIL_OK] rather than [PASS, FAIL_OK] --- I'm
> guessing this is the right way to mark the tests as known failures.
> 
> I admittedly don't fully understand what having a PASS outcome adds to this
---
> does that mean that it doesn't matter if the test starts passing some day?
> Glancing at statusfile.py and testsuite.py isn't really answering this for me.
> 
> Anyways, I'll change this to [FAIL_OK] for now since that's what's in the tree
> already.

I don't know the exact semantics of these either. [PASS, FAIL_OK] sounds useful
for flaky tests. FAIL_OK sounds right for this case.

[arv (Not doing code reviews), 2014-10-14 19:58:42]

LGTM

[arv (Not doing code reviews), 2014-10-14 19:59:00]

Dmitry, care to review and land?

[Jakob Kummerow, 2014-10-14 21:57:29]

DBC to shed some light on test expectations:

As the list notation kind of indicates, the status file defines sets of expected
outcomes. If a test's actual outcome is not in the list of expected outcomes, it
will be reported as a failure. No expectations, or an empty list, are equivalent
to [PASS].
- FAIL means "this test is expected to fail". FAIL also, to the human reader,
implies that the fact that this test is failing is not a good thing.
- FAIL_OK from the test driver's point of view is the same as FAIL, but to the
human reader indicates that this is an acceptable failure, e.g. for intentional
spec violations.
- [PASS, FAIL] is the old way to mark flaky tests; these days there's also
FLAKY. I'm not sure if there's a practical difference between the two. Avoid
flaky tests. Avoid nondeterministic implementations.
- [PASS, FAIL_OK] does not make sense.

[Dmitry Lomov (no reviews), 2014-10-15 06:59:17]

Patch lgtm, will land
(note to self: this needs blink expectations update)

[caitp (gmail), 2014-10-15 12:50:17]

r24632 looks responsible for many of these failures (from a glance, could be
wrong)

[Dmitry Lomov (no reviews), 2014-10-15 12:57:13]

On 2014/10/15 12:50:17, caitp wrote:
> r24632 looks responsible for many of these failures (from a glance, could be
> wrong)

Yes, nothing to worry about yet - the tree is closed anyway

[Dmitry Lomov (no reviews), 2014-10-15 14:01:32]

Committed patchset #1 (id:60001) manually as 24638 (presubmit successful).