15
\$\begingroup\$

I have mongoose model:

var mongoose = require('mongoose'),
    Schema = mongoose.Schema,
    Imager = require('imager'),
    env = process.env.NODE_ENV || 'development',
    config = require('../../config/config')[env],
    imagerConfig = require(config.root + '/config/imager.js');

var LinkSchema = new Schema({
    title: { type: String, 'default': '', trim: true },
    siteName: { type: String, 'default': '', trim: true },
    url: { type: String, 'default': '', trim: true },
    description: { type: String, 'default': '', trim: true },
    image: {
        cdnUri: String,
        files: []
    },
    tags: { type: [], 'default': [] },
    createdAt: { type: Date, 'default': Date.now },

    user: { type: Schema.ObjectId, 'default': null, ref: 'User' }
});

var isStringPresence = function (value) {
    return !!value && value.length > 0;
};

LinkSchema.path('title').validate(function (title) {
    return isStringPresence(title);
}, 'Title cannot be blank');

LinkSchema.path('url').validate(function (url) {
    return isStringPresence(url);
}, 'URL cannot be blank');

LinkSchema.path('user').validate(function (user) {
    return !!user;
}, 'Link must be linked to a user');

LinkSchema.pre('remove', function (next) {
    var imager = new Imager(imagerConfig, 'S3'),
        files = this.image.files;

    // if there are files associated with the link, remove from the cloud too
    imager.remove(files, function (err) {
        if (err) {
            return next(err);
        }
    }, 'link');

  next();
});

LinkSchema.methods = {
    uploadAndSave: function (images, callback) {
        if (!images || !images.length) {
            return this.save(callback);
        }

        var imager = new Imager(imagerConfig, 'S3'),
            self = this;

        imager.upload(images, function (err, cdnUri, files) {
            if (err) {
                return callback(err);
            }
            if (files.length) {
                self.image = {
                    cdnUri: cdnUri,
                    files: files
                };
            }

            self.save(callback);
        }, 'link');
    }
};

LinkSchema.statics = {
    load: function (id, callback) {
        this.findOne({ _id : id })
          .populate('user', 'name email')
          .exec(callback);
    },
    list: function (options, callback) {
        var criteria = options.criteria || {};

        this.find(criteria)
            .populate('user', 'name')
            .sort(options.sort || {'createdAt': -1}) // sort by date
            .limit(options.perPage)
            .skip(options.perPage * options.page)
            .exec(callback);
    }
};

module.exports = mongoose.model('Link', LinkSchema);

I am trying to write correct test file for this model. I am using mocha for this. This is what I end up with:

if ( process.env.NODE_ENV !== 'test' ) {
    console.log('NODE_ENV=' + process.env.NODE_ENV + ' which might cause problems.');
    process.exit(1);
}

var config = require('../config/config')[process.env.NODE_ENV];
var mongoose = require('mongoose');
var Link = require('../app/models/link');
var User = require('../app/models/link');
var should = require('should');
var Factory = require('./support/factories');
var helpers = require('./support/helpers');
var facebookUser = null;

