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

fix: Wait for child process to be ready #19792

Merged
merged 8 commits into from
Feb 17, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions packages/server/.gitignore

This file was deleted.

2 changes: 2 additions & 0 deletions packages/server/lib/plugins/child/run_plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ const runPlugins = (ipc, pluginsFile, projectRoot) => {
ipc.on('execute', (event, ids, args) => {
execute(ipc, event, ids, args)
})

ipc.send('ready')
}

// for testing purposes
Expand Down
4 changes: 3 additions & 1 deletion packages/server/lib/plugins/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ const init = (config, options) => {
Object.keys(config).sort().forEach((key) => orderedConfig[key] = config[key])
config = orderedConfig

ipc.send('load', config)
ipc.on('ready', () => {
ipc.send('load', config)
})

ipc.on('loaded', (newCfg, registrations) => {
_.omit(config, 'projectRoot', 'configFile')
Expand Down
1 change: 0 additions & 1 deletion packages/server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@
"eventsource": "1.0.7",
"express-session": "1.16.1",
"express-useragent": "1.0.15",
"http-mitm-proxy": "0.7.0",
"https-proxy-agent": "3.0.1",
"istanbul": "0.4.5",
"mocha": "7.1.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/server/test/unit/plugins/index_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ describe('lib/plugins/index', () => {
})
})

