fix: detect type for URLs with query parameter or fragment identifier (#3509)

devoto13 · web-flow · commit f399063d1bc8 · 2020-05-16T10:40:39.000-07:00
Closes #3497
diff --git a/docs/dev/05-plugins.md b/docs/dev/05-plugins.md
@@ -27,7 +27,7 @@ A preprocessor is a function that accepts three arguments (`content`, `file`, an
      - **path:** the current file, mutable file path. e. g. `some/file.coffee` -> `some/file.coffee.js` _This path is mutable and may not actually exist._
      - **originalPath:** the original, unmutated path
      - **encodings:** A mutable, keyed object where the keys are a valid encoding type ('gzip', 'compress', 'br', etc.) and the values are the encoded content. Encoded content should be stored here and not resolved using `next(null, encodedContent)`
-     - **type:** the pattern used to match the file
+     - **type:** determines how to include a file, when serving
   - **`next`** function to be called when preprocessing is complete, should be called as `next(null, processedContent)` or `next(error)`
 - example plugins: [karma-coffee-preprocessor], [karma-ng-html2js-preprocessor]
 - use naming convention is `karma-*-preprocessor`
diff --git a/lib/file.js b/lib/file.js
@@ -1,5 +1,7 @@
 'use strict'
 
+const path = require('path')
+
 /**
  * File object used for tracking files in `file-list.js`.
  */
@@ -29,6 +31,14 @@ class File {
     this.isBinary = isBinary === undefined ? null : isBinary
   }
 
+  /**
+   * Detect type from the file extension.
+   * @returns {string} detected file type or empty string
+   */
+  detectType () {
+    return path.extname(this.path).substring(1)
+  }
+
   toString () {
     return this.path
   }
diff --git a/lib/middleware/karma.js b/lib/middleware/karma.js
@@ -11,9 +11,7 @@
  * - setting propert caching headers
  */
 
-const path = require('path')
 const url = require('url')
-const helper = require('../helper')
 
 const log = require('../logger').create('middleware:karma')
 const stripHost = require('./strip_host').stripHost
@@ -164,10 +162,18 @@ function createKarmaMiddleware (
           const scriptTags = []
           for (const file of files.included) {
             let filePath = file.path
-            const fileType = file.type || path.extname(filePath).substring(1)
+            const fileType = file.type || file.detectType()
 
-            if (helper.isDefined(fileType) && !FILE_TYPES.includes(fileType)) {
-              log.warn(`Invalid file type (${fileType}), defaulting to js.`)
+            if (!FILE_TYPES.includes(fileType)) {
+              if (file.type == null) {
+                log.warn(
+                  `Unable to determine file type from the file extension, defaulting to js.\n` +
+                  `  To silence the warning specify a valid type for ${file.originalPath} in the configuration file.\n` +
+                  `  See https://karma-runner.github.io/latest/config/files.html`
+                )
+              } else {
+                log.warn(`Invalid file type (${file.type || 'empty string'}), defaulting to js.`)
+              }
             }
 
             if (!file.isUrl) {
diff --git a/lib/url.js b/lib/url.js
@@ -1,15 +1,27 @@
 'use strict'
 
+const path = require('path')
+const { URL } = require('url')
+
 /**
  * Url object used for tracking files in `file-list.js`.
  */
 class Url {
   constructor (path, type) {
     this.path = path
+    this.originalPath = path
     this.type = type
     this.isUrl = true
   }
 
+  /**
+   * Detect type from the file extension in the path part of the URL.
+   * @returns {string} detected file type or empty string
+   */
+  detectType () {
+    return path.extname(new URL(this.path).pathname).substring(1)
+  }
+
   toString () {
     return this.path
   }
diff --git a/test/e2e/helpful-logs.feature b/test/e2e/helpful-logs.feature
@@ -0,0 +1,24 @@
+Feature: Helpful warning and errors
+  In order to use Karma
+  As a person who wants to write great tests
+  I want to get messages which help me to fix problems
+
+  Scenario: Karma fails to determine a file type from the file extension
+    Given a configuration with:
+      """
+      files = [ 'modules/**/*.mjs' ];
+      browsers = ['ChromeHeadlessNoSandbox'];
+      frameworks = ['mocha', 'chai'];
+      plugins = [
+        'karma-mocha',
+        'karma-chai',
+        'karma-chrome-launcher'
+      ];
+      """
+    When I start Karma
+    Then the stdout matches RegExp:
+      """
+      WARN \[middleware:karma\]: Unable to determine file type from the file extension, defaulting to js.
+        To silence the warning specify a valid type for .+modules/minus.mjs in the configuration file.
+        See https://karma-runner.github.io/latest/config/files.html
+      """
diff --git a/test/unit/file.spec.js b/test/unit/file.spec.js
@@ -0,0 +1,15 @@
+const File = require('../../lib/file')
+
+describe('File', () => {
+  describe('detectType', () => {
+    it('should detect type from the file extension', () => {
+      const file = new File('/path/to/file.js')
+      expect(file.detectType()).to.equal('js')
+    })
+
+    it('should return empty string if file does not have an extension', () => {
+      const file = new File('/path/to/file-without-extension')
+      expect(file.detectType()).to.equal('')
+    })
+  })
+})
diff --git a/test/unit/url.spec.js b/test/unit/url.spec.js
@@ -0,0 +1,30 @@
+const Url = require('../../lib/url')
+
+describe('Url', () => {
+  describe('detectType', () => {
+    it('should detect type from the file extension in the path of the URL', () => {
+      const file = new Url('https://example.com/path/to/file.js')
+      expect(file.detectType()).to.equal('js')
+    })
+
+    it('should detect type for URL with query params', () => {
+      const file = new Url('https://example.com/path/to/file.js?query=simple')
+      expect(file.detectType()).to.equal('js')
+    })
+
+    it('should detect type for URL with a fragment', () => {
+      const file = new Url('https://example.com/path/to/file.js#fragment')
+      expect(file.detectType()).to.equal('js')
+    })
+
+    it('should return empty string if URL does not have path', () => {
+      const file = new Url('https://example.com')
+      expect(file.detectType()).to.equal('')
+    })
+
+    it('should return empty string if path in the URL does not have an extension', () => {
+      const file = new Url('https://example.com/path/to/file-without-extension')
+      expect(file.detectType()).to.equal('')
+    })
+  })
+})
