Skip to content
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

Increase specs for Posts Lists component #49

Open
paul-barry-kenzan opened this issue Dec 13, 2016 · 4 comments
Open

Increase specs for Posts Lists component #49

paul-barry-kenzan opened this issue Dec 13, 2016 · 4 comments
Assignees

Comments

@paul-barry-kenzan
Copy link

The final Post Lists spec has ~15 expect() statements. If one were to fail, the entire block would fail, but wouldn't necessarily indicate WHICH expect was failing.

If the spec is determining that the component "should test that 2 posts are rendered in the component", then the expect should verify that a count/length is "2". Any other expect statement(s) should be relocated to a new spec that describes what is being checked.

// for example
it('should display "Latest Posts" as a title', function () {
  // spec setup if necessary
  // ...
  expect(componentTitle[0].textContent).toBe('Latest Posts');
});
@thescientist13
Copy link
Member

great advice, thanks @paul-barry-kenzan !

@thescientist13
Copy link
Member

thescientist13 commented Jan 21, 2017

@paul-barry-kenzan
So in your recommendation, are you suggesting that testing for DOM node counts should be a unique it block from the testing of the actual content set in those DOM nodes?

@paul-barry-kenzan
Copy link
Author

@thescientist13
Yes, I typically separate tests by concern, functionality vs. DOM. Take a button that toggles a boolean. Some devs write tests from the button down to the functionality, while I write them in functionality -> button. Consider these 2 sets of examples.

// writing button -> functionality
it('should toggle the value from FALSE to TRUE when clicked', function () {
  // find button
  // trigger click
  // assert value is true
});

Seems like a logical test. "I have a button, that when clicked, toggles a value."

I dislike the above type of tests because there is too much coding being done at one time. For the above, the button, click listener, click handler, and logic all have to be in place for the test to pass. This also makes it extremely difficult to change the logic and/or the handlers if/when ACs are updated.

The following tests separate the handler from the target element. This allows for functionality to be isolated and tested regardless of the DOM event.

// ** functionality tests first **
it('should expose a method to toggle value', function () {
  // assert that method is accessible from DOM
});
it('should toggle FALSE to TRUE', function () {
  // assert that value if FALSE to verify starting point
  // call exposed method directly
  // assert that value is TRUE
});
it('should toggle TRUE to FALSE'); // same as FALSE -> TRUE but values reversed

// ** DOM tests **
// at this point we know the method is doing what it's suppose to, regardless of how it's called.
// Now we can test the button is doing what it's suppose to do
it('should call exposed method when clicked', function () {
  // setup spy on exposed method
  // access button with selectors, etc,
  // trigger click event
  // assert that spy was called
});

We don't necessarily have to assert that the value toggles, since we have tests for that already. The button doesn't care WHAT is happening when it gets clicked, just WHICH method is called. In the future, if the toggle method is changed to do something besides toggle, the button tests still pass and are still valid. The tests which verify the value is toggled would need to be updated, but that's expected with functionality updates.

@thescientist13
Copy link
Member

Well reasoned @paul-barry-kenzan, and thanks for the thorough and detailed breakdown!

@thescientist13 thescientist13 added this to the Testing Improvements milestone Jan 24, 2017
@thescientist13 thescientist13 self-assigned this Jan 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants