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

check for undefined values on setPluginResolvedOn function #7960

Merged
merged 8 commits into from Jul 16, 2020

Conversation

jmsansan
Copy link
Contributor

@jmsansan jmsansan commented Jul 13, 2020

User facing changelog

Fixed a bug where recursion function setPluginResolvedOn could receive undefined on resolvedObj parameter.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 13, 2020

Thanks for taking the time to open a PR!

@CLAassistant
Copy link

CLAassistant commented Jul 13, 2020

CLA assistant check
All committers have signed the CLA.

@jmsansan jmsansan changed the title check for undefined values on plugin resolved check for undefined values on setPluginResolvedOn function Jul 13, 2020
Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Could you please sign our CLA?

Additionally, we will need some tests written to verify this new behavior.

There is a Contributing Guide that covers how to contribute and get Cypress running locally here: https://github.com/cypress-io/cypress/blob/develop/.github/CONTRIBUTING.md

Let us know if you have trouble getting the tests running locally.

@jmsansan
Copy link
Contributor Author

Thanks for the contribution! Could you please sign our CLA?

Additionally, we will need some tests written to verify this new behavior.

There is a Contributing Guide that covers how to contribute and get Cypress running locally here: https://github.com/cypress-io/cypress/blob/develop/.github/CONTRIBUTING.md

Let us know if you have trouble getting the tests running locally.

I will do my best since is my first contribution to an open source project 👍
Thanks Jennifer.

@jennifer-shehane
Copy link
Member

@jmsansan Thanks for signing the CLA, unfortunately the author of all of the commits does not match the email address on your GitHub account. There are 2 commits, one made by Jose Sanchez the other made by jmsansan. https://github.com/cypress-io/cypress/pull/7960/commits

Our CLA bot gets confused when they don't match. Here are some solutions to fixing when commits are linked to another wrong user: https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/

@jmsansan
Copy link
Contributor Author

@jennifer-shehane
I could figure out what was the problem with CLA so I solved it, PR should be fine now.
Thanks for your support Jennifer 👍

@jennifer-shehane jennifer-shehane self-requested a review July 15, 2020 10:57
Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

I manually tested the given examples in the original issue. They no longer error. Additionally, they are all available properties on the Cypress.config() as expected.

The configuration in the Settings also displays the objects and arrays correctly appended to the config.

Screen Shot 2020-07-16 at 12 00 34 PM

Looks great. Thanks for the contribution! 💯

packages/server/test/unit/config_spec.js Show resolved Hide resolved
@bahmutov bahmutov merged commit 582fb15 into cypress-io:develop Jul 16, 2020
@mccataldo
Copy link

mccataldo commented Jul 23, 2020

@jennifer-shehane should this PR also resolve #7002 and #1736

@jennifer-shehane
Copy link
Member

@mccataldo Yes! It does appear to have also fixed those issues. Nice catch 👍 I'll update those other issues to reflect that they've been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Plugin] Cannot set property of undefined Unable to use deeply nested config or env in plugins
5 participants