Skip to content

Commit

Permalink
fix(server): Simplify 'dom' inclusion. (karma-runner#3356)
Browse files Browse the repository at this point in the history
The karma preprocessor caches file content: no need to read it async in the server loop.
  • Loading branch information
johnjbarton authored Aug 26, 2019
1 parent 817fbbd commit 5f13e11
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 66 deletions.
17 changes: 1 addition & 16 deletions lib/middleware/karma.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ function createKarmaMiddleware (
filesPromise,
serveStaticFile,
serveFile,
readFilePromise,
injector,
basePath,
urlRoot,
Expand Down Expand Up @@ -136,21 +135,8 @@ function createKarmaMiddleware (
const isRequestingContextFile = requestUrl === '/context.html'
const isRequestingDebugFile = requestUrl === '/debug.html'
const isRequestingClientContextFile = requestUrl === '/client_with_context.html'
const includedContent = new Map() // file.path -> content
if (isRequestingContextFile || isRequestingDebugFile || isRequestingClientContextFile) {
return filesPromise.then((files) => {
// Read any files.included that will be directly written into HTML before HTML is read.
const contentReads = []
for (const file of files.included) {
const fileType = file.type || path.extname(file.path).substring(1)
if (fileType === 'dom') {
contentReads.push(
readFilePromise(file.path).then((content) => includedContent.set(file.path, content))
)
}
}
return Promise.all(contentReads).then(() => files)
}).then(function (files) {
let fileServer
let requestedFileUrl
log.debug('custom files', customContextFile, customDebugFile, customClientContextFile)
Expand Down Expand Up @@ -195,7 +181,7 @@ function createKarmaMiddleware (
if (fileType === 'css') {
scriptTags.push(`<link type="text/css" href="${filePath}" rel="stylesheet">`)
} else if (fileType === 'dom') {
scriptTags.push(includedContent.get(file.path))
scriptTags.push(file.content)
} else if (fileType === 'html') {
scriptTags.push(`<link href="${filePath}" rel="import">`)
} else {
Expand Down Expand Up @@ -253,7 +239,6 @@ createKarmaMiddleware.$inject = [
'filesPromise',
'serveStaticFile',
'serveFile',
'readFilePromise',
'injector',
'config.basePath',
'config.urlRoot',
Expand Down
22 changes: 1 addition & 21 deletions lib/web-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,25 +39,6 @@ function createFilesPromise (emitter, fileList) {
return filesPromise
}

// Bind the filesystem into the injectable file reader function
function createReadFilePromise () {
return (filepath) => {
return new Promise((resolve, reject) => {
fs.readFile(filepath, 'utf8', function (error, data) {
if (error) {
reject(new Error(`Cannot read ${filepath}, got: ${error}`))
} else if (!data) {
reject(new Error(`No content at ${filepath}`))
} else {
resolve(data.split('\n'))
}
})
})
}
}

createReadFilePromise.$inject = []

function createServeStaticFile (config) {
return common.createServeFile(fs, path.normalize(path.join(__dirname, '/../static')), config)
}
Expand Down Expand Up @@ -124,6 +105,5 @@ module.exports = {
createWebServer,
createServeFile,
createServeStaticFile,
createFilesPromise,
createReadFilePromise
createFilesPromise
}
31 changes: 5 additions & 26 deletions test/unit/middleware/karma.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ const HttpRequestMock = mocks.http.ServerRequest

describe('middleware.karma', () => {
let serveFile
let readFilesDeferred
let filesDeferred
let nextSpy
let response

class MockFile extends File {
constructor (path, sha, type) {
constructor (path, sha, type, content) {
super(path, undefined, undefined, type)
this.sha = sha || 'sha-default'
this.content = content
}
}

Expand Down Expand Up @@ -64,7 +64,6 @@ describe('middleware.karma', () => {
filesDeferred.promise,
serveFile,
null,
null,
injector,
'/base/path',
'/__karma__/',
Expand Down Expand Up @@ -121,12 +120,10 @@ describe('middleware.karma', () => {
})

it('should serve client.html', (done) => {
readFilesDeferred = helper.defer()
handler = createKarmaMiddleware(
null,
serveFile,
null,
readFilesDeferred.promise,
injector,
'/base',
'/'
Expand All @@ -142,12 +139,10 @@ describe('middleware.karma', () => {
})

it('should serve /?id=xxx', (done) => {
readFilesDeferred = helper.defer()
handler = createKarmaMiddleware(
null,
serveFile,
null,
readFilesDeferred.promise,
injector,
'/base',
'/'
Expand All @@ -163,13 +158,10 @@ describe('middleware.karma', () => {
})

it('should serve /?x-ua-compatible with replaced values', (done) => {
readFilesDeferred = helper.defer()

handler = createKarmaMiddleware(
null,
serveFile,
null,
readFilesDeferred.promise,
injector,
'/base',
'/'
Expand Down Expand Up @@ -278,31 +270,20 @@ describe('middleware.karma', () => {
})

it('should serve context.html with included DOM content', (done) => {
const readFilePromise = (path) => {
const cases = {
'/some/abc/a.dom': 'a',
'/some/abc/b_test_dom.html': 'b',
'/some/abc/c': 'c',
'/some/abc/d_test_dom.html': 'd'
}
return Promise.resolve(cases[path] || '?unknown ' + path)
}

filesDeferred = helper.defer()
handler = createKarmaMiddleware(
filesDeferred.promise,
serveFile,
null,
readFilePromise,
injector,
'/base',
'/'
)

includedFiles([
new MockFile('/some/abc/a.dom', 'sha1'),
new MockFile('/some/abc/b_test_dom.html', 'sha2', 'dom'),
new MockFile('/some/abc/c', 'sha3', 'dom')
new MockFile('/some/abc/a.dom', 'sha1', undefined, 'a'),
new MockFile('/some/abc/b_test_dom.html', 'sha2', 'dom', 'b'),
new MockFile('/some/abc/c', 'sha3', 'dom', 'c')
])

response.once('end', () => {
Expand Down Expand Up @@ -468,12 +449,10 @@ describe('middleware.karma', () => {

it('should update handle updated configs', (done) => {
let i = 0
readFilesDeferred = helper.defer()
handler = createKarmaMiddleware(
filesDeferred.promise,
serveFile,
null,
readFilesDeferred.promise,
{
get (val) {
if (val === 'config.client') {
Expand Down
3 changes: 0 additions & 3 deletions test/unit/web-server.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ describe('web-server', () => {
filesPromise: ['factory', m.createFilesPromise],
serveStaticFile: ['factory', m.createServeStaticFile],
serveFile: ['factory', m.createServeFile],
readFilePromise: ['factory', m.createReadFilePromise],
capturedBrowsers: ['value', null],
reporter: ['value', null],
executor: ['value', null],
Expand Down Expand Up @@ -233,7 +232,6 @@ describe('web-server', () => {
filesPromise: ['factory', m.createFilesPromise],
serveStaticFile: ['factory', m.createServeStaticFile],
serveFile: ['factory', m.createServeFile],
readFilePromise: ['factory', m.createReadFilePromise],
capturedBrowsers: ['value', null],
reporter: ['value', null],
executor: ['value', null],
Expand Down Expand Up @@ -278,7 +276,6 @@ describe('web-server', () => {
filesPromise: ['factory', m.createFilesPromise],
serveStaticFile: ['factory', m.createServeStaticFile],
serveFile: ['factory', m.createServeFile],
readFilePromise: ['factory', m.createReadFilePromise],
capturedBrowsers: ['value', null],
reporter: ['value', null],
executor: ['value', null],
Expand Down

0 comments on commit 5f13e11

Please sign in to comment.