cypress-io / cypress

Fast, easy and reliable testing for anything that runs in a browser.
https://cypress.io
MIT License
46.97k stars 3.18k forks source link

How to deal with dynamic class names à la CSS Modules or Styled Components? #1212

Closed chapati23 closed 6 years ago

chapati23 commented 6 years ago

Any pointers on how to deal with dynamic class names produced by e.g. CSS Modules or styled-components?

The easy way out would be to just plaster my markup with id="e2e-blabla" but that doesn't feel particularly nice to be honest.

Another way could be to look for partial matches in class names. For example the CSS Modules class .form produces the actual class name .styles__form--3gcRC in the DOM. One could now use a CSS attr selector such as [class*=form] which would work in many case but can lead to name conflicts and kinda removed all the benefits of having locally scoped and unique class names.


Current behavior:

There's no documented way (at least i couldn't find one in neither issues nor docs) to make cypress play well with dynamic class names generated by commonly used tools such as CSS Modules or styled-components

Desired behavior:

There's a well documented way of having cypress be able to look for dynamic class names in the DOM

How to reproduce:

Try to make cypress work with any React app that uses either CSS Modules or styled-components

andreiashu commented 6 years ago

Interested as well in this subject. At the moment we're using a mix of unique class names (ie. "payment-button") with [class*=form] type selectors.

bahmutov commented 6 years ago

how would you prefer doing this? I used [class*=form] and [class^=form], anything beyond this would be framework-specific

jennifer-shehane commented 6 years ago

Wanted to say, @willklein mentioned this in his talk, basically he recommends adding classes purely for testing environment and then stripping them out for production. Perhaps he can elaborate more on how they did that for their Cypress tests.

Link to video here: https://youtu.be/rICGz5qrYJU?t=1h42m28s

chapati23 commented 6 years ago

yeah, one could always come up with a conventional prefix such as className="cypress-email-input" or className="e2e-email-input" which would be relatively easy to instrument away. but then i couldn't run e2e tests against staging or prod environments. And it's also an additional step that has to be remembered while coding. The perfect solution would be to just use the same way of addressing elements that I'm already using in my React components.

Forgive me the question, didn't have time to read too deep into the underlying implementation of cypress yet, but I'd assume you're using webpack?

If that's the case, you could offer people to provide their own loaders. In that case I could just point cypress to the same webpack loaders i'm already using for my dev server (e.g. postcss-loader).

For example, in my cypress test i could then just do the following:

/* src/components/email-form/styles.css */

/*
 * css modules will transform this class
 * into sth like this: .styles__email-input-cmXvUE
 */
.email-input {
  color: blue;
}
/* cypress/integration/email-form.spec.js */

// path is a bit ugly but could be mitigated by using NODE_PATH
import styles from '../../src/components/email-form/styles.css'
const URL = 'http://localhost:8080'

describe('Email Form', () => {
  before(() => {
    cy.visit(`${URL}/`)
  })

  it("should have an email input", () => {
    cy
      // .get("[class*=email-input)") => the current way of doing things
      // the same code i'm using to style my React components which would evaluate to the same .styles__email-input-cmXvUE
      .get(styles.emailInput) // => the new, more exact, way of doing things
      .should('have.attr', 'type', 'email')
  })
})
zth commented 6 years ago

We use the "data-testid"-convention inspired by BEM-naming, which I think works great for us. Example:

<div class="some-class" data-testid="Login__form__container">
  ...
</div>

And then we select test elements using a custom Cypress command that basically just wraps cy.get:

cy.getTestElement('Login__form__container')

// getTestElement, just the function here, but it's a Cypress command in our codebase
function getTestElement(selector) {
  return cy.get(`[data-testid="${selector}"]`);
}

The main benefit for us is that we have a clear convention for how to name test stuff that's not coupled to actual classes or similar. This means that we can refactor stuff and keep our tests as long as we assign the appropriate test id:s to the right elements again when refactoring.

It also allows us to write tooling around that convention (a Chrome extension that lists all available testid:s on a page, with click-to-copy-selector etc), but that type of functionality can of course be implemented regardless of what naming convention you use.

Anyway, just a thought!

willklein commented 6 years ago

Sorry, I just noticed this. @zth describes the technique I prefer: custom data attributes. Using data-* attributes is framework-agnostic and I think the most balanced approach. His reasoning is spot on, too, I like that it decouples your tests from any style-oriented classes. I still like this approach even if you have semantic class-based selectors, though I wouldn't say you need to introduce it unless you run into actual problems.

