fix: Wait for child process to be ready #19792

jhnns · 2022-01-20T17:28:43Z

User facing changelog

Fixes an issue where Cypress did not start. It also adds support for node's experimental --loader feature for plugins.

Additional details

This PR fixes an issue in the server package where the plugin function never gets executed, leaving Cypress stuck in the init phase. In this case, the UI just shows the loading spinner (could be the cause for #19782, and probably more...) . I've created a minimal reproducible test case at: https://github.com/jhnns/cypress-test-tiny/tree/native-esm-with-ts

The original code fork()s the process and immediately sends the load event. There's the implicit assumption that the child process is ready as soon as fork() returns which is not guaranteed afaik. The issue becomes apparent when using node's experimental --loader feature which may defer the process initialization. In this case, send() is executed before the child gets the chance to register for the event.

The test case also demonstrates how this fix could enable TypeScript + native ESM in Cypress plugins (using NODE_OPTIONS='--loader ts-node/esm/transpile-only')

Obviously, this PR is not finished yet. I couldn't get the setup running on my Mac M1. So, I just wanted to send you this PR to get some feedback and maybe support on how to get this merged.

How has the user experience changed?

PR Tasks

Have tests been added/updated?
Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
[na] Has a PR for user-facing changes been opened in cypress-documentation?
[na] Have API changes been updated in the type definitions?
[na] Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2022-01-20T17:28:46Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

CLAassistant · 2022-01-20T17:28:48Z

All committers have signed the CLA.

jhnns · 2022-01-21T09:03:44Z

Marking it as "Ready for review" because I'd like to get some feedback on this change before trying to get the tests up and running on my machine 😁

flotwig · 2022-01-27T16:29:39Z

Marking it as "Ready for review" because I'd like to get some feedback on this change before trying to get the tests up and running on my machine grin

Hey @jhnns, this seems like a good approach. We've seen this bug occasionally but never were able to track it down to a reproducible example, so having this fix and a test for it would be fantastic. Let me know if I can help you progress this PR in any way.

jhnns · 2022-01-27T16:35:03Z

Awesome 👍

Then I'll try to get the repo running on my machine (or GitHub workspaces).

As already mentioned: https://github.com/jhnns/cypress-test-tiny/tree/native-esm-with-ts is a reproducible test case, although with a special node flag which is probably not that common.

…rocess

jhnns · 2022-01-28T10:47:49Z

I've managed to set up the project using the Docker container cypress/browsers:node16.5.0-chrome94-ff93
Now I've pushed a fix for the unit test, but there are a bunch of other tests failing now. I can't see how these failing tests have something to do with my change. Looks like these tests are failing on develop as well. What do you think?

flotwig · 2022-02-02T15:16:30Z

It seems like the hanging system tests are an issue with external contributor PRs, not to do with your PR specifically, we're looking into fixing it so we can get this merged.

…tributors

flotwig · 2022-02-08T18:02:55Z

packages/server/test/unit/plugins/index_spec.js

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


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)?

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

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

