Skip to content

Commit

Permalink
Fix sync XHR in Node.js v12
Browse files Browse the repository at this point in the history
The `spawnSync()` buffer set in nodejs/node#23027 is too small for our use case. This sets the `maxBuffer` option to `Infinity` again as in previous versions of Node.js.
  • Loading branch information
Zirro committed Apr 26, 2019
1 parent 7dd8f18 commit bde71ae
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
2 changes: 1 addition & 1 deletion lib/jsdom/living/xmlhttprequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ module.exports = function createXMLHttpRequest(window) {
const res = spawnSync(
process.execPath,
[syncWorkerFile],
{ input: flagStr }
{ input: flagStr, maxBuffer: Infinity }
);
if (res.status !== 0) {
throw new Error(res.stderr.toString());
Expand Down
9 changes: 7 additions & 2 deletions test/web-platform-tests/run-wpts.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@ const validReasons = new Set([
"timeout",
"flaky",
"mutates-globals",
"fails-node12",
"needs-node10",
"needs-node11"
"needs-node11",
"needs-node12"
]);

const hasNode10 = Number(process.versions.node.split(".")[0]) >= 10;
const hasNode11 = Number(process.versions.node.split(".")[0]) >= 11;
const hasNode12 = Number(process.versions.node.split(".")[0]) >= 12;

const manifestFilename = path.resolve(__dirname, "wpt-manifest.json");
const manifest = readManifest(manifestFilename);
Expand Down Expand Up @@ -54,8 +57,10 @@ describe("web-platform-tests", () => {
const reason = matchingPattern && toRunDoc[matchingPattern][0];
const shouldSkip = ["fail-slow", "timeout", "flaky", "mutates-globals"].includes(reason);
const expectFail = (reason === "fail") ||
(reason === "fails-node12" && hasNode12) ||
(reason === "needs-node10" && !hasNode10) ||
(reason === "needs-node11" && !hasNode11);
(reason === "needs-node11" && !hasNode11) ||
(reason === "needs-node12" && !hasNode12);

if (matchingPattern && shouldSkip) {
specify.skip(`[${reason}] ${testFile}`);
Expand Down
5 changes: 3 additions & 2 deletions test/web-platform-tests/to-run.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ basic.htm: [fail, Unknown]
client-hint-request-headers-2.tentative.htm: [fail, Client Hints not implemented]
client-hint-request-headers.htm: [fail-slow, Client Hints not implemented]
cors-safelisted-request-header.any.html: [fail, Depends on fetch]
credentials-flag.htm: [fail, Unknown]
credentials-flag.htm: [needs-node12, Unknown]
image-tainting-in-cross-origin-iframe.sub.html: [timeout, Unknown]
origin.htm: [timeout, https://github.com/tmpvar/jsdom/issues/1833]
preflight-cache.htm: [timeout, Cache should probably be implemented for simple requests before]
Expand Down Expand Up @@ -126,7 +126,7 @@ DIR: dom/nodes
Comment-constructor.html: [fail, Unknown]
DOMImplementation-createDocument.html: [fail, Unknown]
Document-URL.html: [fail, Unknown]
Document-characterSet-normalization.html: [fail, Some encodings are not supported - see the whatwg-encoding module]
Document-characterSet-normalization.html: [timeout, Some encodings are not supported - see the whatwg-encoding module]
Document-constructor-svg.svg: [fail, Unknown]
Document-constructor-xml.xml: [fail, Unknown]
Document-constructor.html: [fail, new Document().origin should inherit from current Document]
Expand Down Expand Up @@ -976,6 +976,7 @@ open-url-worker-origin.htm: [fail, Needs Worker implementation]
open-url-worker-simple.htm: [timeout, Needs Worker implementation]
overridemimetype-blob.html: [fail, Unknown]
overridemimetype-edge-cases.window.html: [fail, Unknown]
response-method.htm: [fails-node12, The behavior changed with Node's new HTTP parser, https://github.com/nodejs/node/issues/27049]
responseType-document-in-worker.html: [fail, Needs Worker implementation]
responseXML-unavailable-in-worker.html: [fail, Needs Worker implementation]
responsedocument-decoding.htm: [fail, Unknown]
Expand Down

0 comments on commit bde71ae

Please sign in to comment.