search

glenjamin / skin-deep

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

Testing 1.0 #54

Closed erquhart closed 8 years ago

erquhart commented 8 years ago

How close is 1.0? If you're not planning any major api changes from where it's at now, I could use it on a project and report issues. Not client work, so no one dies if it breaks. Let me know if that's fitting or helpful.

glenjamin commented 8 years ago

The main change I need to make is the breaking one: making subTree act like subTreeLike

I keep intending to work on this, but getting caught up in other things

erquhart commented 8 years ago

Ha! That's the one thing was driving me crazy last night setting things up, took a while to realize you were using subTreeLike in those tests. I'm not certain I'll have the time, but I'll try to get a PR in to help with that.

erquhart commented 8 years ago

@glenjamin the obvious path would be to simply make subTree do exactly what subTreeLike does. Is there some difference in your intended approach? And second, have you also considered making the 'every' behavior default as well, always returning a collection? One subTree to rule them all?

glenjamin commented 8 years ago

The plan is to make subTree do what subTreeLike does, yes.

I'll still keep everySubTree distinct, as in my experience a single node search is more common.

The main thing slowing me down is I want to make sure the test coverage is still really high

burabure commented 8 years ago

@glenjamin I've been using this for a large project and would glady contribute some tests and code if you give me some directions =)

glenjamin commented 8 years ago

I'm somewhat in two minds about this at the moment.

To move forward I need to make the breaking changes (possibly with a back-compat API), trim down the interfaces, convert all the old tests to the subTree interfaces, and then test it out and publish.

I've been keeping a close eye on enzyme lately, and they seem to be doing a good job and getting very popular. I'm half tempted to point people there instead of finishing off 1.0.

The main disadvantage of enzyme seems to be that it's not very strict about what to include in it's API - so the surface area is huge and there's some stuff in there I'd recommend staying away from (which is most of the reason for doing skin-deep 1.0 in the first place).

erquhart commented 8 years ago

Interesting, I was just about to mention Enzyme, but didn't want to be that guy. What do you find worth avoiding in that lib?

glenjamin commented 8 years ago

Interesting, I was just about to mention Enzyme, but didn't want to be that guy. What do you find worth avoiding in that lib?

  • Prefer shallow over mount or render.
  • Avoid any search method which takes JSX as an argument see. This is because defaultProps are weird (https://github.com/glenjamin/skin-deep/commit/c7b1b5a3343d45bea1044b88d1c8969d987372a2 was when we moved away from this in skin-deep
  • Avoid exact assertions, only assert on the props you really care about
  • CSS selectors are cute, but the best approach I've ended up with is a combination of component name (or type) + a couple of props. Out of the box EnzymeSelector can only do one or the other, but you can create a predicate which does this.
  • I can't think of a good use for setContext, setProps, setState, state, context etc apart from asserting on implementation details instead of observed behaviour

If you use enzyme, but keep your tree searches simple and your assertions focused on things that a user can actually see, things should be fine.

burabure commented 8 years ago

=( I liked the direction this library had, but I guess I'll have to migrate to enzyme and create some eslint rules to prohibit nasty DOM and markup rendering

burabure commented 8 years ago

geez, the more I look at enzyme the less I like the decisions behind it... It really feels like a collection of overlapping utility functions.

Can I convince you to keep working on getting 1.0 out?, of course I'd love to help! We can probably make the API really small and it'd be awesome.

glenjamin commented 8 years ago

Yeah, I think going through that list might have convinced me to do 1.0 even more!

I have a few free days coming up at the end of the week, if I can get the breaking changes done neatly then the rest should be simple enough.

burabure commented 8 years ago

awesome! Let me know what I can help with!

glenjamin commented 8 years ago

Tracking the work required in #74 - unsure how best to accept further contributions on this yet - I'll see see how much I get done in the next week while I have time off.