* develop: (35 commits) fix(deps): update dependency url-parse to v1.5.6 [security] (#20270) chore: fix cache keys to include PLATFORM (#20279) chore: fix server performance flake (#20271) test(system-tests): support docker-based tests against built binary (#20250) chore: fix system-test-firefox screenshots_spec flake (#20268) chore(deps): update dependency fs-extra to v9 🌟 (#19939) fix: Wait for child process to be ready (#19792) fix: treat form-data bodies as binary (#20144) test: replace cypress-test-example-repos coverage + remove bump (#20186) fix(driver): update wrapErr to ignore number and boolean values (#20172) release 9.5.0 [skip ci] chore: Update Chrome (stable) to 98.0.4758.102 (#20192) chore: enable volar.takeOverMode Add span names, merge develop fix: Update `.type(' ')` to not emit clicks when the keyup event has been prevented (#20156) test: remove redundant "other projects" CI jobs (#20133) chore(driver): move cy.focused and cy.root into their own files (#20054) Move sending root event to own script chore: release @cypress/vue-v3.1.1 chore: release @cypress/react-v5.12.3 ...

commit fc7149e Merge: 0e942ed 0143e13 Author: Chris Breiding <chrisbreiding@gmail.com> Date: Wed Feb 23 10:28:34 2022 -0500 Merge branch 'develop' into feature-multidomain commit 0e942ed Author: Chris Breiding <chrisbreiding@users.noreply.github.com> Date: Wed Feb 23 10:28:03 2022 -0500 chore: Refactor multi-domain communication lifecycle (#20247) commit 0143e13 Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Date: Mon Feb 21 23:01:35 2022 +0000 fix(deps): update dependency url-parse to v1.5.6 [security] (#20270) Co-authored-by: Renovate Bot <bot@renovateapp.com> commit 1fb16b0 Author: Zach Bloomquist <git@chary.us> Date: Sun Feb 20 16:22:08 2022 -0500 chore: fix cache keys to include PLATFORM (#20279) * chore: fix cache keys to include PLATFORM * build this branch * try using platform_key commit 65ea8f7 Author: Ryan Manuel <ryanm@cypress.io> Date: Fri Feb 18 17:57:34 2022 -0600 chore: fix server performance flake (#20271) commit ad2f4de Author: Zach Bloomquist <git@chary.us> Date: Fri Feb 18 18:37:22 2022 -0500 test(system-tests): support docker-based tests against built binary (#20250) Co-authored-by: Ryan Manuel <ryanm@cypress.io> commit 75c8750 Author: Ryan Manuel <ryanm@cypress.io> Date: Fri Feb 18 12:54:26 2022 -0600 chore: fix system-test-firefox screenshots_spec flake (#20268) commit 8d28261 Author: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Date: Fri Feb 18 10:33:27 2022 -0600 chore(deps): update dependency fs-extra to v9 🌟 (#19939) Co-authored-by: Renovate Bot <bot@renovateapp.com> Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com> Co-authored-by: David Munechika <david@cypress.io> commit ad50062 Author: Johannes Ewald <mail@johannesewald.de> Date: Thu Feb 17 19:21:08 2022 +0100 fix: Wait for child process to be ready (#19792) Co-authored-by: Zach Bloomquist <github@chary.us> Co-authored-by: Zach Bloomquist <git@chary.us> commit 0d3e645 Author: Marco Lauinger <marco.lauinger@gmail.com> Date: Thu Feb 17 19:20:33 2022 +0100 fix: treat form-data bodies as binary (#20144) commit 42b0fce Author: Zach Bloomquist <git@chary.us> Date: Wed Feb 16 13:53:04 2022 -0500 test: replace cypress-test-example-repos coverage + remove bump (#20186) commit e55974c Merge: f84bac5 ce956de Author: Blue F <blue@cypress.io> Date: Wed Feb 16 09:47:06 2022 -0800 Merge pull request #20079 from cypress-io/issue-19403-perf-reporter-changes chore: Performance reporter changes commit f84bac5 Author: Ali Kireçligöl <alikirecligol@gmail.com> Date: Wed Feb 16 20:22:57 2022 +0300 fix(driver): update wrapErr to ignore number and boolean values (#20172) Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com> commit 1e6f51a Author: BlueWinds <blue@everblue.info> Date: Tue Feb 15 10:18:49 2022 -0800 release 9.5.0 [skip ci] commit 507b96f Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue Feb 15 06:35:08 2022 -0600 chore: Update Chrome (stable) to 98.0.4758.102 (#20192) Co-authored-by: cypress-bot[bot] <2f0651858c6e38e0+cypress-bot[bot]@users.noreply.github.com> commit e8d7120 Merge: dff45ca bb26e98 Author: BlueWinds <blue@everblue.info> Date: Mon Feb 14 14:15:27 2022 -0800 Merge remote-tracking branch 'origin/master' into develop commit dff45ca Merge: a3f0d63 2bad703 Author: Blue F <blue@cypress.io> Date: Mon Feb 14 12:59:52 2022 -0800 Merge pull request #20142 from cypress-io/9b967e06f5-master-into-develop chore: merge master (9b967e0) into develop commit 2bad703 Merge: 9b967e0 a3f0d63 Author: Blue F <blue@cypress.io> Date: Mon Feb 14 12:26:14 2022 -0800 Merge branch 'develop' into 9b967e0-master-into-develop commit ce956de Author: BlueWinds <blue@everblue.info> Date: Mon Feb 14 08:21:15 2022 -0800 Add span names, merge develop commit ac1faf4 Merge: 961e764 2c88f0c Author: BlueWinds <blue@everblue.info> Date: Mon Feb 14 08:19:17 2022 -0800 Merge remote-tracking branch 'origin/develop' into issue-19403-perf-reporter-changes commit 961e764 Author: BlueWinds <blue@everblue.info> Date: Thu Feb 10 14:21:44 2022 -0800 Move sending root event to own script commit bb26e98 Author: semantic-release-bot <semantic-release-bot@martynus.net> Date: Thu Feb 10 15:16:39 2022 -0500 chore: release @cypress/vue-v3.1.1 [skip ci] commit 6a96ca5 Author: semantic-release-bot <semantic-release-bot@martynus.net> Date: Thu Feb 10 15:16:23 2022 -0500 chore: release @cypress/react-v5.12.3 [skip ci] commit 9b967e0 Author: Lachlan Miller <lachlan.miller.1990@outlook.com> Date: Thu Feb 10 16:26:20 2022 +1000 fix: set correct default when using react-scripts plugin (#20141) commit e709184 Merge: e0bf811 97e6c14 Author: Barthélémy Ledoux <bart@cypress.io> Date: Wed Feb 9 20:03:05 2022 -0600 Merge pull request #20132 from cypress-io/elevatebart/trigger-vue-release commit 97e6c14 Author: ElevateBart <ledouxb@gmail.com> Date: Wed Feb 9 17:52:38 2022 -0600 fix: create a dummy commit to trigger release commit fa0b68a Author: BlueWinds <blue@everblue.info> Date: Tue Feb 8 14:50:04 2022 -0800 Fix path commit f7c46fc Author: BlueWinds <blue@everblue.info> Date: Tue Feb 8 14:35:00 2022 -0800 Refactor async data into more convenient helper commit 57decf4 Author: BlueWinds <blue@everblue.info> Date: Tue Feb 8 13:21:57 2022 -0800 Include honeycomb key so we can send root event commit 3cf4a2b Author: BlueWinds <blue@everblue.info> Date: Tue Feb 8 12:55:50 2022 -0800 Reduce event duplication commit 9d433c7 Author: BlueWinds <blue@everblue.info> Date: Tue Feb 8 12:50:24 2022 -0800 Send root honeycomb event even if node_modules cache already exists commit 705262f Author: BlueWinds <blue@everblue.info> Date: Tue Feb 8 10:48:17 2022 -0800 Fix commit b9c0d5e Author: BlueWinds <blue@everblue.info> Date: Tue Feb 8 10:37:52 2022 -0800 fix indentation commit f694bd2 Author: BlueWinds <blue@everblue.info> Date: Tue Feb 8 10:33:15 2022 -0800 Another attempt to wrap all events into one trace commit 5e96b5c Author: BlueWinds <blue@everblue.info> Date: Tue Feb 8 09:52:17 2022 -0800 Attepmt to associate all circleci jobs for a given build into one trace commit 980b951 Author: BlueWinds <blue@everblue.info> Date: Tue Feb 8 09:34:59 2022 -0800 Tweak honeycomb events further commit b8f1b4f Merge: 0d834bc 52ed6ed Author: BlueWinds <blue@everblue.info> Date: Tue Feb 8 09:12:20 2022 -0800 Merge remote-tracking branch 'origin/develop' into issue-19403-perf-reporter-changes commit 0d834bc Author: BlueWinds <blue@everblue.info> Date: Mon Feb 7 12:25:35 2022 -0800 Add data to all events sent, not just top level commit ac5bfd3 Author: BlueWinds <blue@everblue.info> Date: Mon Feb 7 11:37:55 2022 -0800 Use honeycomb tracing commit 304d8bd Author: BlueWinds <blue@everblue.info> Date: Mon Feb 7 10:22:04 2022 -0800 Make performance-reporter for system tests use get-next-version script

cypress-bot · 2022-02-28T23:44:45Z

Released in 9.5.1.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v9.5.1, please open a new issue.

fix: Wait for child process to be ready

c8c7a68

Merge branch 'develop' into wait-for-child-process

d47c72e

jhnns marked this pull request as ready for review January 21, 2022 09:01

jhnns requested a review from a team as a code owner January 21, 2022 09:01

jhnns requested review from jennifer-shehane and removed request for a team January 21, 2022 09:01

tbiethman requested review from tbiethman and flotwig and removed request for jennifer-shehane January 26, 2022 16:39

flotwig removed the request for review from tbiethman January 27, 2022 16:30

jhnns added 2 commits January 28, 2022 00:35

Fix unit test

43dc5b7

Merge remote-tracking branch 'upstream/develop' into wait-for-child-p…

3d6e5bf

…rocess

Merge branch 'develop' into wait-for-child-process

4a976d7

flotwig mentioned this pull request Feb 4, 2022

system-tests jobs are hanging for external contributors #20057

Closed

flotwig force-pushed the wait-for-child-process branch 2 times, most recently from 04a151e to 73357dd Compare February 7, 2022 18:01

chore(contributing): fix network_error_handling_spec for external con…

3cfc322

…tributors

flotwig force-pushed the wait-for-child-process branch from 73357dd to 3cfc322 Compare February 7, 2022 18:17

flotwig approved these changes Feb 7, 2022

View reviewed changes

Merge branch 'develop' into wait-for-child-process

0f14ba7

ryanthemanuel requested review from ryanthemanuel and tbiethman February 8, 2022 16:52

tbiethman previously approved these changes Feb 8, 2022

View reviewed changes

flotwig suggested changes Feb 8, 2022

View reviewed changes

Test for async ready event

0fc9928

jhnns dismissed tbiethman’s stale review via 0fc9928 February 15, 2022 21:47

flotwig requested review from flotwig and removed request for ryanthemanuel February 16, 2022 16:15

tbiethman approved these changes Feb 17, 2022

View reviewed changes

flotwig approved these changes Feb 17, 2022

View reviewed changes

flotwig merged commit ad50062 into cypress-io:develop Feb 17, 2022

jhnns deleted the wait-for-child-process branch February 17, 2022 18:38

cypress-bot bot locked as resolved and limited conversation to collaborators Feb 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Wait for child process to be ready #19792

fix: Wait for child process to be ready #19792

jhnns commented Jan 20, 2022 •

edited by flotwig

cypress-bot bot commented Jan 20, 2022

CLAassistant commented Jan 20, 2022 •

edited

jhnns commented Jan 21, 2022

flotwig commented Jan 27, 2022

jhnns commented Jan 27, 2022

jhnns commented Jan 28, 2022

flotwig commented Feb 2, 2022 •

edited

flotwig Feb 8, 2022

jhnns Feb 14, 2022

jhnns Feb 15, 2022

cypress-bot bot commented Feb 28, 2022

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

fix: Wait for child process to be ready #19792

fix: Wait for child process to be ready #19792

Conversation

jhnns commented Jan 20, 2022 • edited by flotwig

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented Jan 20, 2022

CLAassistant commented Jan 20, 2022 • edited

jhnns commented Jan 21, 2022

flotwig commented Jan 27, 2022

jhnns commented Jan 27, 2022

jhnns commented Jan 28, 2022

flotwig commented Feb 2, 2022 • edited

flotwig Feb 8, 2022

Choose a reason for hiding this comment

jhnns Feb 14, 2022

Choose a reason for hiding this comment

jhnns Feb 15, 2022

Choose a reason for hiding this comment

cypress-bot bot commented Feb 28, 2022

jhnns commented Jan 20, 2022 •

edited by flotwig

CLAassistant commented Jan 20, 2022 •

edited

flotwig commented Feb 2, 2022 •

edited