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

Object.prototype manipulations cause weirdness #18

Open
isaacs opened this issue Aug 26, 2016 · 0 comments
Open

Object.prototype manipulations cause weirdness #18

isaacs opened this issue Aug 26, 2016 · 0 comments
Labels

Comments

@isaacs
Copy link

isaacs commented Aug 26, 2016

Someone send a PR to node-tap that defends against tests that might mutate Object.prototype, essentially replacing some for (var i in obj) loops with Object.keys(obj).forEach(..), or adding a if(obj.hasOwnProperty(i)) guard.

Here's the test:

// test/has-own-property.js
Object.prototype.foo = 'bar'
require('../').pass('modifying Object.prototype did not break things')

Running with coverage does this:

$ nyc node test/has-own-property.js
Transformation error for /Users/isaacs/dev/js/tap/lib/root.js ; return original code
Cannot assign to read only property 'optional' of bar
Transformation error for /Users/isaacs/dev/js/tap/lib/test.js ; return original code
Cannot assign to read only property 'optional' of bar
Transformation error for /Users/isaacs/dev/js/tap/lib/stack.js ; return original code
Cannot assign to read only property 'optional' of bar
Transformation error for /Users/isaacs/dev/js/tap/lib/assert.js ; return original code
Cannot assign to read only property 'optional' of bar
Transformation error for /Users/isaacs/dev/js/tap/lib/synonyms.js ; return original code
Cannot assign to read only property 'optional' of bar
Transformation error for /Users/isaacs/dev/js/tap/lib/mocha.js ; return original code
Cannot assign to read only property 'optional' of bar
TAP version 13
ok 1 - modifying Object.prototype did not break things
1..1
# time=23.336ms
----------|----------|----------|----------|----------|----------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
All files |  Unknown |  Unknown |  Unknown |  Unknown |                |
----------|----------|----------|----------|----------|----------------|

I was able to make a simpler example but it has a different error:

// t.js
Object.prototype.foo = 'bar'
require('./x.js')
// x.js
var x = { baz: 1 }
$ nyc node t.js
failed to instrument /Users/isaacs/dev/js/tap/x.js with error: You passed `traverse()` a visitor object with the property "Program" that has the invalid property "foo"
----------|----------|----------|----------|----------|----------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
All files |      100 |      100 |      100 |      100 |                |
 t.js     |      100 |      100 |      100 |      100 |                |
----------|----------|----------|----------|----------|----------------|
isaacs added a commit to tapjs/tapjs that referenced this issue Aug 26, 2016
The same things are still tested, but this test file makes `npm test`
spew tons of errors.  Revert this commit when istanbul is resilient
against Object.prototype mutations.

See istanbuljs-archived-repos/istanbul-lib-instrument#18
@bcoe bcoe added the bug label Aug 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants