From e59b1aca890750cd1e5c4cdcbc7cba4545d8e272 Mon Sep 17 00:00:00 2001 From: Matt Mazzola Date: Mon, 22 Aug 2016 10:08:49 -0700 Subject: [PATCH] Setup tslint on build and test to enforce good practices and consistent style. --- gulpfile.js | 21 ++++++++++++- package.json | 2 ++ src/router.ts | 34 ++++++++++----------- test/router.spec.ts | 73 ++++++++++++++++++++++----------------------- tslint.json | 25 ++++++++++++++++ 5 files changed, 99 insertions(+), 56 deletions(-) create mode 100644 tslint.json diff --git a/gulpfile.js b/gulpfile.js index 8667754..a99478d 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -6,6 +6,7 @@ var del = require('del'), moment = require('moment'), rename = require('gulp-rename'), replace = require('gulp-replace'), + tslint = require('gulp-tslint'), typedoc = require("gulp-typedoc"), uglify = require('gulp-uglify'), karma = require('karma'), @@ -23,6 +24,7 @@ var gulpBanner = "/*! " + webpackBanner + " */\n"; gulp.task('build', 'Build for release', function (done) { return runSequence( + 'tslint:build', 'clean:dist', 'compile:ts', 'min', @@ -34,6 +36,7 @@ gulp.task('build', 'Build for release', function (done) { gulp.task('test', 'Run unit tests', function (done) { return runSequence( + 'tslint:test', 'clean:tmp', 'compile:spec', 'test:spec', @@ -139,4 +142,20 @@ gulp.task('test:spec', 'Runs spec tests', function(done) { singleRun: argv.watch ? false : true, captureTimeout: argv.timeout || 20000 }, done); -}); \ No newline at end of file +}); + +gulp.task('tslint:build', 'Run TSLint on src', function () { + return gulp.src(["src/**/*.ts"]) + .pipe(tslint({ + formatter: "verbose" + })) + .pipe(tslint.report()); +}); + +gulp.task('tslint:test', 'Run TSLint on src and tests', function () { + return gulp.src(["src/**/*.ts", "test/**/*.ts"]) + .pipe(tslint({ + formatter: "verbose" + })) + .pipe(tslint.report()); +}); diff --git a/package.json b/package.json index db5df4e..e3bf98d 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "gulp-help": "^1.6.1", "gulp-rename": "^1.2.2", "gulp-replace": "^0.5.4", + "gulp-tslint": "^6.1.1", "gulp-typedoc": "^2.0.0", "gulp-uglify": "^1.5.3", "jasmine-core": "^2.4.1", @@ -46,6 +47,7 @@ "phantomjs-prebuilt": "^2.1.7", "run-sequence": "^1.2.0", "ts-loader": "^0.8.2", + "tslint": "^3.15.0", "typedoc": "^0.4.4", "typescript": "^1.8.10", "typings": "^1.3.2", diff --git a/src/router.ts b/src/router.ts index c89e94a..ba4a7cc 100644 --- a/src/router.ts +++ b/src/router.ts @@ -15,10 +15,10 @@ export class Router { private postRouteRecognizer: RouteRecognizer; private putRouteRecognizer: RouteRecognizer; private deleteRouteRecognizer: RouteRecognizer; - + constructor(handlers: IAddHandler) { this.handlers = handlers; - + /** * TODO: look at generating the router dynamically based on list of supported http methods * instead of hardcoding the creation of these and the methods. @@ -29,32 +29,32 @@ export class Router { this.putRouteRecognizer = new RouteRecognizer(); this.deleteRouteRecognizer = new RouteRecognizer(); } - + get(url: string, handler: IRouterHandler): this { this.registerHandler(this.getRouteRecognizer, "GET", url, handler); return this; } - + patch(url: string, handler: IRouterHandler): this { this.registerHandler(this.patchRouteRecognizer, "PATCH", url, handler); return this; } - + post(url: string, handler: IRouterHandler): this { this.registerHandler(this.postRouteRecognizer, "POST", url, handler); return this; } - + put(url: string, handler: IRouterHandler): this { this.registerHandler(this.putRouteRecognizer, "PUT", url, handler); return this; } - + delete(url: string, handler: IRouterHandler): this { this.registerHandler(this.deleteRouteRecognizer, "DELETE", url, handler); return this; } - + /** * TODO: This method could use some refactoring. There is conflict of interest between keeping clean separation of test and handle method * Test method only returns boolean indicating if request can be handled, and handle method has opportunity to modify response and return promise of it. @@ -71,7 +71,7 @@ export class Router { routeRecognizer.add([ { path: url, handler: routeRecognizerHandler } ]); - + const internalHandler = { test(request: IRequest): boolean { if (request.method !== method) { @@ -79,10 +79,10 @@ export class Router { } const matchingRoutes = routeRecognizer.recognize(request.url); - if(matchingRoutes === undefined) { + if (matchingRoutes === undefined) { return false; } - + /** * Copy parameters from recognized route to the request so they can be used within the handler function * This isn't ideal because it is side affect which modifies the request instead of strictly testing for true or false @@ -93,14 +93,14 @@ export class Router { (request).params = route.params; (request).queryParams = (matchingRoutes).queryParams; (request).handler = route.handler; - + return true; }, handle(request: IExtendedRequest): Promise { return request.handler(request); } }; - + this.handlers.addHandler(internalHandler); } } @@ -113,7 +113,7 @@ export interface IExtendedRequest extends IRequest { export interface IRequest { method: "GET" | "POST" | "PUT" | "DELETE"; - url: string, + url: string; headers: { [key: string]: string }; body: any; } @@ -128,15 +128,15 @@ export class Response implements IResponse { statusCode: number; headers: any; body: any; - + constructor() { this.statusCode = 200; this.headers = {}; this.body = null; } - + send(statusCode: number, body?: any) { this.statusCode = statusCode; this.body = body; } -} \ No newline at end of file +} diff --git a/test/router.spec.ts b/test/router.spec.ts index 11a70c4..552f6d2 100644 --- a/test/router.spec.ts +++ b/test/router.spec.ts @@ -9,7 +9,6 @@ describe('router', function () { addHandler: jasmine.createSpy("spyHandler").and.callFake((handler: any) => { wpmpStub.handlers.push(handler); }), - simulateReceiveMessage(message: any) { wpmpStub.handlers.some(handler => { wpmpStub.testSpy(message); @@ -22,8 +21,7 @@ describe('router', function () { }); } }; - - + describe('common', function () { let router: Router.Router; const testUrl = '/report/pages'; @@ -31,26 +29,26 @@ describe('router', function () { response.send(200, { params: request.params }); }; let internalHandler: any; - + beforeAll(function () { wpmpStub.handlers.length = 0; router = new Router.Router(wpmpStub); router.get(testUrl, testHandler); }); - + beforeEach(function () { internalHandler = wpmpStub.addHandler.calls.mostRecent().args[0]; }); - + afterEach(function () { wpmpStub.testSpy.calls.reset(); wpmpStub.handleSpy.calls.reset(); }); - + afterAll(function () { wpmpStub.addHandler.calls.reset(); }); - + it('the handle method will always return a promise', function () { // Arrange const testData = { @@ -59,17 +57,17 @@ describe('router', function () { url: testUrl } }; - + // Act wpmpStub.simulateReceiveMessage(testData.request); - + // Assert const handleReturnValue: any = wpmpStub.handleResponseSpy.calls.mostRecent().args[0]; expect(wpmpStub.testSpy).toHaveBeenCalledWith(testData.request); expect(wpmpStub.handleSpy).toHaveBeenCalledWith(testData.request); expect(handleReturnValue.then).toBeDefined(); }); - + it('the handle method will call the given handler with request and response object so the response may be modified', function (done) { // Arrange const testData = { @@ -78,10 +76,10 @@ describe('router', function () { url: testUrl } }; - + // Act wpmpStub.simulateReceiveMessage(testData.request); - + // Assert const handleReturnValue: any = wpmpStub.handleResponseSpy.calls.mostRecent().args[0]; expect(wpmpStub.testSpy).toHaveBeenCalled(); @@ -92,12 +90,12 @@ describe('router', function () { done(); }); }); - + it('calling any method on the router instance always returns itself to allow chaining', function () { const returnValue = router.get("abc", testHandler); expect(returnValue).toBe(router); }); - + it('wildcard route can be added', function () { // Arrange router.get('*path', testHandler); @@ -112,16 +110,16 @@ describe('router', function () { path: testData.request.url } }; - + // Act wpmpStub.simulateReceiveMessage(testData.request); - + // Assert expect(wpmpStub.testSpy).toHaveBeenCalledWith(testData.request); expect(wpmpStub.handleSpy).toHaveBeenCalledWith(jasmine.objectContaining(expectedRequest)); }); }); - + describe('http methods', function () { let router: Router.Router; let testUrl = '/report/pages/:pageName'; @@ -129,31 +127,30 @@ describe('router', function () { response.send(200, { params: request.params }); }; let internalHandler: any; - - + beforeAll(function () { wpmpStub.handlers.length = 0; router = new Router.Router(wpmpStub); router.get(testUrl, testHandler); }); - + beforeEach(function () { internalHandler = wpmpStub.addHandler.calls.mostRecent().args[0]; }); - + afterEach(function () { wpmpStub.testSpy.calls.reset(); wpmpStub.handleSpy.calls.reset(); }); - + afterAll(function () { wpmpStub.addHandler.calls.reset(); }); - + it('calling router[method](path, handler) registers handler on the handlers object', function () { expect(wpmpStub.addHandler).toHaveBeenCalled(); }); - + it('the test method will return FALSE if the request.method does not match', function () { // Arrange const testPageName = 'customPage'; @@ -163,13 +160,13 @@ describe('router', function () { url: `/report/pages/${testPageName}` } }; - + // Act - + // Assert expect(internalHandler.test(testData.request)).toEqual(false); }); - + it('the test method will return TRUE if the request method and the url matches a pattern', function () { // Arrange const testPageName = 'customPage'; @@ -180,13 +177,13 @@ describe('router', function () { url: `/report/pages/${testPageName}?title=${testPageTitle}` } }; - + // Act - + // Assert expect(internalHandler.test(testData.request)).toEqual(true); }); - + it('the test method will add the path parameters and query parameters to the request if the request method and the url matches the pattern', function () { // Arrange const testPageName = 'customPage'; @@ -204,16 +201,16 @@ describe('router', function () { myParameter: 'xyz' } }; - + // Act wpmpStub.simulateReceiveMessage(testData.request); - + // Assert - const handleReturnValue: any = wpmpStub.handleResponseSpy.calls.mostRecent().args[0]; + // const handleReturnValue: any = wpmpStub.handleResponseSpy.calls.mostRecent().args[0]; expect(wpmpStub.testSpy).toHaveBeenCalledWith(testData.request); expect(wpmpStub.handleSpy).toHaveBeenCalledWith(jasmine.objectContaining(expectedRequest)); }); - + it('route recognizers are split by http method so two paths with different methods will not conflict', function () { // Arrange router.delete('/report/pages/:pageName', testHandler); @@ -229,11 +226,11 @@ describe('router', function () { pageName: testPageName } }; - const secondInternalHandler: any = wpmpStub.addHandler.calls.mostRecent().args[0]; - + // const secondInternalHandler: any = wpmpStub.addHandler.calls.mostRecent().args[0]; + // Act wpmpStub.simulateReceiveMessage(testData.request); - + // Assert expect(wpmpStub.addHandler).toHaveBeenCalled(); expect(wpmpStub.testSpy).toHaveBeenCalledWith(testData.request); diff --git a/tslint.json b/tslint.json new file mode 100644 index 0000000..6bc5a55 --- /dev/null +++ b/tslint.json @@ -0,0 +1,25 @@ +{ + "extends": "tslint:recommended", + "rules": { + "max-line-length": false, + "member-access": false, + "one-line": [ + "check-whitespace", + "check-open-brace" + ], + "quotemark": [ + "single", + "avoid-escape" + ], + "trailing-comma": false, + "whitespace": [ + "check-branch", + "check-decl", + "check-operator", + "check-module", + "check-seperator", + "check-type" + ], + "object-literal-sort-keys": false + } +} \ No newline at end of file