From 8af91c1a4d6ab62ee2d9e9e76d9990b03da54058 Mon Sep 17 00:00:00 2001 From: Jason Williams Date: Sat, 2 Apr 2016 17:16:20 -0500 Subject: [PATCH] Fix unit test bugs - Stub moment instead of using a fake clock. - Do not mutate fixture data. - Do not modify express.response. --- core/test/unit/config_spec.js | 4 +- core/test/unit/error_handling_spec.js | 56 ++++++++------- core/test/unit/rss_spec.js | 3 +- .../unit/storage_local-file-store_spec.js | 69 ++++++++++++------- 4 files changed, 74 insertions(+), 58 deletions(-) diff --git a/core/test/unit/config_spec.js b/core/test/unit/config_spec.js index 5d41bc79f0..3f83e3baca 100644 --- a/core/test/unit/config_spec.js +++ b/core/test/unit/config_spec.js @@ -7,7 +7,7 @@ var should = require('should'), _ = require('lodash'), testUtils = require('../utils'), - i18n = require('../../server/i18n'), + i18n = require('../../server/i18n'), // Thing we are testing configUtils = require('../utils/configUtils'), @@ -261,7 +261,7 @@ describe('Config', function () { it('should return url for a post from post object', function () { var testContext = 'post', - testData = {post: testUtils.DataGenerator.Content.posts[2]}; + testData = {post: _.cloneDeep(testUtils.DataGenerator.Content.posts[2])}; // url is now provided on the postmodel, permalinkSetting tests are in the model_post_spec.js test testData.post.url = '/short-and-sweet/'; diff --git a/core/test/unit/error_handling_spec.js b/core/test/unit/error_handling_spec.js index 70ca1b7e79..0bd3f56267 100644 --- a/core/test/unit/error_handling_spec.js +++ b/core/test/unit/error_handling_spec.js @@ -311,7 +311,8 @@ describe('Error handling', function () { }); describe('Rendering', function () { - var sandbox; + var app, + sandbox; before(function () { configUtils.set({ @@ -340,6 +341,7 @@ describe('Error handling', function () { }); beforeEach(function () { + app = express(); sandbox = sinon.sandbox.create(); }); @@ -349,10 +351,9 @@ describe('Error handling', function () { it('Renders end-of-middleware 404 errors correctly', function (done) { var req = {method: 'GET'}, - res = express.response; + res = app.response; - sandbox.stub(express.response, 'render', function (view, options, fn) { - /*jshint unused:false */ + sandbox.stub(res, 'render', function (view, options/*, fn*/) { view.should.match(/user-error\.hbs/); // Test that the message is correct @@ -364,15 +365,15 @@ describe('Error handling', function () { done(); }); - sandbox.stub(express.response, 'status', function (status) { - res.statusCode = status; - return res; + sandbox.stub(res, 'status', function (status) { + this.statusCode = status; + return this; }); sandbox.stub(res, 'set', function (value) { // Test that the headers are correct value['Cache-Control'].should.eql('no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0'); - return res; + return this; }); errors.error404(req, res, done); @@ -381,10 +382,9 @@ describe('Error handling', function () { it('Renders thrown 404 errors correctly', function (done) { var err = new Error('A thing was not found'), req = {method: 'GET'}, - res = express.response; + res = app.response; - sandbox.stub(express.response, 'render', function (view, options, fn) { - /*jshint unused:false */ + sandbox.stub(res, 'render', function (view, options/*, fn*/) { view.should.match(/user-error\.hbs/); // Test that the message is correct @@ -396,15 +396,15 @@ describe('Error handling', function () { done(); }); - sandbox.stub(express.response, 'status', function (status) { - res.statusCode = status; - return res; + sandbox.stub(res, 'status', function (status) { + this.statusCode = status; + return this; }); sandbox.stub(res, 'set', function (value) { // Test that the headers are correct value['Cache-Control'].should.eql('no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0'); - return res; + return this; }); err.status = 404; @@ -414,10 +414,9 @@ describe('Error handling', function () { it('Renders thrown errors correctly', function (done) { var err = new Error('I am a big bad error'), req = {method: 'GET'}, - res = express.response; + res = app.response; - sandbox.stub(express.response, 'render', function (view, options, fn) { - /*jshint unused:false */ + sandbox.stub(res, 'render', function (view, options/*, fn*/) { view.should.match(/user-error\.hbs/); // Test that the message is correct @@ -432,12 +431,12 @@ describe('Error handling', function () { sandbox.stub(res, 'set', function (value) { // Test that the headers are correct value['Cache-Control'].should.eql('no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0'); - return res; + return this; }); - sandbox.stub(express.response, 'status', function (status) { - res.statusCode = status; - return res; + sandbox.stub(res, 'status', function (status) { + this.statusCode = status; + return this; }); errors.error500(err, req, res, null); @@ -446,10 +445,9 @@ describe('Error handling', function () { it('Renders 500 errors correctly', function (done) { var err = new Error('I am a big bad error'), req = {method: 'GET'}, - res = express.response; + res = app.response; - sandbox.stub(express.response, 'render', function (view, options, fn) { - /*jshint unused:false */ + sandbox.stub(res, 'render', function (view, options/*, fn*/) { view.should.match(/user-error\.hbs/); // Test that the message is correct @@ -461,15 +459,15 @@ describe('Error handling', function () { done(); }); - sandbox.stub(express.response, 'status', function (status) { - res.statusCode = status; - return res; + sandbox.stub(res, 'status', function (status) { + this.statusCode = status; + return this; }); sandbox.stub(res, 'set', function (value) { // Test that the headers are correct value['Cache-Control'].should.eql('no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0'); - return res; + return this; }); err.statusCode = 500; diff --git a/core/test/unit/rss_spec.js b/core/test/unit/rss_spec.js index ca619e3f6c..736de0f748 100644 --- a/core/test/unit/rss_spec.js +++ b/core/test/unit/rss_spec.js @@ -27,7 +27,8 @@ describe('RSS', function () { var sandbox, req, res, posts; before(function () { - posts = _.filter(testUtils.DataGenerator.forKnex.posts, function filter(post) { + posts = _.cloneDeep(testUtils.DataGenerator.forKnex.posts); + posts = _.filter(posts, function filter(post) { return post.status === 'published' && post.page === false; }); diff --git a/core/test/unit/storage_local-file-store_spec.js b/core/test/unit/storage_local-file-store_spec.js index 3a35a37907..739eb31467 100644 --- a/core/test/unit/storage_local-file-store_spec.js +++ b/core/test/unit/storage_local-file-store_spec.js @@ -1,11 +1,11 @@ -/*globals describe, beforeEach, afterEach, it*/ +/*globals describe, before, after, beforeEach, afterEach, it*/ var fs = require('fs-extra'), + moment = require('moment'), path = require('path'), should = require('should'), sinon = require('sinon'), - rewire = require('rewire'), _ = require('lodash'), - LocalFileStore = rewire('../../server/storage/local-file-store'), + LocalFileStore = require('../../server/storage/local-file-store'), localFileStore, configUtils = require('../utils/configUtils'); @@ -14,7 +14,24 @@ var fs = require('fs-extra'), should.equal(true, true); describe('Local File System Storage', function () { - var image; + var image, + momentStub; + + function fakeDate(mm, yyyy) { + var month = parseInt(mm, 10), + year = parseInt(yyyy, 10); + + momentStub.withArgs('YYYY').returns(year.toString()); + momentStub.withArgs('MM').returns(month < 10 ? '0' + month.toString() : month.toString()); + } + + before(function () { + momentStub = sinon.stub(moment.fn, 'format'); + }); + + after(function () { + momentStub.restore(); + }); beforeEach(function () { sinon.stub(fs, 'mkdirs').yields(); @@ -28,10 +45,9 @@ describe('Local File System Storage', function () { type: 'image/jpeg' }; - // Sat Sep 07 2013 21:24 - this.clock = sinon.useFakeTimers(new Date(2013, 8, 7, 21, 24).getTime()); - localFileStore = new LocalFileStore(); + + fakeDate(9, 2013); }); afterEach(function () { @@ -39,15 +55,14 @@ describe('Local File System Storage', function () { fs.copy.restore(); fs.stat.restore(); fs.unlink.restore(); - this.clock.restore(); - configUtils.restore(); }); it('should send correct path to image when date is in Sep 2013', function (done) { localFileStore.save(image).then(function (url) { url.should.equal('/content/images/2013/09/IMAGE.jpg'); - return done(); + + done(); }).catch(done); }); @@ -55,41 +70,41 @@ describe('Local File System Storage', function () { image.name = 'AN IMAGE.jpg'; localFileStore.save(image).then(function (url) { url.should.equal('/content/images/2013/09/AN-IMAGE.jpg'); - return done(); + + done(); }).catch(done); }); it('should send correct path to image when date is in Jan 2014', function (done) { - // Jan 1 2014 12:00 - this.clock = sinon.useFakeTimers(new Date(2014, 0, 1, 12).getTime()); + fakeDate(1, 2014); + localFileStore.save(image).then(function (url) { url.should.equal('/content/images/2014/01/IMAGE.jpg'); - return done(); + + done(); }).catch(done); }); it('should create month and year directory', function (done) { - localFileStore.save(image).then(function (url) { - /*jshint unused:false*/ + localFileStore.save(image).then(function () { fs.mkdirs.calledOnce.should.be.true(); fs.mkdirs.args[0][0].should.equal(path.resolve('./content/images/2013/09')); + done(); }).catch(done); }); it('should copy temp file to new location', function (done) { - localFileStore.save(image).then(function (url) { - /*jshint unused:false*/ + localFileStore.save(image).then(function () { fs.copy.calledOnce.should.be.true(); fs.copy.args[0][0].should.equal('tmp/123456.jpg'); fs.copy.args[0][1].should.equal(path.resolve('./content/images/2013/09/IMAGE.jpg')); + done(); }).catch(done); }); it('can upload two different images with the same name without overwriting the first', function (done) { - // Sun Sep 08 2013 10:57 - this.clock = sinon.useFakeTimers(new Date(2013, 8, 8, 10, 57).getTime()); fs.stat.withArgs(path.resolve('./content/images/2013/09/IMAGE.jpg')).yields(false); fs.stat.withArgs(path.resolve('./content/images/2013/09/IMAGE-1.jpg')).yields(true); @@ -100,13 +115,12 @@ describe('Local File System Storage', function () { localFileStore.save(image).then(function (url) { url.should.equal('/content/images/2013/09/IMAGE-1.jpg'); - return done(); + + done(); }).catch(done); }); it('can upload five different images with the same name without overwriting the first', function (done) { - // Sun Sep 08 2013 10:57 - this.clock = sinon.useFakeTimers(new Date(2013, 8, 8, 10, 57).getTime()); fs.stat.withArgs(path.resolve('./content/images/2013/09/IMAGE.jpg')).yields(false); fs.stat.withArgs(path.resolve('./content/images/2013/09/IMAGE-1.jpg')).yields(false); fs.stat.withArgs(path.resolve('./content/images/2013/09/IMAGE-2.jpg')).yields(false); @@ -122,7 +136,8 @@ describe('Local File System Storage', function () { localFileStore.save(image).then(function (url) { url.should.equal('/content/images/2013/09/IMAGE-4.jpg'); - return done(); + + done(); }).catch(done); }); @@ -141,7 +156,8 @@ describe('Local File System Storage', function () { it('should send the correct path to image', function (done) { localFileStore.save(image).then(function (url) { url.should.equal('/content/images/2013/09/IMAGE.jpg'); - return done(); + + done(); }).catch(done); }); }); @@ -171,7 +187,8 @@ describe('Local File System Storage', function () { // slashes and it returns a path that needs to be normalized path.normalize(url).should.equal('/content/images/2013/09/IMAGE.jpg'); } - return done(); + + done(); }).catch(done); }); });