fix: Fix Issue 27103. Retain original array after defaultsDeep #27240

NiharPhansalkar · 2023-07-08T06:58:05Z

Closes issue-27103

Additional details

What is affected by this change?
Previously the defaultsDeep function was merging/combining arrays. This PR will fix that issue and restore any arrays modified by the setupNodeEvents function.

Steps to test

To test, copy the cypress.config.js from the reproduction section of the issue and configure for e2e testing. Then run cypress using yarn cypress open --dev --config-file cypress.config.js. After setting any key within the config object to any array value (within the setupNodeEvents function), it should reflect as is in the final config.

How has the user experience changed?

User will get the exact config they supply in the setupNodeEvents function.

PR Tasks

[doubt] Have tests been added/updated?
[na] Has a PR for user-facing changes been opened in cypress-documentation?
[na] Have API changes been updated in the type definitions?

…-combination-fix

CLAassistant · 2023-07-08T06:58:10Z

All committers have signed the CLA.

cypress-app-bot · 2023-07-08T06:58:29Z

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.

cypress · 2023-07-08T07:30:02Z

1 failed and 29 flaky tests on run #48661 ↗︎

1	27936	1349	0	29

Details:

Merge branch 'cypress-io:develop' into issue-27103-defaultsDeep-array-combinatio...
Project: cypress	Commit: `b7e30a42d0`
Status: Failed	Duration: 20:08 💡
Started: Jul 8, 2023 10:48 AM	Ended: Jul 8, 2023 11:09 AM

cypress/e2e/e2e/origin/user_agent_override.cy.ts • 1 failed test • 5x-driver-electron

View Output Video

Test		Artifacts
user agent override > persists modified user agent after cy.go		`Output` `Video`

runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
... > displays a list of recorded runs if a run has been recorded		`Output` `Screenshots` `Video`

specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs		`Output` `Screenshots` `Video`

cypress-origin-communicator.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution		`Output` `Screenshots` `Video`

commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test		Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse		`Output`

commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-electron

View Output Video

Test		Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse		`Output` `Video`

The first 5 flaky specs are shown, see all 17 specs in Cypress Cloud.

