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

Rewrite chai-jQuery, and improve its unexpected behavior #605

Closed
brian-mann opened this Issue Aug 26, 2017 · 1 comment

Comments

2 participants
@brian-mann
Member

brian-mann commented Aug 26, 2017

We currently use https://github.com/chaijs/chai-jquery for many assertions, but the library has some considerable problems which are difficult to overcome without a complete rewrite.

Internally not only is our code more complex, but this library does some pretty unexpected things.

  1. It changes the subject in unexpected ways
  2. Non jquery objects "fall through" or provide terrible errors
  3. It uses hard coded $ instanceof checks
  4. It adds more than 1 assertion for things which is visually confusing

(1) causes all kinds of problems with new users to Cypress. We see people getting tripped up and accidentally changing the subject, and then due to the poor error messaging have NO idea what went wrong.

(2) + (3) + (4) are easy to fix.

Current Behavior

A typical problem:

it "changes subject in unexpected ways", function(){
  cy
    .visit("https://example.cypress.io/")
    .get("a:first")
    .should("have.prop", "href", "https://example.cypress.io/")
    .and("have.class", "navbar-brand")
})

screen shot 2017-08-25 at 9 24 28 pm

This is because these assertions change the subject:

// here it makes sense to change the subject for chainability
cy.get("a").should("have.prop", "href")
cy.get("a").should("have.prop", "href").and("match", /google/)

// here it makes no sense to change the subject (?????)
// the subject is now the string 'https://google.com'
cy.get("a").should("have.prop", "href", "https://google.com")

Proposed Behavior

  1. Change the subject when using prop, attr, css by itself
  2. Do not change the subject when providing an equality matcher to the 3 listed above
  3. Update many of the error messages to make them clearer
  4. Remove "double internal assertions" even when the user only wrote a single assertion
  5. Gracefully handle whether assertions on either DOM objects or jquery objects
// this continues to work as before
cy.get("a").should("have.prop", "href")
cy.get("a").should("have.prop", "href").and("match", /google/)

// this no longer changes the subject to the string, and instead returns
// the original <a> for further chainability
cy.get("a").should("have.prop", "href", "https://google.com").and("have.class", "link")
@brian-mann

This comment has been minimized.

Member

brian-mann commented Aug 26, 2017

The code for this is done, but this has yet to be released. We'll update the issue and reference the changelog when it's released.

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