describe('Link Model', function() {

    before(function(done) {
        if (mongoose.connection.db) {
            return done();
        }
        mongoose.connect(config.db, done);
    });

    after(function(done){
        mongoose.connection.db.dropDatabase(function(){
            mongoose.connection.close(done);
        });
    });

    beforeEach(function(done){
        mongoose.connection.db.dropDatabase(function(err){
            if (err) return done(err);
            Factory.build('facebookUser', function(user) {
               facebookUser = user;
               done();
            });
        });
    });


    var saveLinkWithBlankTitle = function(done) {
        return function(link) {
            link.save(function (err, storedLink) {
                err.errors.title.type.should.equal('Title cannot be blank');
                done();
            });
        };
    };

    it("Creates invalid link with title='')", function(done){
        Factory.build('link', {user: facebookUser, title: ''}, saveLinkWithBlankTitle(done));
    });

    it('Creates invalid link with title=null)', function(done){
        Factory.build('link', {user: facebookUser, title: null}, saveLinkWithBlankTitle(done));
    });

    it('Creates invalid link with title=undefined)', function(done){
        Factory.build('link', {user: facebookUser, title: undefined}, saveLinkWithBlankTitle(done));
    });


    var saveLinkWithBlankUrl = function(done) {
        return function(link) {
            link.save(function (err, storedLink) {
                err.errors.url.type.should.equal('URL cannot be blank');
                done();
            });
        };
    };

    it("Creates invalid link with url='')", function(done){
        Factory.build('link', {user: facebookUser, url: ''}, saveLinkWithBlankUrl(done));
    });

    it('Creates invalid link with url=null)', function(done){
        Factory.build('link', {user: facebookUser, url: null}, saveLinkWithBlankUrl(done));
    });

    it('Creates invalid link with url=undefined)', function(done){
        Factory.build('link', {user: facebookUser, url: undefined}, saveLinkWithBlankUrl(done));
    });


    var saveLinkWithNoUser = function(done) {
        return function(link) {
            link.save(function (err, storedLink) {
                err.errors.user.type.should.equal('Link must be linked to a user');
                done();
            });
        };
    };

    it('Creates invalid link with no user', function(done){
        Factory.build('link', saveLinkWithNoUser(done));
    });

    it('Creates invalid link with user=null', function(done){
        Factory.build('link', {user: null}, saveLinkWithNoUser(done));
    });

    it('Creates invalid link with user=undefined', function(done){
        Factory.build('link', {user: undefined}, saveLinkWithNoUser(done));
    });


    it('Creates a valid link (without image)', function(done){
        Factory.build('link', {user: facebookUser}, function(link) {
            link.save(function (err, storedLink) {
                if (err) return done(err);
                storedLink.title.should.equal(link.title);
                storedLink.siteName.should.equal(link.siteName);
                storedLink.url.should.equal(link.url);
                storedLink.description.should.equal(link.description);
                storedLink.tags.should.equal(link.tags);
                done();
            });
        });
    });

    it('Loads existing link', function(done){
        Factory.create('link', {user: facebookUser}, function(storedLink) {
            Link.load(storedLink._id, function(err, loadedLink) {
                if (err) return done(err);
                loadedLink._id.equals(storedLink._id).should.be.true;
                done();
            });
        });
    });

    it('Loads links list', function(done){
        Factory.create('link', {user: facebookUser, title: 'Title1', description: 'a'}, function(link1) {
            Factory.create('link', {user: facebookUser, title: 'Title2', description: 'b'}, function(link2) {
                Factory.create('link', {user: facebookUser, title: 'Title3', description: 'a'}, function(link3) {
                    Factory.create('link', {user: facebookUser, title: 'Title4', description: 'b'}, function(link4) {
                        Factory.create('link', {user: facebookUser, title: 'Title5', description: 'a'}, function(link5) {
                            Factory.create('link', {user: facebookUser, title: 'Title6', description: 'b'}, function(link6) {
                                Factory.create('link', {user: facebookUser, title: 'Title7', description: 'a'}, function(link7) {
                                    Link.list({
                                        criteria: {description: 'a'},
                                        sort: {'createdAt': 1},
                                        perPage: 2,
                                        page: 1
                                    }, function(err, list) {
                                        if (err) return done(err);
                                        list.length.should.equal(2);
                                        list[0].title.should.equal('Title5');
                                        list[1].title.should.equal('Title7');
                                        done();
                                    });
                                });
                            });
                        });
                    });
                });
            });
        });
    });
});

I am using factory lady, here is the code:

var Factory = require('factory-lady');
var Link = require('../../app/models/link');
var User = require('../../app/models/user');

Factory.define('link', Link, {
    title: 'My title',
    siteName: 'My site name',
    url: 'http://www.example.com',
    description: 'My description',
    tags: ['MyFirstTag', 'MySecondTag', 'MyThirdTag']
});

Factory.define('facebookUser', User, {
    name: 'Facebook User',
    email: 'user@facebook.com',
    username: 'fbuser',
    provider: 'facebook',
    hashed_password: 'hashed',
    salt: 'salt',
    facebook: { }
});

module.exports = Factory;
  1. This is my first time of writing tests. Does my code fine?
  2. Do I miss anything things?
  3. How can I make my code shorter?
  4. How can I prevent the callbacks pyramid?
\$\endgroup\$
9
\$\begingroup\$

The code looks pretty good. There are just a few stylistic things I would do differently.

1. Nested Describes

You have quite a few tests which essentially test the same method of your model. Specifically, your tests which call functions like: saveLinkWithNoUser are all testing the Link.save. In cases like this, you can further organize your tests to be more descriptive by running those in a nested define.

eg:

describe( 'Link Model', function() {
  // ... setup code, helper functions, etc
  describe( '#save', function() {
    it("Creates invalid link with title='')", function(done){
      //.. run the test
    });
  });
});

2. Improper 'it' descriptions The output of your tests should be a definition of your model. On lines like:

it('Creates invalid link with title=null)' ...)};

you are describing the test, not the model. Your output will say something like:

Link Model
  Creates invalid link with title=null

The preferred description in my opinion would describe the models actual behavior with invalid links:

Link Model
  Disallows creation of links with title=null

If you follow bold points 1 and 2 your output would be something like:

Link Model
  #save
    Disallows creation of links with title=null

3. Explicitly changing property values in Factory build chain

This point may be caused to improvements to factory-lady since the time of your post. But the factory-lady documentation shows that you can use functions to change properties of contiguously built instances. This would take quite a bit of the hardcoded values out of your links list test.

Sidenote: consider switching to factory-girl which is an enhanced fork of factory-lady that implements some candy like Factory.buildMany.

\$\endgroup\$
3
\$\begingroup\$

Fun code,

there are a few Mocha / Jasmine submissions on this site, and this one is by far the easiest to follow. Which is all kinds of funny since this is the first time you are writing tests ;)

For your questions:

  • Your code looks fine, it seems that Mocha and should.js play very nice together and naturally drive you to grokkable code.
  • I did not find anything obviously missing
  • I did not find your code too long, you have to set up the test and you have to validate the result, seems fine to me
  • Callback pyramids -> There are a bunch of modules for that our there, you need to do some research, I like Q myself
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.