New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for Symbol.prototype.description #1590

Merged
merged 3 commits into from Jun 8, 2018

Conversation

Projects
None yet
5 participants
@joyeecheung
Copy link
Contributor

joyeecheung commented Jun 7, 2018

@joyeecheung

This comment has been minimized.

Copy link
Contributor

joyeecheung commented Jun 7, 2018

const desc = Object.getOwnPropertyDescriptor(Symbol.prototype, 'description');
assert.sameValue(typeof desc.get, 'function');
assert.sameValue(desc.set, undefined);
assert.sameValue(desc.writable, undefined);

This comment has been minimized.

@ljharb

ljharb Jun 7, 2018

Member

There’s a propertyHelper that could be used here

assert.sameValue(symbol.description, 'test');
assert.sameValue(symbol.hasOwnProperty('description'), false);

const empty = Symbol();

This comment was marked as resolved.

@ljharb

ljharb Jun 7, 2018

Member

Could you also test Symbol(undefined) and Symbol(‘’)?

---*/

assert.throws(TypeError, function() {
Symbol.prototype.description.call(null);

This comment has been minimized.

@michaelficarra

michaelficarra Jun 7, 2018

Member

This fails, but for the wrong reason. The getter is being called on Symbol.prototype. You should instead extract the getter with Object.getOwnPropertyDescriptor.

@joyeecheung joyeecheung force-pushed the joyeecheung:symbol-desc branch from 7508adf to 28a66ce Jun 7, 2018

@joyeecheung

This comment has been minimized.

Copy link
Contributor

joyeecheung commented Jun 7, 2018

@ljharb @michaelficarra Thanks for the reviews. I have updated the PR, PTAL.

@ljharb
Copy link
Member

ljharb left a comment

LGTM - we may also want to add some tests that .call-ing the getter on real symbols works too?

@joyeecheung joyeecheung dismissed stale reviews from gsathya, ljharb, and michaelficarra via 21c2bab Jun 7, 2018

@joyeecheung joyeecheung force-pushed the joyeecheung:symbol-desc branch from 21c2bab to 70b9738 Jun 7, 2018

@joyeecheung

This comment has been minimized.

Copy link
Contributor

joyeecheung commented Jun 7, 2018

LGTM - we may also want to add some tests that .call-ing the getter on real symbols works too?

@ljharb Sure! I've updated the PR, PTAL.

@ljharb

ljharb approved these changes Jun 7, 2018

@rwaldron rwaldron merged commit 5d58479 into tc39:master Jun 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@michaelficarra michaelficarra referenced this pull request Jun 8, 2018

Closed

test262 tests #5

@ljharb ljharb referenced this pull request Aug 29, 2018

Closed

Stage 4 Tracking #7

17 of 19 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment