💄 🐷 Test consistency (#8199)

no issue

- change out should.equal for // jshint ignore:line
- ensure should is the first require in every test, and ALWAYS require
- make sinon the second require, and sandbox the last thing
- ALWAYS use sandbox, futureproofs tests against contributors who don't know it
- change require formatting

core/server/apps/amp/tests/amp_components_spec.js

@@ -1,7 +1,7 @@
-var should         = require('should'),
+var should = require('should'),
 
 // Stuff we are testing
-    ampComponentsHelper    = require('../lib/helpers/amp_components');
+    ampComponentsHelper = require('../lib/helpers/amp_components');
 
 describe('{{amp_components}} helper', function () {
     it('adds script tag for a gif', function () {

core/server/apps/amp/tests/amp_content_spec.js

@@ -1,9 +1,9 @@
-var should         = require('should'),
-    rewire         = require('rewire'),
-    configUtils    = require('../../../../test/utils/configUtils'),
+var should = require('should'),
+    rewire = require('rewire'),
+    configUtils = require('../../../../test/utils/configUtils'),
 
 // Stuff we are testing
-    ampContentHelper    = rewire('../lib/helpers/amp_content');
+    ampContentHelper = rewire('../lib/helpers/amp_content');
 
 // TODO: Amperize really needs to get stubbed, so we can test returning errors
 // properly and make this test faster!

core/server/apps/amp/tests/router_spec.js

@@ -1,14 +1,15 @@
-/*globals describe, beforeEach, afterEach, it*/
-var rewire            = require('rewire'),
-    ampController     = rewire('../lib/router'),
-    path              = require('path'),
-    sinon             = require('sinon'),
-    Promise           = require('bluebird'),
-    errors            = require('../../../errors'),
-    should            = require('should'),
-    configUtils       = require('../../../../test/utils/configUtils'),
-    themes            = require('../../../themes'),
-    sandbox           = sinon.sandbox.create();
+var should = require('should'),
+    sinon = require('sinon'),
+    rewire = require('rewire'),
+    path = require('path'),
+    Promise = require('bluebird'),
+
+    ampController = rewire('../lib/router'),
+    errors = require('../../../errors'),
+    configUtils = require('../../../../test/utils/configUtils'),
+    themes = require('../../../themes'),
+
+    sandbox = sinon.sandbox.create();
 
 // Helper function to prevent unit tests
 // from failing via timeout when they

core/server/apps/default-cards/tests/hr_spec.js

@@ -1,9 +1,8 @@
-var should  = require('should'),
-    card    = require('../cards/hr'),
-    SimpleDom   = require('simple-dom'),
+var should = require('should'), // jshint ignore:line
+    card = require('../cards/hr'),
+    SimpleDom = require('simple-dom'),
     opts;
 
-should = should;
 describe('HR card', function () {
     it('generates a horizontal rule', function () {
         opts = {

core/server/apps/default-cards/tests/html_spec.js

@@ -1,9 +1,8 @@
-var should  = require('should'),
-    card    = require('../cards/html'),
-    SimpleDom   = require('simple-dom'),
+var should = require('should'), // jshint ignore:line
+    card = require('../cards/html'),
+    SimpleDom = require('simple-dom'),
     opts;
 
-should = should;
 describe('HTML card', function () {
     it('HTML Card renders', function () {
         opts = {

core/server/apps/default-cards/tests/image_spec.js

@@ -1,9 +1,8 @@
-var should  = require('should'),
-    card    = require('../cards/image'),
-    SimpleDom   = require('simple-dom'),
+var should = require('should'),  // jshint ignore:line
+    card = require('../cards/image'),
+    SimpleDom = require('simple-dom'),
     opts;
 
-should = should;
 describe('Image card', function () {
     it('generates an image', function () {
         opts = {

core/server/apps/default-cards/tests/markdown_spec.js

@@ -1,9 +1,8 @@
-var should  = require('should'),
-    card    = require('../cards/markdown'),
-    SimpleDom   = require('simple-dom'),
+var should = require('should'),  // jshint ignore:line
+    card = require('../cards/markdown'),
+    SimpleDom = require('simple-dom'),
     opts;
 
-should = should;
 describe('Markdown card', function () {
     it('Markdown Card renders', function () {
         opts = {

core/server/apps/default-cards/tests/soft-return_spec.js

@@ -1,9 +1,8 @@
-var should  = require('should'),
-    card    = require('../atoms/soft-return'),
-    SimpleDom   = require('simple-dom'),
+var should = require('should'), // jshint ignore:line
+    card = require('../atoms/soft-return'),
+    SimpleDom = require('simple-dom'),
     opts;
 
-should = should;
 describe('Soft return card', function () {
     it('generates a `br` tag', function () {
         opts = {

core/server/apps/private-blogging/tests/controller_spec.js

@@ -1,10 +1,11 @@
 /*globals describe, beforeEach, afterEach, it*/
-var privateController = require('../lib/router').controller,
-    should            = require('should'),
-    path              = require('path'),
-    sinon             = require('sinon'),
-    configUtils       = require('../../../../test/utils/configUtils'),
-    themes            = require('../../../themes'),
+var should = require('should'),
+    sinon = require('sinon'),
+    configUtils = require('../../../../test/utils/configUtils'),
+    path = require('path'),
+    themes = require('../../../themes'),
+    privateController = require('../lib/router').controller,
+
     sandbox = sinon.sandbox.create();
 
 describe('Private Controller', function () {

core/server/apps/private-blogging/tests/input_password_spec.js

@@ -1,6 +1,6 @@
 // We use the name input_password to match the helper for consistency:
 // jscs:disable requireCamelCaseOrUpperCaseIdentifiers
-var should         = require('should'),
+var should = require('should'),
 
 // Stuff we are testing
     input_password = require('../lib/helpers/input_password');

core/server/apps/private-blogging/tests/middleware_spec.js

@@ -1,14 +1,15 @@
 /*globals describe, beforeEach, afterEach, it*/
-var crypto          = require('crypto'),
-    should          = require('should'),
-    sinon           = require('sinon'),
-    Promise         = require('bluebird'),
+var should = require('should'), // jshint ignore:line
+    sinon = require('sinon'),
+    crypto = require('crypto'),
+    Promise = require('bluebird'),
+    api = require('../../../api'),
+    errors = require('../../../errors'),
+    fs = require('fs'),
+
     privateBlogging = require('../lib/middleware'),
-    api             = require('../../../api'),
-    errors          = require('../../../errors'),
-    fs              = require('fs');
 
-should.equal(true, true);
+    sandbox = sinon.sandbox.create();
 
 function hash(password, salt) {
     var hasher = crypto.createHash('sha256');
@@ -19,12 +20,7 @@ function hash(password, salt) {
 }
 
 describe('Private Blogging', function () {
-    var sandbox,
-        apiSettingsStub;
-
-    beforeEach(function () {
-        sandbox = sinon.sandbox.create();
-    });
+    var apiSettingsStub;
 
     afterEach(function () {
         sandbox.restore();
@@ -98,8 +94,10 @@ describe('Private Blogging', function () {
                     status: function () {
                         return this;
                     },
-                    send: function () {},
-                    set: function () {},
+                    send: function () {
+                    },
+                    set: function () {
+                    },
                     isPrivateBlog: true
                 };
             });

core/test/unit/apps/subscribers/lib/router_spec.js → core/server/apps/subscribers/tests/router_spec.js

@@ -1,5 +1,5 @@
 var should = require('should'),
-    router = require('../../../../../server/apps/subscribers/lib/router');
+    router = require('../lib/router');
 
 describe('UNIT: Apps Subscriber Router', function () {
     it('[failure] email is invalid, ensure it`s sanitized', function (done) {

core/test/functional/module/module_spec.js

@@ -1,9 +1,9 @@
 // # Module tests
 // This tests using Ghost as an npm module
-var should     = require('should'),
-    testUtils  = require('../../utils'),
-    ghost      = testUtils.startGhost,
-    i18n       = require('../../../../core/server/i18n');
+var should = require('should'),
+    testUtils = require('../../utils'),
+    ghost = testUtils.startGhost,
+    i18n = require('../../../../core/server/i18n');
 
 i18n.init();
 

core/test/functional/routes/admin_spec.js

@@ -3,12 +3,13 @@
 // Mocking out the models to not touch the DB would turn these into unit tests, and should probably be done in future,
 // But then again testing real code, rather than mock code, might be more useful...
 
-var request = require('supertest'),
-    should = require('should'),
+var should = require('should'),
+    supertest = require('supertest'),
     testUtils = require('../../utils'),
     ghost = testUtils.startGhost,
     i18n = require('../../../../core/server/i18n'),
-    config = require('../../../../core/server/config');
+    config = require('../../../../core/server/config'),
+    request;
 
 i18n.init();
 
@@ -49,7 +50,7 @@ describe('Admin Routing', function () {
                 ghostServer = _ghostServer;
                 return ghostServer.start();
             }).then(function () {
-                request = request(config.get('url'));
+                request = supertest.agent(config.get('url'));
                 done();
             }).catch(function (e) {
                 console.log('Ghost Error: ', e);
@@ -135,7 +136,7 @@ describe('Admin Routing', function () {
     describe('FORK', function () {
         // we'll use X-Forwarded-Proto: https to simulate an 'https://' request behind a proxy
         describe('Require HTTPS - redirect', function () {
-            var forkedGhost, request;
+            var forkedGhost;
 
             before(function (done) {
                 testUtils.fork.ghost({
@@ -146,8 +147,7 @@ describe('Admin Routing', function () {
                 }, 'testhttps')
                     .then(function (child) {
                         forkedGhost = child;
-                        request = require('supertest');
-                        request = request('http://localhost:2390');
+                        request = supertest.agent('http://localhost:2390');
                     }).then(done)
                     .catch(done);
             });

core/test/functional/routes/api/authentication_spec.js

@@ -1,13 +1,13 @@
-var supertest     = require('supertest'),
-    should        = require('should'),
-    moment        = require('moment'),
-    testUtils     = require('../../../utils'),
-    user          = testUtils.DataGenerator.forModel.users[0],
-    userForKnex   = testUtils.DataGenerator.forKnex.users[0],
-    models        = require('../../../../../core/server/models'),
-    config        = require('../../../../../core/server/config'),
-    utils         = require('../../../../../core/server/utils'),
-    ghost         = testUtils.startGhost,
+var should = require('should'),
+    supertest = require('supertest'),
+    testUtils = require('../../../utils'),
+    moment = require('moment'),
+    user = testUtils.DataGenerator.forModel.users[0],
+    userForKnex = testUtils.DataGenerator.forKnex.users[0],
+    models = require('../../../../../core/server/models'),
+    config = require('../../../../../core/server/config'),
+    utils = require('../../../../../core/server/utils'),
+    ghost = testUtils.startGhost,
     request;
 
 describe('Authentication API', function () {

core/test/functional/routes/api/configuration_spec.js

@@ -1,8 +1,8 @@
-var testUtils     = require('../../../utils'),
-    should        = require('should'),
-    supertest     = require('supertest'),
-    config        = require('../../../../../core/server/config'),
-    ghost         = testUtils.startGhost,
+var should = require('should'),
+    supertest = require('supertest'),
+    testUtils = require('../../../utils'),
+    config = require('../../../../../core/server/config'),
+    ghost = testUtils.startGhost,
     request;
 
 describe('Configuration API', function () {

core/test/functional/routes/api/db_spec.js

@@ -1,9 +1,9 @@
-var supertest     = require('supertest'),
-    should        = require('should'),
-    path          = require('path'),
-    testUtils     = require('../../../utils'),
-    config        = require('../../../../../core/server/config'),
-    ghost         = testUtils.startGhost,
+var should = require('should'),
+    supertest = require('supertest'),
+    testUtils = require('../../../utils'),
+    path = require('path'),
+    config = require('../../../../../core/server/config'),
+    ghost = testUtils.startGhost,
     request;
 
 describe('DB API', function () {

core/test/functional/routes/api/error_spec.js

@@ -2,12 +2,11 @@
 // As it stands, these tests depend on the database, and as such are integration tests.
 // Mocking out the models to not touch the DB would turn these into unit tests, and should probably be done in future,
 // But then again testing real code, rather than mock code, might be more useful...
-
-var supertest     = require('supertest'),
-    should        = require('should'),
-    testUtils     = require('../../../utils'),
-    config        = require('../../../../../core/server/config'),
-    ghost         = testUtils.startGhost,
+var should = require('should'),
+    supertest = require('supertest'),
+    testUtils = require('../../../utils'),
+    config = require('../../../../../core/server/config'),
+    ghost = testUtils.startGhost,
     request;
 
 require('should-http');

core/test/functional/routes/api/notifications_spec.js

@@ -1,8 +1,8 @@
-var testUtils     = require('../../../utils'),
-    supertest     = require('supertest'),
-    should        = require('should'),
-    config        = require('../../../../../core/server/config'),
-    ghost         = testUtils.startGhost,
+var should = require('should'),
+    supertest = require('supertest'),
+    testUtils = require('../../../utils'),
+    config = require('../../../../../core/server/config'),
+    ghost = testUtils.startGhost,
     request;
 
 describe('Notifications API', function () {

core/test/functional/routes/api/posts_spec.js

@@ -1,10 +1,10 @@
-var testUtils     = require('../../../utils'),
-    should        = require('should'),
-    supertest     = require('supertest'),
-    _             = require('lodash'),
-    ObjectId      = require('bson-objectid'),
-    config        = require('../../../../../core/server/config'),
-    ghost         = testUtils.startGhost,
+var should = require('should'),
+    supertest = require('supertest'),
+    testUtils = require('../../../utils'),
+    _ = require('lodash'),
+    ObjectId = require('bson-objectid'),
+    config = require('../../../../../core/server/config'),
+    ghost = testUtils.startGhost,
     request;
 
 describe('Post API', function () {

core/test/functional/routes/api/public_api_spec.js

@@ -1,9 +1,9 @@
-var testUtils     = require('../../../utils'),
-    should        = require('should'),
-    supertest     = require('supertest'),
-    _             = require('lodash'),
-    config        = require('../../../../../core/server/config'),
-    ghost         = testUtils.startGhost,
+var should = require('should'),
+    supertest = require('supertest'),
+    testUtils = require('../../../utils'),
+    _ = require('lodash'),
+    config = require('../../../../../core/server/config'),
+    ghost = testUtils.startGhost,
     request;
 
 describe('Public API', function () {

core/test/functional/routes/api/settings_spec.js

@@ -1,8 +1,8 @@
-var testUtils     = require('../../../utils'),
-    should        = require('should'),
-    supertest     = require('supertest'),
-    config        = require('../../../../../core/server/config'),
-    ghost         = testUtils.startGhost,
+var should = require('should'),
+    supertest = require('supertest'),
+    testUtils = require('../../../utils'),
+    config = require('../../../../../core/server/config'),
+    ghost = testUtils.startGhost,
     request;
 
 describe('Settings API', function () {