refactor(server): refactor bundleResource in lib/server.js (#3029)

karma-runner · Jun 8, 2018 · 011a90c · 011a90c
1 parent c1a9567
commit 011a90c
Show file tree

Hide file tree

Showing 9 changed files with 156 additions and 82 deletions.
diff --git a/.eslintignore b/.eslintignore
@@ -1,4 +1,5 @@
 test/e2e/support/error/under-test.js
+test/unit/fixtures/bundled.js
 static/karma.js
 static/context.js
 tmp/*
diff --git a/.gitignore b/.gitignore
@@ -15,4 +15,5 @@ test/e2e/coverageRequirejs/coverage
 test/e2e/coffee-coverage/coverage
 test-results.xml
 test/unit/test.log
+test/unit/fixtures/bundled.js
 .DS_Store
diff --git a/lib/server.js b/lib/server.js
@@ -8,6 +8,7 @@ const spawn = require('child_process').spawn
 const tmp = require('tmp')
 const fs = require('fs')
 const path = require('path')
+const BundleUtils = require('./utils/bundle-utils')
 const root = global || window || this
 
 const cfg = require('./config')
@@ -31,20 +32,6 @@ const BrowserCollection = require('./browser_collection')
 const EmitterWrapper = require('./emitter_wrapper')
 const processWrapper = new EmitterWrapper(process)
 
-const karmaJsPath = path.join(__dirname, '/../static/karma.js')
-const contextJsPath = path.join(__dirname, '/../static/context.js')
-
-function bundleResource (inPath, outPath) {
-  const browserify = require('browserify')
-  return new Promise((resolve, reject) => {
-    browserify(inPath)
-      .bundle()
-      .pipe(fs.createWriteStream(outPath))
-      .once('finish', () => resolve())
-      .once('error', (e) => reject(e))
-  })
-}
-
 function createSocketIoServer (webServer, executor, config) {
   const server = new SocketIO(webServer, {
     // avoid destroying http upgrades from socket.io to get proxied websockets working
@@ -152,39 +139,32 @@ class Server extends KarmaEventEmitter {
         this._injector.invoke(watcher.watch)
       }
 
-      const startWebServer = () => {
-        webServer.listen(config.port, config.listenAddress, () => {
-          this.log.info(`Karma v${constant.VERSION} server started at ${config.protocol}//${config.listenAddress}:${config.port}${config.urlRoot}`)
-
-          this.emit('listening', config.port)
-          if (config.browsers && config.browsers.length) {
-            this._injector.invoke(launcher.launch, launcher).forEach((browserLauncher) => {
-              singleRunDoneBrowsers[browserLauncher.id] = false
-            })
-          }
-          if (this.loadErrors.length > 0) {
-            this.log.error('Found %d load error%s', this.loadErrors.length, this.loadErrors.length === 1 ? '' : 's')
-            process.exitCode = 1
-            process.kill(process.pid, 'SIGINT')
-          }
-        })
-      }
-
-      if (fs.existsSync(karmaJsPath) && fs.existsSync(contextJsPath)) {
-        startWebServer()
-      } else {
-        this.log.info('Front-end scripts not present. Compiling...')
-        Promise.all([
-          bundleResource(path.join(__dirname, '/../client/main.js'), karmaJsPath),
-          bundleResource(path.join(__dirname, '/../context/main.js'), contextJsPath)
-        ])
-          .then(startWebServer)
-          .catch((error) => {
-            this.log.error('Front-end script compile failed with error: ' + error)
-            process.exitCode = 1
-            process.kill(process.pid, 'SIGINT')
+      return Promise.all([
+        BundleUtils.bundleResourceIfNotExist('client/main.js', 'static/karma.js'),
+        BundleUtils.bundleResourceIfNotExist('context/main.js', 'static/context.js')
+      ])
+        .then(() => {
+          webServer.listen(config.port, config.listenAddress, () => {
+            this.log.info(`Karma v${constant.VERSION} server started at ${config.protocol}//${config.listenAddress}:${config.port}${config.urlRoot}`)
+
+            this.emit('listening', config.port)
+            if (config.browsers && config.browsers.length) {
+              this._injector.invoke(launcher.launch, launcher).forEach((browserLauncher) => {
+                singleRunDoneBrowsers[browserLauncher.id] = false
+              })
+            }
+            if (this.loadErrors.length > 0) {
+              this.log.error('Found %d load error%s', this.loadErrors.length, this.loadErrors.length === 1 ? '' : 's')
+              process.exitCode = 1
+              process.kill(process.pid, 'SIGINT')
+            }
           })
-      }
+        })
+        .catch((error) => {
+          this.log.error('Front-end script compile failed with error: ' + error)
+          process.exitCode = 1
+          process.kill(process.pid, 'SIGINT')
+        })
     }
 
     fileList.refresh().then(afterPreprocess, afterPreprocess)

diff --git a/lib/utils/bundle-utils.js b/lib/utils/bundle-utils.js
@@ -0,0 +1,27 @@
+'use strict'
+const PathUtils = require('./path-utils')
+const fs = require('fs')
+const Promise = require('bluebird')
+
+const BundleUtils = {
+  bundleResource (inPath, outPath) {
+    return new Promise((resolve, reject) => {
+      require('browserify')(inPath)
+        .bundle()
+        .pipe(fs.createWriteStream(outPath))
+        .once('finish', () => resolve())
+        .once('error', (e) => reject(e))
+    })
+  },
+
+  bundleResourceIfNotExist (inPath, outPath) {
+    inPath = PathUtils.calculateAbsolutePath(inPath)
+    outPath = PathUtils.calculateAbsolutePath(outPath)
+
+    return fs.existsSync(outPath)
+      ? Promise.resolve()
+      : BundleUtils.bundleResource(inPath, outPath)
+  }
+}
+
+module.exports = BundleUtils
diff --git a/lib/utils/file-utils.js b/lib/utils/file-utils.js
@@ -13,6 +13,12 @@ const FileUtils = {
 
   copyFile (src, dest) {
     FileUtils.saveFile(dest, FileUtils.readFile(src))
+  },
+
+  removeFileIfExists (src) {
+    if (fs.existsSync(src)) {
+      fs.unlinkSync(src)
+    }
   }
 }
 

diff --git a/lib/utils/path-utils.js b/lib/utils/path-utils.js
@@ -1,9 +1,16 @@
 'use strict'
 
+const path = require('path')
+
 const PathUtils = {
   formatPathMapping (path, line, column) {
     return path + (line ? `:${line}` : '') + (column ? `:${column}` : '')
+  },
+
+  calculateAbsolutePath (karmaRelativePath) {
+    return path.join(__dirname, '..', '..', karmaRelativePath)
   }
+
 }
 
 module.exports = PathUtils
diff --git a/test/unit/server.spec.js b/test/unit/server.spec.js
@@ -1,7 +1,6 @@
 var Server = require('../../lib/server')
+var BundleUtils = require('../../lib/utils/bundle-utils')
 var BrowserCollection = require('../../lib/browser_collection')
-var fs = require('fs')
-var path = require('path')
 
 describe('server', () => {
   var mockConfig
@@ -110,69 +109,74 @@ describe('server', () => {
   // server._start()
   // ============================================================================
   describe('_start', () => {
-    it('should compile static resources on first run', function (done) {
-      this.timeout(5000)
+    it('should compile static resources', (done) => {
+      sinon.spy(BundleUtils, 'bundleResourceIfNotExist')
+
       server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
-      fileListOnResolve()
 
-      setTimeout(() => {
-        expect(fs.existsSync(path.join(__dirname, '/../../static/karma.js')) && fs.existsSync(path.join(__dirname, '/../../static/context.js'))).to.be.true
+      fileListOnResolve().then(() => {
+        expect(BundleUtils.bundleResourceIfNotExist).to.have.been.calledWith('client/main.js', 'static/karma.js')
+        expect(BundleUtils.bundleResourceIfNotExist).to.have.been.calledWith('context/main.js', 'static/context.js')
         done()
-      }, 4000)
+      })
     })
 
-    it('should start the web server after all files have been preprocessed successfully', () => {
+    it('should start the web server after all files have been preprocessed successfully', (done) => {
       server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
 
       expect(mockFileList.refresh).to.have.been.called
       expect(fileListOnResolve).not.to.be.null
       expect(mockWebServer.listen).not.to.have.been.called
       expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)
 
-      fileListOnResolve()
-
-      expect(mockWebServer.listen).to.have.been.called
-      expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
+      fileListOnResolve().then(() => {
+        expect(mockWebServer.listen).to.have.been.called
+        expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
+        done()
+      })
     })
 
-    it('should start the web server after all files have been preprocessed with an error', () => {
+    it('should start the web server after all files have been preprocessed with an error', (done) => {
       server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
 
       expect(mockFileList.refresh).to.have.been.called
       expect(fileListOnReject).not.to.be.null
       expect(mockWebServer.listen).not.to.have.been.called
       expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)
 
-      fileListOnReject()
-
-      expect(mockWebServer.listen).to.have.been.called
-      expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
+      fileListOnReject().then(() => {
+        expect(mockWebServer.listen).to.have.been.called
+        expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
+        done()
+      })
     })
 
-    it('should launch browsers after the web server has started', () => {
+    it('should launch browsers after the web server has started', (done) => {
       server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
 
       expect(mockWebServer.listen).not.to.have.been.called
       expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)
 
-      fileListOnResolve()
-
-      expect(mockWebServer.listen).to.have.been.called
-      expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
+      fileListOnResolve().then(() => {
+        expect(mockWebServer.listen).to.have.been.called
+        expect(server._injector.invoke).to.have.been.calledWith(mockLauncher.launch, mockLauncher)
+        done()
+      })
     })
 
-    it('should listen on the listenAddress in the config', () => {
+    it('should listen on the listenAddress in the config', (done) => {
       server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
 
       expect(mockWebServer.listen).not.to.have.been.called
       expect(webServerOnError).not.to.be.null
 
       expect(mockConfig.listenAddress).to.be.equal('127.0.0.1')
 
-      fileListOnResolve()
-
-      expect(mockWebServer.listen).to.have.been.calledWith(9876, '127.0.0.1')
-      expect(mockConfig.listenAddress).to.be.equal('127.0.0.1')
+      fileListOnResolve().then(() => {
+        expect(mockWebServer.listen).to.have.been.calledWith(9876, '127.0.0.1')
+        expect(mockConfig.listenAddress).to.be.equal('127.0.0.1')
+        done()
+      })
     })
 
     it('should try next port if already in use', () => {
@@ -183,27 +187,24 @@ describe('server', () => {
 
       expect(mockConfig.port).to.be.equal(9876)
 
-      fileListOnResolve()
-
-      expect(mockWebServer.listen).to.have.been.calledWith(9876)
-
       webServerOnError({code: 'EADDRINUSE'})
 
       expect(mockWebServer.listen).to.have.been.calledWith(9877)
       expect(mockConfig.port).to.be.equal(9877)
     })
 
-    it('should emit a listening event once server begin accepting connections', () => {
+    it('should emit a listening event once server begin accepting connections', (done) => {
       server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
 
       var listening = sinon.spy()
       server.on('listening', listening)
 
       expect(listening).not.to.have.been.called
 
-      fileListOnResolve()
-
-      expect(listening).to.have.been.calledWith(9876)
+      fileListOnResolve().then(() => {
+        expect(listening).to.have.been.calledWith(9876)
+        done()
+      })
     })
 
     it('should emit a browsers_ready event once all the browsers are captured', () => {

diff --git a/test/unit/utils/bundle-utils.spec.js b/test/unit/utils/bundle-utils.spec.js
@@ -0,0 +1,42 @@
+'use strict'
+
+const BundleUtils = require('../../../lib/utils/bundle-utils')
+const PathUtils = require('../../../lib/utils/path-utils')
+const FileUtils = require('../../../lib/utils/file-utils')
+const fs = require('fs')
+
+const sandbox = sinon.sandbox.create()
+
+describe('BundleUtils.bundleResource', () => {
+  beforeEach(() => FileUtils.removeFileIfExists(PathUtils.calculateAbsolutePath('test/unit/fixtures/bundled.js')))
+
+  it('create bundle file in requested output path', (done) => {
+    BundleUtils.bundleResource('test/unit/fixtures/format-error-root.js', 'test/unit/fixtures/bundled.js')
+      .then(() => {
+        expect(fs.existsSync(PathUtils.calculateAbsolutePath('test/unit/fixtures/bundled.js'))).to.be.true
+        done()
+      })
+  })
+})
+
+describe('BundleUtils.bundleResourceIfNotExist', () => {
+  beforeEach(() => {
+    sandbox.stub(BundleUtils, 'bundleResource').resolves()
+  })
+
+  afterEach(() => sandbox.restore())
+
+  it('bundle resource when output path file not exists', () => {
+    sandbox.stub(fs, 'existsSync').returns(false)
+
+    BundleUtils.bundleResourceIfNotExist('context/main.js', 'static/context.js')
+    expect(BundleUtils.bundleResource).to.have.been.called
+  })
+
+  it('does not bundle resource when output path file exists', () => {
+    sandbox.stub(fs, 'existsSync').returns(true)
+
+    BundleUtils.bundleResourceIfNotExist('context/main.js', 'static/context.js')
+    expect(BundleUtils.bundleResource).to.not.have.been.called
+  })
+})
diff --git a/test/unit/utils/path-utils.spec.js b/test/unit/utils/path-utils.spec.js
@@ -0,0 +1,9 @@
+'use strict'
+const PathUtils = require('../../../lib/utils/path-utils')
+const fs = require('fs')
+
+describe('PathUtils.calculateAbsolutePath', () => {
+  it('returns absolute path from karma project relative path', () => {
+    expect(fs.existsSync(PathUtils.calculateAbsolutePath('logo/banner.png'))).to.be.true
+  })
+})