chore(tests): [RO-26598] Improve test coverage (#83)

* chore(tests): [RO-26598] Imporoved test coverage

* chore(tests): [RO-26598] Updated .vscode/settings.json to disable Prettier instead of disabling formatOnSave

* chore(tests): [RO-26598] Added test for express-device

* chore(tests): [RO-26598] Added option for express-device to parse user agent so it returns a value for the name field

* chore(tests): [RO-26598] Addressed PR Feedback

* chore(tests): [RO-26598] Updated coverage threshold

* chore(tests): [RO-26598] Update to make it clear the view engine is a mock

.editorconfig

@@ -1,12 +1,14 @@
 root = true
 
 [*]
-indent_style = space
-indent_size = 2
-end_of_line = lf
 charset = utf-8
-trim_trailing_whitespace = true
+end_of_line = lf
+indent_size = 2
+indent_style = space
 insert_final_newline = true
+max_line_length = 120
+quote_type = single
+trim_trailing_whitespace = true
 
 [node_modules/**.js]
 codepaint = false

.vscode/settings.json

@@ -0,0 +1,3 @@
+{
+  "prettier.enable": false,
+}

jest.config.js

@@ -6,14 +6,14 @@ module.exports = {
   coveragePathIgnorePatterns: ['/node_modules/'],
   coverageThreshold: {
     global: {
-      branches: 62,
-      functions: 84,
-      lines: 89,
-      statements: 89
-    }
+      branches: 95,
+      functions: 98,
+      lines: 98,
+      statements: 98,
+    },
   },
   maxWorkers: 1,
-  moduleFileExtensions: ['js','json'],
+  moduleFileExtensions: ['js', 'json'],
   rootDir: '.',
   testEnvironment: 'node',
   testMatch: ['**/*.spec.js'],

src/middleware/DefaultMiddleware.js

@@ -14,7 +14,7 @@ module.exports = function (
       this.app.use(bodyParser.urlencoded({ extended: false }));
       this.app.use(bodyParser.json());
       this.app.use(methodOverride());
-      this.app.use(expressDevice.capture());
+      this.app.use(expressDevice.capture({ parseUserAgent: true }));
     }
 
   }

src/middleware/ErrorMiddleware.js

@@ -1,14 +1,8 @@
 const _some = require('lodash.some');
 const _assignIn = require('lodash.assignin');
 
-module.exports = function (
-  SpurErrors,
-  Logger,
-  HtmlErrorRender,
-  BaseMiddleware
-) {
+module.exports = function (SpurErrors, Logger, HtmlErrorRender, BaseMiddleware) {
   class ErrorMiddleware extends BaseMiddleware {
-
     configure(app) {
       super.configure(app);
 
@@ -34,29 +28,29 @@ module.exports = function (
       res.format({
         text: () => this.sendTextResponse(err, req, res),
         html: () => this.sendHtmlResponse(err, req, res),
-        json: () => this.sendJsonResponse(err, req, res)
+        json: () => this.sendJsonResponse(err, req, res),
       });
 
       next();
     }
 
-    logErrorStack(err) {
+    logErrorStack(error) {
+      const err = error ?? {};
       const statusCode = err.statusCode || 0;
       const checkStatus = (status) => status === statusCode;
 
       if (!_some(this.EXCLUDE_STATUSCODE_FROM_LOGS, checkStatus)) {
-        Logger.error(err, '\n', err.stack, '\n', (err.data || ''));
+        Logger.error(err, '\n', err?.stack ?? '', '\n', err?.data ?? '');
       }
     }
 
     appendRequestData(err, req) {
-      if (err.data == null) {
-        err.data = {};
-      }
-
-      err.data = _assignIn(err.data, {
-        url: req.url
-      });
+      err.data = _assignIn(
+        { ...err.data },
+        {
+          url: req.url,
+        },
+      );
     }
 
     sendTextResponse(err, req, res) {

src/middleware/PromiseMiddleware.js

@@ -1,10 +1,5 @@
-module.exports = function (
-  Promise,
-  BaseMiddleware
-) {
-
+module.exports = function (Promise, BaseMiddleware) {
   class PromiseMiddleware extends BaseMiddleware {
-
     configure(app) {
       super.configure(app);
 
@@ -14,8 +9,8 @@ module.exports = function (
           .catch(this.req.next);
       };
 
-      this.app.response.renderAsync = function (view, properties = {}) {
-        return Promise.props(properties)
+      this.app.response.renderAsync = function (view, properties) {
+        return Promise.props(properties ?? {})
           .then((props) => this.render(view, props))
           .catch(this.req.next);
       };
@@ -42,7 +37,7 @@ module.exports = function (
               },
               json: () => {
                 this.json(results);
-              }
+              },
             });
           })
           .catch(this.req.next);

src/webserver/BaseWebServer.js

@@ -7,22 +7,15 @@ module.exports = function (
   ErrorMiddleware,
   config,
   ControllerRegistration,
-  WinstonRequestLoggingMiddleware
+  WinstonRequestLoggingMiddleware,
 ) {
   class BaseWebServer {
-
     constructor() {
       this.app = express();
     }
 
     getPort() {
-      let port = config.Port;
-
-      if (this.server) {
-        port = this.server.address().port || port;
-      }
-
-      return port;
+      return this.server?.address()?.port ?? config.Port;
     }
 
     registerDefaultMiddleware() {
@@ -88,19 +81,20 @@ module.exports = function (
     }
 
     getCloseAsync() {
-      if (this.server && this.server.closeAsync) {
+      if (this.server?.closeAsync) {
         return this.server.closeAsync();
       }
 
       return Promise.resolve();
     }
 
     startedMessage() {
+      const port = this.getPort();
       if (this.cluster) {
-        return `Worker ${this.cluster.worker.id} started on port ${this.getPort()}`;
+        return `Worker ${this.cluster.worker.id} started on port ${port}`;
       }
 
-      return `Express app started on port ${this.getPort()}`;
+      return `Express app started on port ${port}`;
     }
 
     logSectionHeader(message) {

test/fixtures/controllers/MockController.js

@@ -3,9 +3,13 @@ module.exports = function (BaseController) {
   class MockController extends BaseController {
     configure(app) {
       super.configure(app);
-      app.get("/", (req, res) => { res.status(200).send("SomeIndex") });
+      app.get("/", this.handleIndexRoute);
       app.get("/with-error", (req, res) => { throw new Error("Throwing a basic error") });
     }
+
+    handleIndexRoute(req, res) {
+      res.status(200).send("SomeIndex");
+    }
   }
 
   return new MockController();

test/fixtures/controllers/PromiseMiddlewareTestController.js

@@ -1,9 +1,24 @@
-module.exports = function (BaseController, Promise) {
+const fs = require('fs');
 
+module.exports = function (BaseController, Promise) {
   class PromiseMiddlewareTestController extends BaseController {
-
     configure(app) {
       super.configure(app);
+
+      const mockOpenTableTemplateViewEngine = (filePath, options, callback) => {
+        fs.readFile(filePath, (err, content) => {
+          const { settings, _locals, cache, ...vars } = options;
+          let rendered = content.toString();
+          Object.entries(vars).forEach(([placeholder, value]) => {
+            rendered = rendered.replace(`{{${placeholder}}}`, value);
+          });
+          return callback(null, rendered);
+        });
+      };
+
+      app.engine('ott', mockOpenTableTemplateViewEngine);
+      app.set('views', `${__dirname}/views`);
+
       app.get('/promise-middleware-test--jsonasync', this.getJsonAsync);
       app.get('/promise-middleware-test--renderasync', this.getRenderAsync);
       app.get('/promise-middleware-test--sendAsync', this.getSendAsync);
@@ -16,7 +31,15 @@ module.exports = function (BaseController, Promise) {
     }
 
     getRenderAsync(req, res) {
-      res.renderAsync('renderView', Promise.resolve({ microapp: 'renderAsync success' }));
+      const { headers } = req;
+      const view = headers['x-view'];
+      const viewPropsHeader = headers['x-view-props'];
+      const viewProps = viewPropsHeader !== 'undefined' ? JSON.parse(viewPropsHeader) : undefined;
+      if (viewProps) {
+        res.renderAsync(view, Promise.resolve(viewProps));
+      } else {
+        res.renderAsync(view);
+      }
     }
 
     getSendAsync(req, res) {
@@ -28,7 +51,7 @@ module.exports = function (BaseController, Promise) {
     }
 
     getFormatAsync(req, res) {
-      res.formatAsync(Promise.resolve('formatAsync success'));
+      res.formatAsync('innerHTML', Promise.resolve('formatAsync success'));
     }
   }
 

test/fixtures/controllers/views/home.ott

@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <title>Testing</title>
+</head>
+<body>
+  No render view-props provided
+</body>
+</html>

test/fixtures/controllers/views/microapp.ott

@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <title>Testing</title>
+</head>
+<body>
+  {{microapp}}
+</body>
+</html>

test/unit/middleware/ErrorMiddleware.spec.js

@@ -2,8 +2,7 @@ describe('ErrorMiddleware', function () {
   let htmlErrorRenderRenderSpy, errorMiddlewareSendTextResponseSpy, errorMiddlewareSendHtmlResponseSpy, errorMiddlewareSendJsonResponseSpy, loggerErrorSpy;
 
   beforeEach(() => {
-    injector().inject((ErrorMiddleware, HTTPService,
-      TestWebServer, HtmlErrorRender, Logger, config) => {
+    injector().inject((ErrorMiddleware, HTTPService, TestWebServer, HtmlErrorRender, Logger, config) => {
       this.ErrorMiddleware = ErrorMiddleware;
       this.HTTPService = HTTPService;
       this.TestWebServer = TestWebServer;
@@ -13,6 +12,8 @@ describe('ErrorMiddleware', function () {
 
       this.mockPort = 9080;
 
+      Logger.useNoop();
+
       htmlErrorRenderRenderSpy = jest.spyOn(this.HtmlErrorRender, 'render');
       errorMiddlewareSendTextResponseSpy = jest.spyOn(this.ErrorMiddleware, 'sendTextResponse');
       errorMiddlewareSendHtmlResponseSpy = jest.spyOn(this.ErrorMiddleware, 'sendHtmlResponse');
@@ -46,7 +47,7 @@ describe('ErrorMiddleware', function () {
           expect.any(String),
           expect.any(String),
           expect.any(String),
-          { url: expectUrl }
+          { url: expectUrl },
         );
       };
     });
@@ -164,4 +165,11 @@ describe('ErrorMiddleware', function () {
       });
     });
   });
+
+  it("should not throw an error when logErrorStack is called without error argument and log 'empty' error", () => {
+    this.ErrorMiddleware.logErrorStack();
+
+    expect(loggerErrorSpy).toHaveBeenCalledTimes(1);
+    expect(loggerErrorSpy).toHaveBeenCalledWith({}, '\n', '', '\n', '');
+  });
 });

test/unit/middleware/NoCacheMiddleware.spec.js

@@ -1,5 +1,4 @@
 describe('NoCacheMiddleware', function () {
-
   beforeEach(() => {
     return injector().inject((NoCacheMiddleware) => {
       this.NoCacheMiddleware = NoCacheMiddleware;
@@ -9,4 +8,21 @@ describe('NoCacheMiddleware', function () {
   it('should exist', () => {
     expect(this.NoCacheMiddleware).toBeDefined();
   });
+
+  it('should set no-cache headers', () => {
+    const response = {
+      headers: { Expires: 5184000 },
+      header: (name, value) => {
+        response.headers[name] = value;
+      },
+    };
+
+    this.NoCacheMiddleware({}, response, () => {});
+
+    expect(response.headers).toStrictEqual({
+      'Cache-Control': 'no-cache, no-store, must-revalidate',
+      Pragma: 'no-cache',
+      Expires: 0,
+    });
+  });
 });