googlearchive / angularfire-seed

Seed project for AngularFire apps
http://angularfire.com
MIT License
410 stars 191 forks source link

Testing $loaded and other methods on fbutil.syncArray #53

Closed zommbot closed 9 years ago

zommbot commented 9 years ago

I'm a little confused how to set up a spy and mock the fbutil.syncArray prototypes using your fbutilStub object.

Right now I'm just adding this line to mock out a returned array of [1,2,3]:

obj.syncArray.andCallFake(function() { return [1,2,3]; });

quick reference of the original stub you wrote: function fbutilStub() { var obj = jasmine.createSpyObj('fbutil', ['syncObject', 'syncArray', 'ref']); obj.$$ref = new Firebase(); obj.syncObject.andCallFake(function() { return {}; }); obj.syncArray.andCallFake(function() { return []; }); obj.ref.andCallFake(function() { return obj.$$ref; }); fbutilStub.$$last = obj; return obj; }

i'm new to jasmine and firebase so i wasnt sure if i should add those as prototypes spys or what. A quick example would go a lonnnng way.

Your tests are great on the simplelogin so I was hoping you could please point me in the right direction because i think the syncArray method will be the most tested part of firebase outside of simplelogin.

katowulf commented 9 years ago

Hi there!

So you had the right repo, since this was a question about AngularFire test units. However, fbutil.syncArray() is a method in AngularFire-seed's service and with the lack of details, I was a bit lost. This is making a bit more sense now. Let's just keep this here and see what we can make of it, shall we?

Can you explain exactly what you're attempting to test? You probably shouldn't be testing the syncArray method (we do that) or fbutil (we do that, too) or any third party libs, except during your e2e tests, which naturally exercise those on your behalf.

If you are attempting to create your own factories and test those, you should see how we test $FirebaseArray and follow suit (you can probably just copy/paste that test lib and put in your own class instead).

Let me know what we're aiming for here and I'll be glad to help work out the kinks.

Cheers,

zommbot commented 9 years ago

Wow! Thanks for the fast response....

I just spent some time looking over the $FirebaseArray testing in the angularfire project and I'm starting to get a lot better idea of how I should write these tests. I'm still new to testing angular/jasmine and your source code has been extremely helpful in understanding best-practices, so whenever you have a chance it would help a lot if you could just tell me how you would test the same thing I'm trying to write....

the error I was getting in my tests is "'undefined' is not a function (evaluating 'things.$loaded()')"

Basically I have a a route resolving a 'user', then inside the main controller i am passing in that user and using it to get $scope.things synced with firebase... so this is in the controller:

$scope.things = fbutil.syncArray('users/'+user.id+'/things')

then I am parsing the $scope.things into 'active' and 'inactive' like this:

$scope.activeThings = _.where($scope.things, function(thing){ return thing.active = true; })

this wasnt being resolved on page load though so I added this:

$scope.things.$loaded().then(function(){ $scope.activeThings = _.where($scope.things, function(thing){ return thing.active = true; }) })

So when I added the above code to the controller the $scope.activeThings loaded in the browser but broke all my tests on the controller because scope.things.$loaded doesnt exist in the 'test world'

On the MainCtrl test I am testing that certain things are being loaded on the scope. I used fbutilStub and some other code I got from you in order to mock out fbutil. Again, I'm new to writing angular/jasmine tests so I have no idea if this is the right way to do things.

Here's my MainCtrl test to give you a better idea: (let me know if you want me to convert this to js, I know ppl get annoyed with reading coffee) /////////// 'use strict'

describe 'Controller: MainCtrl', ->

beforeEach module 'adfireApp' beforeEach module 'tpl/app_dashboard.html' beforeEach module 'mock.firebase'

stub = -> out = {} angular.forEach arguments, (m) -> out[m] = jasmine.createSpy() out

fbutilStub = -> obj = jasmine.createSpyObj("fbutil", [ "syncObject" "syncArray" "ref" ]) obj.$$ref = new Firebase() obj.syncObject.andCallFake -> {} obj.syncArray.andCallFake -> [1,2,3] obj.ref.andCallFake -> obj.$$ref fbutilStub.$$last = obj obj

resolve = -> def = $q.defer() def.resolve.apply def, Array::slice.call(arguments, 0) def.promise

authStub = -> list = [ "$login" "$logout" "$createUser" "$changePassword" "$removeUser" "$getCurrentUser" "$sendPasswordResetEmail" ] obj = jasmine.createSpyObj("$firebaseSimpleLogin", list) angular.forEach list, (m) -> obj[m].andReturn resolve(authStub.$$user) authStub.$$last = obj obj

flush = -> try loop

flush until there is nothing left to flush (i.e. $timeout throws an error)

    # this is necessary for createAccount which uses some chained promises that
    # iterate through set/remove/promise calls, all of which have to get flushed
    fbutilStub.$$last.$$ref.flush()
    $timeout.flush()
