HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Test mongoose model

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
modeltestmongoose

Problem

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 = {

Solution

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
    });
  });
});


  1. 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


  1. 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.

Code Snippets

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

Context

StackExchange Code Review Q#28758, answer score: 9

Revisions (0)

No revisions yet.