Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
May 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/dev/05-plugins.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
10 changes: 10 additions & 0 deletions lib/file.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict'

const path = require('path')

/**
* File object used for tracking files in `file-list.js`.
*/
Expand Down Expand Up @@ -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
}
Expand Down
16 changes: 11 additions & 5 deletions lib/middleware/karma.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

helper.isDefined(fileType) always returns true with current code, that's why it was removed.

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 http://karma-runner.github.io/latest/config/files.html`
)
} else {
log.warn(`Invalid file type (${file.type || 'empty string'}), defaulting to js.`)
}
}

if (!file.isUrl) {
Expand Down
12 changes: 12 additions & 0 deletions lib/url.js
Original file line number Diff line number Diff line change
@@ -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
}
Expand Down
24 changes: 24 additions & 0 deletions test/e2e/helpful-logs.feature
Original file line number Diff line number Diff line change
@@ -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 http://karma-runner.github.io/latest/config/files.html
"""
15 changes: 15 additions & 0 deletions test/unit/file.spec.js
Original file line number Diff line number Diff line change
@@ -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('')
})
})
})
30 changes: 30 additions & 0 deletions test/unit/url.spec.js
Original file line number Diff line number Diff line change
@@ -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('')
})
})
})