search

numo-labs / isearch-ui

:yellow_heart: :mag: The ui for inspirational search!
GNU General Public License v3.0
7 stars 7 forks source link

Improve tests #541

Closed jackcarlisle closed 8 years ago

jackcarlisle commented 8 years ago

ready for review

nelsonic commented 8 years ago

@jackcarlisle looking good so far. keep going. we ❤️ tests!

nelsonic commented 8 years ago

@jackcarlisle have we lost our CodeCov test coverage report? is it still configured on CodeShip?

lennym commented 8 years ago

@nelsonic Istanbul reporting was lost when we shifted the ui tests to run in karma/phantom (in order to get around the issues they had with browser APIs being required to run).

The combination of compiling with babel and webpack and istanbul wasn't working and I had sunk days into trying to get it to work before giving up.

You're welcome to try to re-implement istanbul if you like.

nelsonic commented 8 years ago

@lennym oh, hadn't realised. Is there an alternative way of tracking/reporting the "completeness" of our tests with the karma/phantom approach...? (apologies, I'm haven't used Karma since angular days... thanks!)

lennym commented 8 years ago

Having reviewed a good proportion of the code, there's a bit of a recurring theme of "magic number" tests, which are performing assertions on things that are not really relevant or appropriate to the actual functionality we're testing.

A particularly striking example is the following:

expect(wrapper.find('div')).to.have.length(47); - here

These don't really provide any value in terms of confirming the correct functionality of components, but will add maintenance overhead when it comes to making updates to components since the tests are incredibly sensitive to change.

I'd probably want to try to be more specific about what aspects of components are key to their functionality, which generally will mean making more assertions about components in terms of the classNames and text content, and ensuring the scope of the assertions is limited only to the aspects under tests.

jackcarlisle commented 8 years ago

@lennym thanks for the comments! Looking into changing this now!

jackcarlisle commented 8 years ago

TODO: refactor tests for src/components

nelsonic commented 8 years ago

@jackcarlisle can we merge this PR in? (hope your "day off" is going well...)

jackcarlisle commented 8 years ago

@nelsonic Yeah it's good to go!