return

is okay

ErrorWithCode = (code, msg) -> @code = code @msg = msg return $q = undefined $timeout = undefined

fbutil = null MainCtrl = undefined scope = undefined $rootScope = null simpleLoginMock = { init: -> null, getUser: -> null } testUser = id: 1 email: 'test@test.com'

beforeEach -> MockFirebase.override() module 'adfireApp', ($provide) -> $provide.value 'simpleLogin', simpleLoginMock $provide.value "fbutil", fbutilStub() return inject ($controller, $rootScope, simpleLogin, fbutil) -> $rootScope = $rootScope fbutil = fbutil simpleLogin = simpleLogin scope = $rootScope.$new() simpleLoginMock.getUser = -> testUser spyOn(simpleLoginMock, 'getUser').andCallThrough() MainCtrl = $controller 'MainCtrl', $scope: scope user: simpleLogin.getUser()

it 'should populate scope.user with ui-router resolve.user', -> expect(scope.user).toBeDefined() expect(scope.user).toEqual testUser

it 'should return things as an array from firebase', -> expect(scope.things).toBeDefined()

it 'should be able to add a thing', -> expect(scope.add_thing).toBeA('function')

it 'should be able to remove a thing', -> expect(scope.remove_thing).toBeA('function')

it 'should be able to update a thing', -> expect(scope.update_thing).toBeA('function')

it 'should call the right path', -> expect(fbutil.syncArray).toHaveBeenCalledWith('users/1/things')

it 'should return the right amount of things', -> expect(scope.things.length).toBe(3) ///////////

And here's the controller: /////////// .controller 'MainCtrl', ($rootScope, $scope, $http, fbutil, user, $timeout) ->

$scope.activeThings = [] $scope.user = user $scope.things = things = fbutil.syncArray('users/'+user.id+'/things')

temp hack to get tests to work

if $scope.things.$loaded things.$loaded().then -> $scope.activeThings = (thing for thing in things when thing.active)

$scope.add_thing = (thing) -> $scope.things.$add(thing)

$scope.remove_thing = (thing) -> $scope.things.$remove(thing)

$scope.update_thing = (thing) -> $scope.things.$update(thing) ///////////

I added a quick hack in the controller above to make the tests pass: if $scope.things.$loaded

get $scope.activeThings here

The goal is still to test activeThings and make the mock will sync up with the actual FirebaseArray structure.... after looking through that code just now I'm still a little blurry on how to best write a test for that. Please let me know if this is the kind of setup you would make if you were creating tests for something like this. This isn't necessarily an 'issue' so please take your time replying, its not at all urgent. I appreciate any advice you have.

katowulf commented 9 years ago

There are probably two issues here. The first is that you have a timing issue and user is null at the time the user.id is called, which is causing your undefined issue. This is probably because you need to call $login() before your controller loads (maybe in a beforeEach()).

The second issue here is that testing controllers is tricky stuff. I generally don't write tests for controllers because it feels like a time sink. In general, the majority of your controller logic is best exercised in an end-to-end test rather than a test unit. Just use the pages as designed and let the end-to-end test make sure that the right things show up in the DOM. If so, the controller works as intended and all the moving parts are playing nicely.

To write controller tests, I'd recommend you mock everything that goes into the controller rather than trying to rely on all these services working correctly in a test environment, which is a lot of async juggling to get correct.

Since test units should test exactly one bit of functionality, the mocks can be fairly simple. For example:

describe('ControllerTest', function() {
    // set up our dependencies and our mocks
    beforeEach(function() {
        var self = this;
        module("dependency...");
        module("app...', function($provide) {
              // spy on our function to make sure it is called
              // return an array so controller does not explode
              self.spy = jasmine.createSpy().and.callFake(function() { return []; });
              // replace $firebase with a mock
              $provide.value('$firebase', function() {
                    return function() {
                          // include our spy
                          return { $asArray: self.spy };
                    }
              });
       });
   });

   // set up our controller instances
  beforeEach(inject(function ($rootScope, $controller, $location) {
        this.$location = $location;
        this.scope = $rootScope.$new();

        this.createController = function() {
            return $controller('NavCtrl', {
                '$scope': this.scope
            });
        };
    }));

   it('should add an array to $scope', function() {
        var controller = this.createController();
        expect( this.spy ).toHaveBeenCalled(); // $asArray() was invoked
        expect( this.scope.list ).toBeDefined(); // created $scope.list
   });
});

In summary:

(on formatting and etiquette: Try to use the formatting for your code blocks by surrounding it in ``` tags so it has proper spacing. Don't mind coffee script much, but unindented code is tough to grok)

zommbot commented 9 years ago

Just FYI I've spent the last day going over this and all of your other tests on firebase to get a better idea of how to model my tests. Thanks for all your help on this!!!!!

katowulf commented 9 years ago

Glad to help. Happy TDD!