Skip to content

Conversation

@AnnaMag
Copy link
Member

@AnnaMagAnnaMag commented Dec 14, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

tests

Description of change

A test addressing #6287:
Deleting property in the vm context has no effect
is added to the known_issues directory.
It will be fixed with the 5.5 V8 API changes

@nodejs-github-botnodejs-github-bot added test Issues and PRs related to the tests. lts-watch-v6.x labels Dec 14, 2016
@AnnaMagAnnaMag mentioned this pull request Dec 14, 2016
3 tasks
@fhinkel
Copy link
Member

Is this test "moved" or newly added?

@AnnaMag
Copy link
MemberAuthor

added- comment fixed. thanks!

const assert = require('assert');
const vm = require('vm');

var context = vm.createContext();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be const?

const res = vm.runInContext(`
this.x = 'prop'
delete this.x;
Object.getOwnPropertyDescriptor(this, "x");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use single quotes with x here?

@@ -0,0 +1,15 @@
'use strict'
// Refs:https://github.com/nodejs/node/issues/6287
Copy link
Contributor

Choose a reason for hiding this comment

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

Space after the colon please.

Deleting property in the vm context has no effect as reported in nodejs#6287 The test is moved to the known_issues and will be fixed with the 5.5 V8 API changes.
@AnnaMag
Copy link
MemberAuthor

@cjihrig, done

Copy link
Contributor

@Fishrock123Fishrock123 left a comment

Choose a reason for hiding this comment

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

@italoacasas
Copy link

Landed 0d3ac89

Thanks @AnnaMag

italoacasas pushed a commit that referenced this pull request Dec 18, 2016
Deleting property in the vm context has no effect as reported in #6287 The test is moved to the known_issues and will be fixed with the 5.5 V8 API changes. PR-URL: #10272 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
cjihrig pushed a commit to cjihrig/node that referenced this pull request Dec 20, 2016
Deleting property in the vm context has no effect as reported in nodejs#6287 The test is moved to the known_issues and will be fixed with the 5.5 V8 API changes. PR-URL: nodejs#10272 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@italoacasasitaloacasas mentioned this pull request Dec 20, 2016
cjihrig pushed a commit that referenced this pull request Dec 20, 2016
Deleting property in the vm context has no effect as reported in #6287 The test is moved to the known_issues and will be fixed with the 5.5 V8 API changes. PR-URL: #10272 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2017
Deleting property in the vm context has no effect as reported in #6287 The test is moved to the known_issues and will be fixed with the 5.5 V8 API changes. PR-URL: #10272 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
Deleting property in the vm context has no effect as reported in #6287 The test is moved to the known_issues and will be fixed with the 5.5 V8 API changes. PR-URL: #10272 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Jan 24, 2017
@AnnaMagAnnaMag deleted the vm-tests_known_issues branch January 25, 2017 14:58
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
Deleting property in the vm context has no effect as reported in #6287 The test is moved to the known_issues and will be fixed with the 5.5 V8 API changes. PR-URL: #10272 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@AnnaMag@fhinkel@italoacasas@Fishrock123@cjihrig@MylesBorins@nodejs-github-bot