This repository has been archived by the owner on Oct 15, 2020. It is now read-only.
Enable inspector tests for ChakraCore #247
Merged
Conversation
This file contains 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
kfarnung
requested review from
digitalinfinity,
jianchun,
agarwal-sandeep and
kunalspathak
digitalinfinity
approved these changes
May 18, 2017
test/inspector/inspector.status
Outdated
| @@ -9,4 +9,3 @@ prefix inspector | |||
| [$system==win32] | |||
|
|
|||
| [$jsEngine==chakracore] | |||
There was a problem hiding this comment.
Why not remove this section altogether and have it match upstream?
There was a problem hiding this comment.
Easy enough, I'll do that.
| @@ -8,7 +8,7 @@ const { test, assert_equals } = common.WPT; | |||
| const additionalTestCases = require( | |||
| path.join(common.fixturesDir, 'url-setter-tests-additional.js')); | |||
|
|
|||
| if (!common.hasIntl) { | |||
| if (!common.hasIntl || common.isChakraEngine) { | |||
There was a problem hiding this comment.
Isn't this too broad? Like Intl should be available on Windows, no?
There was a problem hiding this comment.
I can take another look, but this was an issue on Windows in my testing.
There was a problem hiding this comment.
Odd, here's the failure on see on Windows:
=== release test-whatwg-url-setters ===
Path: parallel/test-whatwg-url-setters
TypeError: Object doesn't support property or method 'ToString'
at href.set (internal/url.js:307:7)
at Anonymous function (E:\GitHub\node-chakracore\test\parallel\test-whatwg-url-setters.js:121:27)
at _tryBlock (assert.js:503:5)
at _throws (assert.js:522:3)
at throws (assert.js:552:3)
at Anonymous function (E:\GitHub\node-chakracore\test\parallel\test-whatwg-url-setters.js:121:7)
at Module.prototype._compile (module.js:582:5)
at Module._extensions[.js] (module.js:593:3)
at Module.prototype.load (module.js:516:3)
at tryModuleLoad (module.js:466:5)
Command: E:\GitHub\node-chakracore\Release\node.exe E:\GitHub\node-chakracore\test\parallel\test-whatwg-url-setters.js
I'll investigate further.
|
LGTM. Agree with @digitalinfinity feedback though. |
kfarnung
force-pushed
the
inspect_tests
branch
2 times, most recently
from
c870860
to
f923b15
Compare
|
This is the first part of #227. |
|
This should also resolve #226 |
2 tasks
* Enabled intl support to get UTF-8/UTF-16 conversion for inspector. This is a temporary move before actual intl support is enabled. * Removed workaround for UTF-8 conversion functions that weren't available before ICU was enabled. PR-URL: nodejs#247 Reviewed-By: Hitesh Kanwathirtha <[email protected]> Reviewed-By: Kunal Pathak <[email protected]>
Enabled tests for inspector and updated them to account for differences between V8 and ChakraCore. PR-URL: nodejs#247 Reviewed-By: Hitesh Kanwathirtha <[email protected]> Reviewed-By: Kunal Pathak <[email protected]>
kfarnung
commented
May 19, 2017
| @@ -1,476 +1,445 @@ | |||
| #include "inspector_io.h" | |||
There was a problem hiding this comment.
This is the result of git checkout nodejs/master src/inspector_io.cc. At some point the line endings must have gotten messed up. The diff here just removes the UTF-8/UTF-16 conversion workarounds that I added for inspector.
A new rule was added to enforce function declaration style. PR-URL: nodejs#247 Reviewed-By: Hitesh Kanwathirtha <[email protected]> Reviewed-By: Kunal Pathak <[email protected]>
kfarnung
added a commit
to kfarnung/node-chakracore
that referenced
this pull request
May 23, 2017
A new rule was added to enforce function declaration style. PR-URL: nodejs#247 Reviewed-By: Hitesh Kanwathirtha <[email protected]> Reviewed-By: Kunal Pathak <[email protected]>
kfarnung
added a commit
to kfarnung/node-chakracore
that referenced
this pull request
May 23, 2017
* Enabled intl support to get UTF-8/UTF-16 conversion for inspector. This is a temporary move before actual intl support is enabled. * Removed workaround for UTF-8 conversion functions that weren't available before ICU was enabled. PR-URL: nodejs#247 Reviewed-By: Hitesh Kanwathirtha <[email protected]> Reviewed-By: Kunal Pathak <[email protected]>
kfarnung
added a commit
to kfarnung/node-chakracore
that referenced
this pull request
May 23, 2017
Enabled tests for inspector and updated them to account for differences between V8 and ChakraCore. PR-URL: nodejs#247 Reviewed-By: Hitesh Kanwathirtha <[email protected]> Reviewed-By: Kunal Pathak <[email protected]>
kfarnung
added a commit
to kfarnung/node-chakracore
that referenced
this pull request
May 23, 2017
* Enabled intl support to get UTF-8/UTF-16 conversion for inspector. This is a temporary move before actual intl support is enabled. * Removed workaround for UTF-8 conversion functions that weren't available before ICU was enabled. PR-URL: nodejs#247 Reviewed-By: Hitesh Kanwathirtha <[email protected]> Reviewed-By: Kunal Pathak <[email protected]>
kfarnung
added a commit
to kfarnung/node-chakracore
that referenced
this pull request
May 23, 2017
Enabled tests for inspector and updated them to account for differences between V8 and ChakraCore. PR-URL: nodejs#247 Reviewed-By: Hitesh Kanwathirtha <[email protected]> Reviewed-By: Kunal Pathak <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
build, test