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

CollectionBinder unit tests #209

Closed

Conversation

platinumazure
Copy link
Contributor

This pull request is being made to address #208 (lack of unit tests for Backbone.CollectionBinder).

Get involved!

Want to help out? This could get done faster if multiple people chip in :-) Just follow these steps:

  1. Fork the repository, if needed;
  2. Create a local branch, if desired;
  3. Create a new remote using my Github repository clone URL if needed (https://github.com/platinumazure/Backbone.ModelBinder.git);
  4. Merge or rebase from platinumazure/Backbone.ModelBinder:collectionbinder-tests;
  5. Add your commits to your local branch;
  6. Open a pull request, setting the base fork to the platinumazure Backbone.ModelBinder and the base to "collectionbinder-tests".
  7. If that pull request looks good, I will merge the changes and they will automatically be added to this pull request.

Current Status

It's going to take some time to get them all done, so I'm going to use this initial comment to provide a running status update as I add commits.

Last updated 2/21/2015 03:38 GMT

Done

  • Basic tests around Backbone.CollectionBinder constructor, Backbone.CollectionBinder#bind(), and Backbone.CollectionBinder#unbind()
  • Backbone.CollectionBinder#getManagerForEl and other support-ish methods
  • Backbone.CollectionBinder.ElManagerFactory tests

To Do

  • Unit tests around the element manager factories (this is where the main functionality is!):
    • Backbone.CollectionBinder.ViewManagerFactory

Note: Auto-sort testing has been covered by previously merged pull requests

});
});

describe("Using collection events", function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this section. It doesn't test any behavior expected by a user.

Instead we should test that CollectionBinder adds elements on collection.add, remove on collection.remove, do complex actions on collection.set/reset (that is what you actually do in the other sections below).

Conflicts:
	spec/SpecRunner.html
Just need to test HTML string templating next.
Using a different style than ElManagerFactory tests which I think is better. Need to refactor ElManagerFactory tests to match and possibly augment the CollectionBinder tests.
@platinumazure
Copy link
Contributor Author

@amakhrov I've checked in bare bones for ElManagerFactory and ViewManagerFactory tests. Which do you like better? (ElManagerFactory is more integration level, ViewManagerFactory is more unit level. I'm leaning towards ViewManagerFactory at the moment because then I can rewrite some of the CollectionBinder tests in terms of elManagerFactory/elManager stub invocations, allowing the tests to have the same separation of concerns that the code currently allows for.)

@theironcook As I was writing some of these, I was thinking maybe it might be worth (eventually) refactoring ElManagerFactory in terms of ViewManagerFactory? (That is, ElManagerFactory is a ViewManagerFactory with a preconfigured view that just spews out the given HTML or template, in such a way as to maintain the contract and behavior of the current ElManagerFactory.) I feel like it would make testing and reasoning about this a bit easier; and it would also eventually allow for only one elManagerFactory (ViewManagerFactory) and we could just expose factory functions for the preconfigured views. Let me know what you think.

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

Successfully merging this pull request may close these issues.

None yet

2 participants