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

Add assertions to '.select()' doc #216

Closed
jennifer-shehane opened this issue Nov 3, 2017 · 12 comments
Closed

Add assertions to '.select()' doc #216

jennifer-shehane opened this issue Nov 3, 2017 · 12 comments

Comments

@jennifer-shehane
Copy link
Member

See cypress-io/cypress-example-kitchensink#23

@jennifer-shehane jennifer-shehane changed the title Add assertions to 'select' docs Add assertions to '.select()' doc Nov 3, 2017
@ootsuka
Copy link
Contributor

ootsuka commented Feb 28, 2018

Hi Jennifer, I am a newbie to open source and wondering if I can work on this issue? If yes where's a good place to start?

@jennifer-shehane
Copy link
Member Author

jennifer-shehane commented Feb 28, 2018

Hey @ootsuka, the document that needs to be updated can be found here: https://github.com/cypress-io/cypress-documentation/blob/develop/source/api/commands/select.md which is the source for this page: https://docs.cypress.io/api/commands/select.html

I think we just wanted the current Examples updated to include assertions like instead of this:

cy.get('select').select('apples')

It would show this:

cy.get('select').select('apples')
  .should('have.value', '456')

@ootsuka
Copy link
Contributor

ootsuka commented Mar 1, 2018

Thank you. I am trying to include the assertions but I am having trouble with

cy.get('.action-select-multiple')
        .select(['apples', 'oranges', 'bananas'])

Do you mind share how you would do the assertion for this one?

@jennifer-shehane
Copy link
Member Author

@ootsuka This one definitely is trickier!

The value of a select with multiple options selected will be an array of the option's values.

So, we'll need to pull out the value first using .invoke() so that we can make a better comparison with just the array.

We'll also need to use the deep.equal assertion since comparing arrays equality is tricky (see https://github.com/chaijs/deep-eql for more on how that works).

cy.get('.action-select-multiple')
  .select(['apples', 'oranges', 'bananas']).invoke('val')
  .should('deep.equal', ['456', '457', '458'])

@ootsuka
Copy link
Contributor

ootsuka commented Mar 1, 2018

Yes. I was trying to go for the deep equal but could not get the value cause I did not know the invoke(‘val’).
Thanks a ton!

@brian-mann
Copy link
Member

The question you have to ask yourself is why add an assertion after doing a .select([]). If it's because you have application logic that may modify the value - then this is a good test.

However, if you aren't modifying anything, then you don't need an assertion because you can assume Cypress is doing the right thing. If you tell Cypress to select something that's an invalid value - it will already throw an error. So if the .select() resolves, then you know it's correct.

@jennifer-shehane
Copy link
Member Author

@brian-mann This isn't really a terribly useful real use case, it's more of a good place to outline how to assert on select values, because people were having a hard time writing the assertions.

Although, I generally test things like this because of how my onChange logic is written. Sometimes you can literally type or select, but the value is still borked.

@brian-mann
Copy link
Member

Right if you are modifying the value then it makes sense to add an assertion that the right stuff was selected.

I'm pretty sure you can just do .select(['foo', 'bar', 'baz']).should('have.value', ['foo', 'bar', 'baz']) without needing to yield the value and use deep.eq but haven't tried it.

@jennifer-shehane
Copy link
Member Author

No, you can't just write .select(['foo', 'bar', 'baz']).should('have.value', ['foo', 'bar', 'baz']), this is not using the proper equality operator for comparison.

@brian-mann
Copy link
Member

We control the have.value assertion right here: https://github.com/cypress-io/cypress/blob/develop/packages/driver/src/cypress/chai_jquery.coffee#L130

Instead of the expression: actual is value we could instead use _.isEqual(actual, value).

Doing it that way ends up being the same thing as deep.eq since array's and objects are deeply compared instead of by reference. That would enable you to use arrays and objects for have.value

@ootsuka
Copy link
Contributor

ootsuka commented Mar 1, 2018

@brian-mann I think that's a good idea. @jennifer-shehane do you think it's the right way to do it?

This was referenced Mar 3, 2018
@richdouglasevans
Copy link
Contributor

Can this issue be closed?

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

No branches or pull requests

4 participants