We didn't need to BEM name things, but that's also a pretty good idea. In our case, we put semantic descriptors in our form element containers along with data model specific descriptors because we had them. We had a lot of shared, pure components and with only modifying about five lines of template code, we instrumented an infinite number of fields, buttons, and links. Instrumenting other apps may require more changes, but it should still be worthwhile.

How you configure your build to remove this for production will get into framework-specific tooling. If you're using React and Babel, this is what I use:

https://github.com/oliviertassinari/babel-plugin-react-remove-properties.

data-test is the default property but it can be configured if you want something different, or to use more than one.

If there's any desire from the Cypress team, I'm happy to try writing up a guide on this if it fits in the documentation.

brian-mann commented 6 years ago

We are all for data-test or data-cy.

In fact we've talked about making this the default attribute we use for the Selector Playground - which would make it super nice to automatically find these.

krishnaxv commented 6 years ago

@brian-mann @willklein Thumbs up 👍 for data-test or data-cy approach.

Any input on this approach for a React project with Create React App (CRA) and styled-components without ejecting?

Create React App does not use .babelrc specified in the project. In webpack.config.prod.js of CRA, babelrc is set to false. I tried react-scripts@2.0.0-next.47d2d941 also, but again unable to use this approach without ejecting CRA.

Any thoughts?

willklein commented 6 years ago

@krishnaxv that seems like a clear trade off when using something like Create React App: you're giving up control/configuration to have things managed for you. I definitely try to keep from ejecting whenever possible, but if I'm starting to spin up a lot of tooling and need my own Babel plugins (I often do), I find it's time to eject.

pgroot91 commented 6 years ago

@andreiashu @bahmutov do you guys know already if there is a solution for those use cases? Maybe i missed something but didn't saw anyone mentioning your questions with a solution or any workaround or this is something on the map to be added in near future in cypress maybe?

bahmutov commented 6 years ago

We strongly recommend adding custom attributes for testing. Especially if they follow the convention, then they become even compatible with CSS Selector Playground. See these two resources:

andrewdang17 commented 6 years ago

Custom attributes seems like a good workaround for selecting elements but what about when you are testing for the class name itself? Is there a way to test that the class name just contains a word?

For example:cy.get('[data-cy=div').should('have.class', 'active') won't work because the class name would have a random hash at the end like active__3h43

kuceb commented 6 years ago

@andrewdang17 this should work: cy.get('...').invoke('attr', 'class').should('contain', 'className')

MariiaB commented 6 years ago

@Bkucera Your suggestion works perfectly in assertions, thanks. Any ideas how to deal with dynamic names in "if statements" in conditional testing? For example:

cy.get('[data-e2e="companiesFilter"]').then(($curEl) => {
    if($curEl.hasClass('togglerCollapsed')) {
      cy.get('[data-e2e="companiesFilterHeader"]').click();
    }
});

doesn't work because class name is class="togglerCollapsed___1kw8a toggler___3zlB6"

kuceb commented 6 years ago

@MariiaB sure, you can get the class names with .attr('class') and then use includes('..')

if (cy.$$('[data-e2e="companiesFilter"]').attr('class').includes('togglerCollapsed')) {
      // your code here 
    }
danielkcz commented 6 years ago

I am curious. Since custom data- attributes is a recommended way, why not to include some utility functions/commands for that? I mean it's easy to define a custom command that does it, but everyone has to find out first how to do that instead of just grab a function and use it. In my opinion, having this more build in would ease initial traction on the recommended approach. I would almost say it should be a first-class citizen having such function so people don't get burned by doing cy.get('button')

pawellesniowski commented 6 years ago

@Bkucera Thank you! I works. ;-)

const checkClassPresence = (element, className) => {
  cy
    .get(element)
    .invoke('attr', 'class')
    .should('contain', className);
};
arshmakker commented 5 years ago

Hi @Bkucera

I tried your approach

cy.get('div').invoke('attr', 'class').should('match', /MuiSelect-select-/);

but i think it would be a lot of performance hit and also it is not working :(

Also the second approach of putting custom data attributes, doesn't sound too good as it would make me add attributes just to test things.

In Jest, it is easier, as we can always import the component in the test framework and compare the snapshot with the rendered output. example:

// Link.react.test.js import React from 'react'; import Link from '../Link.react'; import renderer from 'react-test-renderer';

test('Link changes the class when hovered', () => { const component = renderer.create(

Facebook, ); let tree = component.toJSON(); expect(tree).toMatchSnapshot(); // manually trigger the callback tree.props.onMouseEnter(); // re-rendering tree = component.toJSON(); expect(tree).toMatchSnapshot(); // manually trigger the callback tree.props.onMouseLeave(); // re-rendering tree = component.toJSON(); expect(tree).toMatchSnapshot(); });

courtesy: https://jestjs.io/docs/en/tutorial-react

but yes Jest is specific to the frameworks like React/Vue and Angular. I hate the fact that it lacks the nice integration platform which is offered by cypress. +1 to cypress for that. PS: I am using material-ui for rendering React controls.

bahmutov commented 5 years ago

@arshmakker see our example in http://on.cypress.io/assertions#Should-callback

danielkcz commented 5 years ago

The thing about data-testid is problematic for a custom component. Imagine you have a composition of like 2-3 components where only the last one in the chain is actually rendering any DOM. Adding hardcoded data-testid to this down level component is not always feasible as there might be more of those on the page. So it ends up with passing that ID through-out other components and it's rather messy.

Partial matching of classnames will surely have a performance hit, so I rather not...

This needs some new thinking out of the box. For example, some code post-processing plugins that would take care of this shenanigans in the background. I am not sure. It kinda makes me avoid testing these problematic scenarios which are a shame. Seeing this on Cypress landing page, it makes me 😢

image

bahmutov commented 5 years ago

@FredyC feel free to use data attribute or partial class name match like we show, it is up to you. Whatever you think will be maintainable in the long term and less likely to break. As far as performance - you are literally testing your code by loading the app again and gain in each test. Slightly less optimal selector would NOT even be close to loading the application over the network or affecting the speed of the test.

We suggest best practices for selectors in https://on.cypress.io/best-practices#Selecting-Elements and I personally recommend using https://github.com/kentcdodds/cypress-testing-library too

danielkcz commented 5 years ago

@bahmutov I've explained the problem of the data attribute, not sure what you don't understand about that. Also, I am using Emotion and there are not even remotely accessible class names to match against.

bahmutov commented 5 years ago

Sure @FredyC I understand - you are using 3rd party library that does not allow adding data attributes and does not have good class names. Well, what can Cypress do besides giving you cy.contains? If you don't expose good selectors, there are no good selectors. I think what would make you happy is a React-specific plugin for Cypress that would allow you to select by React component name, something like cy.react.get('<MyComponent>') or something similar

kuceb commented 5 years ago

@FredyC might not help you in prod, but you can try this package https://github.com/lttb/reselector to automatically add data attrs from component name

edit: I believe it assigns an internal id to each component

danielkcz commented 5 years ago

Sure @FredyC I understand - you are using 3rd party library that does not allow adding data attributes and does not have good class names.

Not only 3rd party components. Imagine a situation there is a generic Button component which actually renders some markup. Then there is PrimaryButton which adds some styling to that Button. Lastly, there can be EditButton which adds some extra logic to above PrimaryButton. To get data attribute down to the Button, I would have to pass it through all those components. And this is a contrived example. In bigger apps, it can get rather nasty.

Well, what can Cypress do besides giving you cy.contains?

I am not saying it's a fault of Cypress (or anyones), I am mostly trying to open discussion if there isn't some better way. Writing tests shouldn't be hard, right?

If you don't expose good selectors, there are no good selectors. I think what would make you happy is a React-specific plugin for Cypress that would allow you to select by React component name, something like cy.react.get('<MyComponent>') or something similar

Not sure how that could work considering you have only the DOM at your disposal. If it's somehow doable then it would definitely improve things a lot in my opinion. Although it would probably need to support selectors as well because only the name of the component doesn't need to be unique. Or would it be chainable with regular selectors? It's really intriguing idea :)

JonJamesDesign commented 5 years ago

If classes need to be dynamic, they should be set based on state.

E.g. using CSS Modules:

import React, { useState } from 'react'
import css from './Menu.module.css'

const Menu = () => {
  const [menuOpen, setMenuOpen] = useState(false)

  const _toggleMenu = () => {
    const newMenuOpenState = !menuOpen
    setMenuOpen(newMenuOpenState)
  }

  return (
    <>
      <button onClick={_toggleMenu}>Open / Close Menu</button>
      <nav className={`${css.menu} ${menuOpen ? css.menuOpen : css.menuClosed}`} />
    </>
  )
}
ModestasDaunys commented 5 years ago

@andrewdang17 this should work: cy.get('...').invoke('attr', 'class').should('contain', 'className')

This saved my life. Thank you.

maemreyo commented 6 months ago

I had stuck with this when working with Cypress, but I found this on the document of Cypress: https://docs.cypress.io/api/commands/get#Find-the-link-with-an-href-attribute-containing-the-word-questions-and-click-it We can use this for the class attribute instead of adding additional testid (or other id on the element) For example: cy.get('div[class*="container"]').should('be.visible');