Skip to content
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 extended tests for defineProperty #355

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Xotic750
Copy link
Contributor

IE7 & 8, Safari 6, 7, 8 & 9, Chrome 26, 27, Node 0.8 & 0.10 all failed some of these tests.

Some versions of eslint also have a problem when linting these tests.

/home/graham/source_projects/es5-shim/node_modules/eslint/lib/rules/no-extend-native.js:71
                object = subject.object;
                                ^

TypeError: Cannot read property 'object' of undefined
    at EventEmitter.CallExpression (/home/graham/source_projects/es5-shim/node_modules/eslint/lib/rules/no-extend-native.js:71:33)
    at emitOne (events.js:82:20)
    at EventEmitter.emit (events.js:169:7)
    at NodeEventGenerator.enterNode (/home/graham/source_projects/es5-shim/node_modules/eslint/lib/util/node-event-generator.js:42:22)
    at CommentEventGenerator.enterNode (/home/graham/source_projects/es5-shim/node_modules/eslint/lib/util/comment-event-generator.js:98:23)
    at Controller.controller.traverse.enter (/home/graham/source_projects/es5-shim/node_modules/eslint/lib/eslint.js:771:36)
    at Controller.__execute (/home/graham/source_projects/es5-shim/node_modules/eslint/node_modules/estraverse/estraverse.js:397:31)
    at Controller.traverse (/home/graham/source_projects/es5-shim/node_modules/eslint/node_modules/estraverse/estraverse.js:495:28)
    at EventEmitter.module.exports.api.verify (/home/graham/source_projects/es5-shim/node_modules/eslint/lib/eslint.js:768:24)
    at processText (/home/graham/source_projects/es5-shim/node_modules/eslint/lib/cli-engine.js:225:27)

The tests could use a little thinning out, and could give better detail of the problem. Some tests for default properties and if they work (on native) would probably also be nice.

Of course, failures here also generally mean failures in defineProperties aswell.

});

it('should not throw an error definining elements on arrays using trailing point numbers', function () {
var obj = Object.defineProperty([], 1., {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in what JS engine is (1.) distinct from (1) in any way, shape, or form???

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably none, I blindly ported everything and as I mentioned, some of this needs thinning out. A lot of my tests are if you don't try it then you will never know, but that's how I manage to find some of this stuff. ;)

@ljharb
Copy link
Member

ljharb commented Apr 22, 2022

@Xotic750 can you please check the "allow edits" checkbox on the RHS of the PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants