glenjamin / skin-deep

Test assertion helpers for use with React's shallowRender test utils
MIT License
200 stars 40 forks source link

More granular query methods with predicate function and/or props #9

Open jrunning opened 8 years ago

jrunning commented 8 years ago

It would be great to be able to do more granular searches by passing a props or function argument to query methods):

// Given the following tree:
<div>
  <Widget title="First" count="10" active />
  <Widget title="Second" count="20"/>
  <Widget title="Third" count="30"/>
</div>

tree.findComponent('Widget', { title: 'First' });
// => <Widget title="First" .../>

tree.findComponent('Widget', { active: false });
// => <Widget title="Second" .../>

tree.findComponent('Widget', (props) => props.count > 25);
// => <Widget title="Third" .../>

On a related note, it would be nice if there was an everySubTreeLike method analogous to findComponentLike and more "plural" methods to return multiple nodes/components e.g. findNodes/findComponents(Like)/whathaveyou.

glenjamin commented 8 years ago

Hi!

Thanks for the feedback.

The current inconsistent APIs are mostly a result of evolution from the original API and various holes we found via internal use. Every time I added a feature I kept backwards-compatibility with the previous stuff.

I'm not entirely sure that all the existing APIs make sense.

I like the idea of being able to pass a filter function as well.

There's also a semi-hidden chai plugin you can require as skin-deep/chai, which can lead to some nicer assertions.

At the moment there's 3 different families:

And currently subTree has the every variant, and component has the like variant.

This seems a bit muddled - internally the implementation is all just a single tree-walk function which takes a predicate - so any combination of the above is pretty doable.

We could just expand all 3 flavours so they support all the variants, but ideally I'd like to trim down a bit.

glenjamin commented 8 years ago

Here's some stats from our 3 internal testsuites - I'll ask around the office as well and see what people think.

grep -hriEo 'findComponent(like)?|(every)?subTree|findNode' -- test | sort | uniq -c | sort -n
   4 findComponent
  10 everySubTree
  36 findComponentLike
  45 subTree
  86 findNode
   2 subTree
   5 findComponentLike
 101 findNode
   2 findComponentLike
   2 subTree
  63 findNode

The skew towards findNode is likely because that was the original API, rather than it being any better.

jrunning commented 8 years ago

Thanks, that's good info. I agree that the API could use some refinement.

One thing about my use case is that my app has very few ids or classNames (we use react-css-modules, so we have mostly styleNames), so I found findNode to be of limited utility.

glenjamin commented 8 years ago

We've actually been putting in specific classes for our end-to-end selenium tests, and then hooking some unit tests of those, eg .qa-login-button. We're starting to move away from this for unit tests towards more subTree and findComponent based approaches.

glenjamin commented 8 years ago

One thing I've noticed we do a lot is findNode('ComponentName') and then assert on props - this doesn't help if you have many components, but does give better errors if there's one.

glenjamin commented 8 years ago

We had a chat internally, current thinking is to trim down to

And expand these so they all take a selector as the first argument, and a props object / predicate as the second argument.

glenjamin commented 8 years ago

Summary of current plan

jrunning commented 8 years ago

That sounds great!

glenjamin commented 8 years ago

I'm working on writing up notes for a v1.0 API - what do people think about changing subTree to do what subTreeLike currently does, and adding a subTreeExact function?

The idea is to change the API to push users towards loose selectors over strict ones.

/cc @jrunning @hoegrammer @foiseworth @donabrams @hartzis

glenjamin commented 8 years ago

A possible approach would be to use the fact that the new API will accept a function as the second argument for the overloading:

// match subset of props
tree.subTree('Component', { some: 'prop' });

// match exact props
tree.subTree('Component', sd.exact({exact: 'props'}));

This would allow trimming down the top-level selection API to only subTree and everySubTree

glenjamin commented 8 years ago

I've posted a draft of introduction and overview docs for what would become the 1.0 API: https://github.com/glenjamin/skin-deep/tree/one-point-oh

hartzis commented 8 years ago

Definitely agree with the change to subTree to match functionality of subTreeLike, and everything else you're proposing in the docs for 1.0!

Have you thought about dropping support for react 0.13 when jumping to v1.0?

glenjamin commented 8 years ago

I thought about dropping 0.13, but we've not upgraded at work yet, and the compat layer is quite thin so I'll probably keep it.

Might do 0.99 and save 1.0 for when 0.13 drops