FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: add new.target add-on regression test by bnoordhuis · Pull Request #9689 · nodejs/node · GitHub

/ node Public

test: add new.target add-on regression test - #9689

Merged
bnoordhuis merged 1 commit into
nodejs:masterfrom
bnoordhuis:new-target-addon-test
Nov 21, 2016
ConversationCommits (1)ChecksFiles changed
Merged

test: add new.target add-on regression test#9689
bnoordhuis merged 1 commit into
nodejs:masterfrom
bnoordhuis:new-target-addon-test

Conversation

bnoordhuis commented Nov 18, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

Add a test that checks that new.target inheritance works when inheriting
from a constructor defined in C++.

Refs: #9288
Refs: #9293

CI: https://ci.nodejs.org/job/node-test-pull-request/4900/

nodejs-github-bot added the test Issues and PRs related to the tests. label Nov 18, 2016
addaleax added the addons Issues and PRs related to native addons. label Nov 18, 2016
addaleax approved these changes Nov 18, 2016
View reviewed changes

addaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Comment thread test/addons/new-target/test.js Outdated

addaleax Nov 18, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Just a suggestion but maybe check new Class() instanceof binding.Class, too?

bnoordhuis Nov 18, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Good idea, done. Also fixed the lint warning.

Comment thread test/addons/new-target/binding.cc Outdated

addaleax Nov 18, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The linter seems to want anonymous namespace :P

bnoordhuis mentioned this pull request Nov 18, 2016
Merged
test: add new.target add-on regression test
13c4f44 
Add a test that checks that new.target inheritance works when inheriting
from a constructor defined in C++.

PR-URL: nodejs#9689
Refs: nodejs#9288
Refs: nodejs#9293
Reviewed-By: Anna Henningsen <anna@addaleax.net>
bnoordhuis force-pushed the new-target-addon-test branch from 2614eca to 13c4f44 Compare November 21, 2016 13:37
bnoordhuis closed this Nov 21, 2016
bnoordhuis deleted the new-target-addon-test branch November 21, 2016 13:37
bnoordhuis merged commit 13c4f44 into nodejs:master Nov 21, 2016
addaleax pushed a commit that referenced this pull request Nov 22, 2016
test: add new.target add-on regression test
7c1a2f5 
Add a test that checks that new.target inheritance works when inheriting
from a constructor defined in C++.

PR-URL: #9689
Refs: #9288
Refs: #9293
Reviewed-By: Anna Henningsen <anna@addaleax.net>

MylesBorins commented Dec 20, 2016

Copy link
Copy Markdown
Contributor

@bnoordhuis this is causing failures. Can you manually backport?

MylesBorins pushed a commit that referenced this pull request Dec 20, 2016
test: add new.target add-on regression test
550393d 
Add a test that checks that new.target inheritance works when inheriting
from a constructor defined in C++.

PR-URL: #9689
Refs: #9288
Refs: #9293
Reviewed-By: Anna Henningsen <anna@addaleax.net>

addaleax commented Dec 21, 2016

Copy link
Copy Markdown
Member

@thealphanerd The v6.x backport is at #9293 (so I’m removing lts-watch-v6.x) and I am not sure this is feasible for v4.x at all

MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
test: add new.target add-on regression test
350ed47 
Add a test that checks that new.target inheritance works when inheriting
from a constructor defined in C++.

PR-URL: #9689
Refs: #9288
Refs: #9293
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins mentioned this pull request Dec 21, 2016
Merged
bnoordhuis added a commit to bnoordhuis/io.js that referenced this pull request Mar 2, 2017
test: add new.target add-on regression test
2ebceed 
Add a test that checks that new.target inheritance works when inheriting
from a constructor defined in C++.

PR-URL: nodejs#9689
Refs: nodejs#9288
Refs: nodejs#9293
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
test: add new.target add-on regression test
87760cc 
Add a test that checks that new.target inheritance works when inheriting
from a constructor defined in C++.

PR-URL: #9689
Refs: #9288
Refs: #9293
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins mentioned this pull request Mar 9, 2017
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

addaleax addaleax approved these changes

Assignees

No one assigned

Labels

addons Issues and PRs related to native addons. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL