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

test: fix V8 test on big-endian machines #12186

Closed
wants to merge 1 commit into from

Conversation

@addaleax
Copy link
Member

commented Apr 3, 2017

@addaleax

This comment has been minimized.

Copy link
Member Author

commented Apr 3, 2017

CI: https://ci.nodejs.org/job/node-test-commit/8852/

I’d like to land this if the it comes back green to un-break CI … sorry everyone :/

@addaleax addaleax added the V8 Engine label Apr 3, 2017

@gibfahn

This comment has been minimized.

Copy link
Member

commented Apr 3, 2017

+1 to landing as soon as CI comes back (it already passed on linuxone so we're looking good).

Thanks for getting on this so quickly @addaleax !

@gibfahn gibfahn added aix ppc s390 labels Apr 3, 2017

@gibfahn
gibfahn approved these changes Apr 3, 2017
@mhdawson
Copy link
Member

left a comment

LGTM

@targos
targos approved these changes Apr 3, 2017
assert.deepStrictEqual(v8.deserialize(buf),
new Uint16Array([0xdead, 0xbeef]));
expectedResult);

This comment has been minimized.

Copy link
@targos

targos Apr 3, 2017

Member

nit: this doesn't need to be on a new line anymore

@cjihrig
cjihrig approved these changes Apr 3, 2017
@addaleax

This comment has been minimized.

Copy link
Member Author

commented Apr 3, 2017

CI is passing on aix, linuxone and ppc, so yes, this seems good. I’ve landed this in 9b05393 (with @targos’ style nit fixed) given that this change seems unlikely to result in surprises and all relevant platforms are covered.

Sorry again!

@addaleax addaleax closed this Apr 3, 2017

@addaleax addaleax deleted the addaleax:v8-serdes-be-fix branch Apr 3, 2017

addaleax added a commit that referenced this pull request Apr 3, 2017
test: fix V8 test on big-endian machines
Ref: #12143 (comment)
PR-URL: #12186
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@jasnell jasnell referenced this pull request Apr 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.