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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 11 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4fe3754
modifying object after array combination to hold older values
NiharPhansalkar Jul 8, 2023
b7e30a4
Merge branch 'cypress-io:develop' into issue-27103-defaultsDeep-array…
NiharPhansalkar Jul 8, 2023
47c12f1
regression test
lmiller1990 Jul 11, 2023
d257aed
made suggested changes
NiharPhansalkar Jul 11, 2023
587dea3
Merge branch 'develop' into issue-27103-defaultsDeep-array-combinatio…
NiharPhansalkar Jul 11, 2023
1deb871
config defaults
lmiller1990 Jul 12, 2023
12b9b64
log
lmiller1990 Jul 14, 2023
3aefe95
Merge remote-tracking branch 'origin/develop' into Nihar-fixx
lmiller1990 Jul 14, 2023
a434b55
logs
lmiller1990 Jul 14, 2023
97e0280
log
lmiller1990 Jul 14, 2023
8cff6d9
log again
lmiller1990 Jul 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions packages/app/cypress/e2e/subscriptions/specChange-subscription.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,40 @@ e2e: {

cy.get('[data-cy="file-match-indicator"]', { timeout: 7500 })
.should('contain', '3 matches')

// Regession for https://github.com/cypress-io/cypress/issues/27103
cy.withCtx(async (ctx) => {
await ctx.actions.file.writeFileInProject('cypress.config.js',
`
module.exports = {
projectId: 'abc123',
experimentalInteractiveRunEvents: true,
component: {
specPattern: 'src/**/*.{spec,cy}.{js,jsx,ts,tsx}',
supportFile: false,
devServer: {
framework: 'react',
bundler: 'webpack',
}
},
e2e: {
specPattern: ['cypress/e2e/**/dom-cont*.spec.{js,ts}'],
supportFile: false,
setupNodeEvents(on, config) {
/**
* This should make Cypress yield a "no specs found" error.
*
* This stops being the case if 'specPattern' is an array.
*/
config.specPattern = [];
return config;
},
},
}`)
})

cy.get('[data-cy="create-spec-page-title"]')
.should('contain', defaultMessages.createSpec.page.customPatternNoSpecs.title)
})
})
})
8 changes: 8 additions & 0 deletions packages/config/src/project/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,17 @@ export function updateWithPluginValues (cfg: FullConfig, modifiedConfig: any, te
debug('resolved config object %o', cfg.resolved)
}

const diffsClone = _.cloneDeep(diffs)

// merge cfg into overrides
const merged = _.defaultsDeep(diffs, cfg)

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@NiharPhansalkar NiharPhansalkar Jul 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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?

debug('merged config object %o', merged)

// the above _.defaultsDeep combines arrays,
Expand Down
Loading