it('sends \'load\' event with config via ipc', () => {
it('sends \'load\' event with config via ipc once it receives \'ready\'', () => {
ipc.on.withArgs('ready').yields([])
Copy link
Contributor

Choose a reason for hiding this comment

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

This test seems to pass without the changes in this PR. Can you add a test that demonstrates the failing case fixed by this PR (asynchronous plugin registration)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I'll try to fix that in the next couple of days.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the test. Ready to be reviewed again :)

ipc.on.withArgs('loaded').yields([])
const config = { pluginsFile: 'cypress-plugin', testingType: 'e2e' }

Expand Down
1 change: 0 additions & 1 deletion system-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
"fluent-ffmpeg": "2.1.2",
"fs-extra": "8.1.0",
"glob": "7.2.0",
"http-mitm-proxy": "0.7.0",
"https-proxy-agent": "3.0.1",
"human-interval": "1.0.0",
"image-size": "0.8.3",
Expand Down
52 changes: 22 additions & 30 deletions system-tests/test/network_error_handling_spec.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
const _ = require('lodash')
const express = require('express')
const http = require('http')
const https = require('https')
const path = require('path')
const debug = require('debug')('cypress:server:network-error-handling-spec')
const Promise = require('bluebird')
Expand All @@ -12,7 +10,6 @@ const chrome = require('@packages/server/lib/browsers/chrome')
const systemTests = require('../lib/system-tests').default
const random = require('@packages/server/lib/util/random')
const Fixtures = require('../lib/fixtures')
let mitmProxy = require('http-mitm-proxy')

const PORT = 13370
const PROXY_PORT = 13371
Expand Down Expand Up @@ -348,19 +345,19 @@ describe('e2e network error handling', function () {
})

context('Cypress', () => {
let debugProxy

beforeEach(() => {
delete process.env.HTTP_PROXY
delete process.env.HTTPS_PROXY

return delete process.env.NO_PROXY
delete process.env.NO_PROXY
})

afterEach(function () {
if (this.debugProxy) {
return this.debugProxy.stop()
.then(() => {
this.debugProxy = null
})
afterEach(async function () {
if (debugProxy) {
await debugProxy.stop()
debugProxy = null
}
})

Expand Down Expand Up @@ -415,11 +412,11 @@ describe('e2e network error handling', function () {
return true
})

this.debugProxy = new DebugProxy({
debugProxy = new DebugProxy({
onConnect,
})

return this.debugProxy
return debugProxy
.start(PROXY_PORT)
.then(() => {
process.env.HTTP_PROXY = `http://localhost:${PROXY_PORT}`
Expand Down Expand Up @@ -465,9 +462,9 @@ describe('e2e network error handling', function () {
})

it('behind a proxy', function () {
this.debugProxy = new DebugProxy()
debugProxy = new DebugProxy()

return this.debugProxy
return debugProxy
.start(PROXY_PORT)
.then(() => {
process.env.HTTP_PROXY = `http://localhost:${PROXY_PORT}`
Expand All @@ -485,27 +482,22 @@ describe('e2e network error handling', function () {
})
})

it('behind a proxy with transfer-encoding: chunked', function () {
mitmProxy = mitmProxy()

mitmProxy.onRequest((ctx, callback) => {
return callback()
})

mitmProxy.listen({
host: '127.0.0.1',
port: PROXY_PORT,
keepAlive: true,
httpAgent: http.globalAgent,
httpsAgent: https.globalAgent,
forceSNI: false,
forceChunkedRequest: true,
it('behind a proxy with transfer-encoding: chunked', async function () {
debugProxy = new DebugProxy({
onRequest: (reqUrl, req, res) => {
expect(req.headers).to.have.property('content-length')
// delete content-length to force te: chunked
delete req.headers['content-length']
debugProxy._onRequest(reqUrl, req, res)
},
})

process.env.HTTP_PROXY = `http://localhost:${PROXY_PORT}`
process.env.NO_PROXY = ''

return systemTests.exec(this, {
await debugProxy.start(PROXY_PORT)

await systemTests.exec(this, {
spec: 'network_error_304_handling_spec.js',
video: false,
config: {
Expand Down
24 changes: 3 additions & 21 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -10983,7 +10983,7 @@ async@>=0.2.9, async@^3.2.0:
resolved "https://registry.yarnpkg.com/async/-/async-3.2.0.tgz#b3a2685c5ebb641d3de02d161002c60fc9f85720"
integrity sha512-TR2mEZFVOj2pLStYxLht7TyfuRzaydfpxr3k9RpHIzMgw7A64dzsdqCxH1WJyQdoe8T10nDXd9wnEigmiuHIZw==

async@^2.1.4, async@^2.4.1, async@^2.5.0, async@^2.6.2:
async@^2.1.4, async@^2.4.1, async@^2.6.2:
version "2.6.3"
resolved "https://registry.yarnpkg.com/async/-/async-2.6.3.tgz#d72625e2344a3656e3a3ad4fa749fa83299d82ff"
integrity sha512-zflvls11DCy+dQWzTW2dzuilv8Z5X/pjfmZOWba6TNIVDm+2UDaJmXSOXlasHKfNBs8oo3M0aT50fDEWfKZjXg==
Expand Down Expand Up @@ -21993,19 +21993,6 @@ http-errors@~1.6.2:
setprototypeof "1.1.0"
statuses ">= 1.4.0 < 2"

http-mitm-proxy@0.7.0:
version "0.7.0"
resolved "https://registry.yarnpkg.com/http-mitm-proxy/-/http-mitm-proxy-0.7.0.tgz#82933137ae1c06713961afe50f38ca84cf80bb0c"
integrity sha512-rRMRfQCVwEO31Q6GFiQHfECdMn3Z0ddWWLNgmeyIUDMf0gr/Ek+lhZ17gWzKL4NXZkMc1h982BYl8blRXv7/og==
dependencies:
async "^2.5.0"
debug "^4.1.0"
mkdirp "^0.5.1"
node-forge "^0.8.0"
optimist "^0.6.1"
semaphore "^1.1.0"
ws "^3.2.0"

http-parser-js@>=0.5.1:
version "0.5.3"
resolved "https://registry.yarnpkg.com/http-parser-js/-/http-parser-js-0.5.3.tgz#01d2709c79d41698bb01d4decc5e9da4e4a033d9"
Expand Down Expand Up @@ -28363,11 +28350,6 @@ node-forge@^0.10.0:
resolved "https://registry.yarnpkg.com/node-forge/-/node-forge-0.10.0.tgz#32dea2afb3e9926f02ee5ce8794902691a676bf3"
integrity sha512-PPmu8eEeG9saEUvI97fm4OYxXVB6bFvyNTyiUOBichBpFG8A1Ljw3bY62+5oOjDEMHRnd0Y7HQ+x7uzxOzC6JA==

node-forge@^0.8.0:
version "0.8.5"
resolved "https://registry.yarnpkg.com/node-forge/-/node-forge-0.8.5.tgz#57906f07614dc72762c84cef442f427c0e1b86ee"
integrity sha512-vFMQIWt+J/7FLNyKouZ9TazT74PRV3wgv9UT4cRjC8BffxFbKXkgIWR42URCPSnHm/QDz6BOlb2Q0U4+VQT67Q==

node-gyp@^5.0.2, node-gyp@^5.1.0:
version "5.1.1"
resolved "https://registry.yarnpkg.com/node-gyp/-/node-gyp-5.1.1.tgz#eb915f7b631c937d282e33aed44cb7a025f62a3e"
Expand Down Expand Up @@ -35188,7 +35170,7 @@ semantic-release@17.4.2:
signale "^1.2.1"
yargs "^16.2.0"

semaphore@1.1.0, semaphore@^1.1.0:
semaphore@1.1.0:
version "1.1.0"
resolved "https://registry.yarnpkg.com/semaphore/-/semaphore-1.1.0.tgz#aaad8b86b20fe8e9b32b16dc2ee682a8cd26a8aa"
integrity sha512-O4OZEaNtkMd/K0i6js9SL+gqy0ZCBMgUvlSqHKi4IBdjhe7wB8pwztUk1BbZ1fmrvpwFrPbHzqd2w5pTcJH6LA==
Expand Down Expand Up @@ -41508,7 +41490,7 @@ write@1.0.3:
dependencies:
mkdirp "^0.5.1"

ws@3.3.x, ws@^3.2.0:
ws@3.3.x:
version "3.3.3"
resolved "https://registry.yarnpkg.com/ws/-/ws-3.3.3.tgz#f1cf84fe2d5e901ebce94efaece785f187a228f2"
integrity sha512-nnWLa/NwZSt4KQJu51MYlCcSQ5g7INpOrOMt4XV8j4dqTXdmlUmSHQ8/oLC069ckre0fRsgfvsKwbTdtKLCDkA==
Expand Down