_{This comment has been generated by cypress-bot as a result of this
project's GitHub integration settings.}

lmiller1990

Great job. I've recommended one refactor, and added a test to ensure this won't regress again. 47c12f1

lmiller1990 · 2023-07-11T02:11:51Z

packages/config/src/project/index.ts

+  for (const key in diffsClone) {
+    if (Array.isArray(diffsClone[key])) {
+      merged[key] = _.cloneDeep(diffsClone[key])
+    }
+  }
+


Suggested change

for (const key in diffsClone) {

if (Array.isArray(diffsClone[key])) {

merged[key] = _.cloneDeep(diffsClone[key])

}

}

for (const [key, value] of Object.entries(diffsClone)) {

if (Array.isArray(value)) {

merged[key] = _.cloneDeep(value)

}

}

I think this can be a bit more concise. I did not try this, but it should work.

I've also added a test. 47c12f1e4e712f62b0cec582b7dfda98f3177583

You could use this to see if my refactor works. cd packages/app then yarn dev. Select the spec (or use it.only to only run the one test I added).

Hello @lmiller1990. I made the suggested changes and locally tried the original reproduction repo, and it works perfectly fine for it. However, I was not able to run the tests locally. After selecting the spec, I was met with a lot of errors. I modified the it() block under which you had added the test to it.only(), however it gave me the following error.

cy.task('__internal_withCtx') timed out after waiting 60000ms. Because this error occurred during a before each hook we are skipping the remaining tests in the current suite: specChange subscription

Please guide me on how I can actually try and run the tests on my local device?

…n-fix

lmiller1990 · 2023-07-12T00:21:25Z

Let me run CI and see how it goes.

lmiller1990 · 2023-07-12T00:51:51Z

This seems to massively break things. https://app.circleci.com/pipelines/github/cypress-io/cypress?branch=pull%2F27240

What operating system are you on @NiharPhansalkar? Does yarn complete when you first clone the repo? Note you must use Chrome for the packages/app tests.

Are you able to run other tests - try yarn cypress:open in packages/app and choose Component Testing -- do those run?

NiharPhansalkar · 2023-07-12T01:20:54Z

@lmiller1990 Hello, I am running this in a VM running Fedora 38. Yes yarn does complete (however, one catch is I had to disable the NX daemon in order for it to work properly) and yes I ran it on Google Chrome. However, when I select component testing, it says the following

Cannot convert undefined or null to object

TypeError: Cannot convert undefined or null to object
    at Function.entries ()
    at updateWithPluginValues (/home/vm_work/cypress/packages/config/src/project/index.ts:113:37)
    at ProjectConfigManager.handleSetupTestingTypeReply (/home/vm_work/cypress/packages/data-context/src/data/ProjectConfigManager.ts:333:73)
    at ProjectConfigManager.setupNodeEvents (/home/vm_work/cypress/packages/data-context/src/data/ProjectConfigManager.ts:267:7)

NiharPhansalkar · 2023-07-12T01:22:26Z

At least here on GitHub where it shows the checks, it was previously passing most tests, failing about 7. Now all of a sudden it is failing 25 tests. I hope this is only related to the change I made? And has nothing to do with me updating the branch?

lmiller1990 · 2023-07-12T22:10:50Z

@NiharPhansalkar I think we need a default value: 1deb871

It was trying to do Object.entries(undefined). Let's see how this goes.

NiharPhansalkar · 2023-07-13T02:03:44Z

@lmiller1990 Hello Lachlan, the cy.startAppServer() keeps on giving me a timeout error when I try to run the e2e tests. I even tried commenting out some code, but of course, other things start breaking.

lmiller1990 · 2023-07-14T01:27:55Z

I'll try and get this one merged up.

NiharPhansalkar · 2023-07-14T01:49:37Z

Yes, but say for future contributions for cypress, I may have to test other parts? Where can I actually understand the reason for such issues occurring and try to fix them?

lmiller1990 · 2023-07-14T02:10:53Z

Right, I understand your question.

VM running Fedora 38.

I haven't run on Fedora or under a VM before, but the first thing I usually do to debug is run the build with DEBUG=cypress:* yarn dev and see what comes up in the logs. I have run the development scripts on Ubuntu, MacOS and windows before. If you'd like to give that a try and see if there's anything useful in the debug logs.

NiharPhansalkar · 2023-07-14T02:12:42Z

Sure! Thank you.

NiharPhansalkar · 2023-07-16T03:59:56Z

@lmiller1990 Hello Lachlan. I did run it with DEBUG=* yarn dev and there were quite a few errors. Can you tell me if you too have received such errors?

cypress:ts registering ts-node on directory undefined
nexusTypegen: cypress:ts registering ts-node on directory undefined'
cypress:server environment error Cannot read properties of undefined (reading 'commandLine')
cypress:packherd:error Error: Cannot find module 'pnpapi' (Lot of packherd errors)
NexusSlowGuard: Taking more than 100ms to execute ["currentProject","projectId"] for query HeaderBar_HeaderBarQuery (total time 161ms)

lmiller1990 · 2023-07-17T00:19:51Z

Here is the entirely of what I see. I am not sure I can help with running of Fedora specifically; I don't have the time / bandwidth to try Fedora development right now. If you are able to try another distro (eg Ubuntu) where it's known to run, I can possibly help.

In the meantime, I'll get this PR ready to go.

logs.txt

lmiller1990 · 2023-07-17T05:46:49Z

Close in favor of #27312

* modifying object after array combination to hold older values * regression test * made suggested changes * config defaults * log * logs * log * log again * update log * changelog * updates changelog * fix typo and move changelog entry and fix link --------- Co-authored-by: Nihar Phansalkar <phansalkarnihar@gmail.com> Co-authored-by: Jennifer Shehane <jennifer@cypress.io> Co-authored-by: Cacie Prins <cacieprins@users.noreply.github.com> Co-authored-by: Cacie Prins <cacie@cypress.io> Co-authored-by: AtofStryker <bglesias@gmail.com>

NiharPhansalkar added 2 commits July 8, 2023 11:13

modifying object after array combination to hold older values

4fe3754

Merge branch 'cypress-io:develop' into issue-27103-defaultsDeep-array…

b7e30a4

…-combination-fix

NiharPhansalkar changed the title ~~fix Issue 27103~~ fix: Fix Issue 27103. Retain original array after defaultsDeep Jul 8, 2023

NiharPhansalkar marked this pull request as draft July 8, 2023 07:05

NiharPhansalkar marked this pull request as ready for review July 8, 2023 10:37

nagash77 assigned lmiller1990 Jul 10, 2023

regression test

47c12f1

lmiller1990 reviewed Jul 11, 2023

View reviewed changes

NiharPhansalkar added 2 commits July 11, 2023 21:33

made suggested changes

d257aed

Merge branch 'develop' into issue-27103-defaultsDeep-array-combinatio…

587dea3

…n-fix

config defaults

1deb871

log

12b9b64

lmiller1990 added 2 commits July 14, 2023 12:12

Merge remote-tracking branch 'origin/develop' into Nihar-fixx

3aefe95

logs

a434b55

lmiller1990 added 2 commits July 14, 2023 12:27

log

97e0280

log again

8cff6d9

lmiller1990 mentioned this pull request Jul 17, 2023

fix: retain original array after defaultsDeep #27240 #27312

Merged

lmiller1990 closed this Jul 17, 2023

cypress-bot bot locked as resolved and limited conversation to collaborators Apr 2, 2024

cypress-io deleted a comment from cypress-bot bot Apr 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Fix Issue 27103. Retain original array after defaultsDeep #27240

fix: Fix Issue 27103. Retain original array after defaultsDeep #27240

NiharPhansalkar commented Jul 8, 2023

CLAassistant commented Jul 8, 2023 •

edited

Loading

cypress-app-bot commented Jul 8, 2023

cypress bot commented Jul 8, 2023 •

edited

Loading

lmiller1990 left a comment

lmiller1990 Jul 11, 2023

NiharPhansalkar Jul 11, 2023 •

edited

Loading

lmiller1990 commented Jul 12, 2023

lmiller1990 commented Jul 12, 2023

NiharPhansalkar commented Jul 12, 2023 •

edited

Loading

NiharPhansalkar commented Jul 12, 2023 •

edited

Loading

lmiller1990 commented Jul 12, 2023

NiharPhansalkar commented Jul 13, 2023

lmiller1990 commented Jul 14, 2023

NiharPhansalkar commented Jul 14, 2023

lmiller1990 commented Jul 14, 2023

NiharPhansalkar commented Jul 14, 2023

NiharPhansalkar commented Jul 16, 2023

lmiller1990 commented Jul 17, 2023

lmiller1990 commented Jul 17, 2023

fix: Fix Issue 27103. Retain original array after defaultsDeep #27240

fix: Fix Issue 27103. Retain original array after defaultsDeep #27240

Conversation

NiharPhansalkar commented Jul 8, 2023

Additional details

Steps to test

How has the user experience changed?

PR Tasks

CLAassistant commented Jul 8, 2023 • edited Loading

cypress-app-bot commented Jul 8, 2023

cypress bot commented Jul 8, 2023 • edited Loading

1 failed and 29 flaky tests on run #48661 ↗︎

lmiller1990 left a comment

Choose a reason for hiding this comment

lmiller1990 Jul 11, 2023

Choose a reason for hiding this comment

NiharPhansalkar Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

lmiller1990 commented Jul 12, 2023

lmiller1990 commented Jul 12, 2023

NiharPhansalkar commented Jul 12, 2023 • edited Loading

NiharPhansalkar commented Jul 12, 2023 • edited Loading

lmiller1990 commented Jul 12, 2023

NiharPhansalkar commented Jul 13, 2023

lmiller1990 commented Jul 14, 2023

NiharPhansalkar commented Jul 14, 2023

lmiller1990 commented Jul 14, 2023

NiharPhansalkar commented Jul 14, 2023

NiharPhansalkar commented Jul 16, 2023

lmiller1990 commented Jul 17, 2023

lmiller1990 commented Jul 17, 2023

CLAassistant commented Jul 8, 2023 •

edited

Loading

cypress bot commented Jul 8, 2023 •

edited

Loading

NiharPhansalkar Jul 11, 2023 •

edited

Loading

NiharPhansalkar commented Jul 12, 2023 •

edited

Loading

NiharPhansalkar commented Jul 12, 2023 •

edited

Loading