Repository navigation
10.7.0 broke the Yarn tests #21907
Description
Activity
@arcanis This does not look to be observed when run without the mock from https://github.com/yarnpkg/yarn/blob/master/__tests__/__mocks__/request.js.
A reproducable testcase without all the other
yarnstuff (but withrequest)const request = require('request'); const https = require('https'); const fs = require('fs'); const opt = {"url":"https://registry.yarnpkg.com/yeoman-environment","method":"GET","headers":{"User-Agent":"yarn/1.10.0-0 npm/? node/v10.7.0 linux x64","Accept":"application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*"},"json":true,"gzip":true,"forever":true,"retryAttempts":0,"strictSSL":true,"cert":"","key":"","timeout":30000}; const req = request(opt, () => {}); req.httpModule = { request: function request(options, callback) { const loc = 'yeoman-environment.bin'; options.agent = null; options.socketPath = null; options.createConnection = () => { return fs.createReadStream(loc); }; return https.request(options, callback); } };
This is basically what Yarn does.
You're right, seems like this is caused in particular by the mock of
createConnection:options.createConnection = (): ReadStream => { return fs.createReadStream(loc); };
The previous code was attaching on the
connectevent from the fake socket, which was never triggered and so the setTimeout was never called. Now that setTimeout is always called when the socket is there, the bug triggers.I think this is on Yarn, so I'm going to close this issue. Feel free to reopen if you think it hides a more problematic regression.
And thanks for your help, greatly appreciated!
If it's the mock, my guess is that it is due to
options.createConnection = (): ReadStream => { return fs.createReadStream(loc); };Wow a lot of conversation happened while I was posted that :) disregard
@arcanis This is one of the reasons why trying to keep the reproducable testcases minimal matters.
I attempted to construct a testcase without yarn. To trace that, I noticed that the error was coming from the
requestmodule (as observed in the original trace), and tried to check how exactly are you usingrequest. That was done by adding these two lines to it:--- node_modules/request/request.js *************** *** 93,94 **** --- 93,96 ---- function Request (options) { + console.log('Request', JSON.stringify(options)); + console.log((new Error()).stack) // if given the method property in options, set property explicitMethod to true
That brought me directly to the mock file (and gave the short testcase in the comment above).
Hope that helps 😉.
Yeah, I actually logged the options to try to make repros, but completely forgot we mocked request, so didn't check the stack 😐
Related: There's a PR to get
yarnintocitgmbut it stalled. If someone wants to take it over and get it across the finish line, it would be great to havecitgmflag these kinds of things for us before a release: nodejs/citgm#560Reacted by Maël Nison, Sagir Khan, Nikita Skovoroda and killa
The 10.7.0 release is causing the Yarn test suite to fail (which prevents us from releasing the 1.9, but also might cause issues with everyone already using Yarn), something related to timeouts:
A single commit has been pushed to this file between the 10.6.0 and the 10.7.0 and it happens to change the timeout behavior, which would be consistent with the reported error: 949e885 (cc @killagu).
The repro: clone git@github.com:yarnpkg/yarn and run:
I realize it's not a great repro, I'm still working on finding a shorter one.