Empty returned specPattern doesn't yield error if specPattern was originally an array

[badeball]

Current behavior

Setting specPattern to an empty array [] within setupNodeEvents() { .. } makes Cypress correctly yield a no spec files error. However, if specPattern was originally configured as an array, this is no longer the case.

Desired behavior

For Cypress to yield the above-mentioned error no matter if specPattern was originally specified as a string or an array of strings.

Test code to reproduce

https://github.com/badeball/reproducible-issues/tree/master/cypress/empty-spec-pattern

Cypress Version

12.15.0

Node version

v18.8.0

Operating System

Arch Linux

Debug Logs

No response

Other

No response

[badeball]

For reference, this might happen in a plugin trying to filter specs.

[mike-plummer]

Hi @badeball , thanks for the submission and the reproduction! I will forward this ticket to the appropriate team. They will soon evaluate the priority of this ticket and consider their capacity to pick it up. Please note that this does not guarantee that this issue will be resolved. The ticket will indicate status changes during evaluation, so we ask that you please refrain from asking for updates. Thanks!

For anyone considering picking this ticket up I believe the offending logic is in this area, and we have already dealt with a similar issue in the past based on the comments immediately below. The defaultsDeep function from lodash being used here treats arrays differently than other field types (merging instead of replacing).

[NiharPhansalkar]

Hello, can I try to work on this issue? Also, I am new to making contributions, so can I also get some help?

And if I can work on it, I would just like to ask, how exactly am I supposed to check the output in the reproducible code written by @badeball? As in, I would like to see the behaviour difference being mentioned? (I have cloned the repo but am now confused)

[mike-plummer]

Hey @NiharPhansalkar , thanks for offering to help! A good place to start is to review our Contributing Guide which outlines the process for fixing, testing, and committing a change to Cypress.

For this particular issue, you can view the project's effective configuration by:

1. Launch cypress in open mode: npx cypress open
2. Select the "Settings" icon on the left
3. Expand "Project Settings"
4. Scroll down until you find specPattern

In the reproduction case, specPattern is getting initialized to one value within cypress.config.js then overridden by another within the setupNodeEvents function in that file. This particular edge case is triggered when the original value is an array and the overriding value is an empty array - the empty array does not end up replacing the original value. We would want to ensure this is handled as expected (and verify there aren't any other lurking edge cases like replacing with an empty/populated string, replacing with a non-empty array replaces rather than merges, etc), and write some tests to make sure it doesn't break in the future

[NiharPhansalkar]

@mike-plummer Thank you Mike! Appreciate the help! I am having some build issues with cypress, the moment it gets fixed, I will get to work on this, and let you know if I get stuck anywhere else!

[NiharPhansalkar]

@mike-plummer Hello, so for the diffs object, do we want to stop the combination of both arrays and objects from the cfg object? (And for all arrays and objects in diffs?)
And whatever arrays/objects might be specified within setupNodeEvents (i.e. the diffs object) we want to maintain them without any combination from the cfg correct?