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.

Sign up for GitHub

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

Jump to bottom

feat(preprocessor): preprocessor_priority execution order. #3303

Merged
merged 1 commit into from
Aug 21, 2019
Merged

feat(preprocessor): preprocessor_priority execution order. #3303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions docs/config/04-preprocessors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,9 @@ preprocessors: {

Then karma will execute `'a'` before executing `'b'`.

If a file matches multiple keys, karma will do its best to execute the
preprocessors in a reasonable order. So if you have:
If a file matches multiple keys, karma will use the `config.preprocessor_priority`
map to set the order. If this config option is not set, karma do its best to
execute the preprocessors in a reasonable order. So if you have:

```js
preprocessors: {
Expand Down
1 change: 1 addition & 0 deletions lib/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ class Config {
this.proxies = {}
this.proxyValidateSSL = true
this.preprocessors = {}
this.preprocessor_priority = {}
this.urlRoot = '/'
this.upstreamProxy = undefined
this.reportSlowerThan = 0
Expand Down
20 changes: 17 additions & 3 deletions lib/preprocessor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function createNextProcessor (preprocessors, file, done) {
}
}

function createPreprocessor (config, basePath, injector) {
function createPriorityPreprocessor (config, preprocessorPriority, basePath, injector) {
const emitter = injector.get('emitter')
const alreadyDisplayedErrors = {}
const instances = {}
Expand Down Expand Up @@ -96,9 +96,15 @@ function createPreprocessor (config, basePath, injector) {
}
})

// Apply preprocessor priority.
let sortedPreprocessorNames = preprocessorNames
.map((name) => [name, preprocessorPriority[name] || 0])
.sort((a, b) => b[1] - a[1])
.map((duo) => duo[0])

let preprocessors = []
const nextPreprocessor = createNextProcessor(preprocessors, file, done)
preprocessorNames.forEach((name) => {
sortedPreprocessorNames.forEach((name) => {
const p = instances[name] || instantiatePreprocessor(name)

if (p == null) {
Expand All @@ -125,6 +131,14 @@ function createPreprocessor (config, basePath, injector) {
}
}

// Deprecated API
function createPreprocessor (preprocessors, basePath, injector) {
console.log('Deprecated private createPreprocessor() API')
const preprocessorPriority = injector.get('config.preprocessor_priority')
return createPriorityPreprocessor(preprocessors, preprocessorPriority, basePath, injector)
}
createPreprocessor.$inject = ['config.preprocessors', 'config.basePath', 'injector']

exports.createPreprocessor = createPreprocessor

createPriorityPreprocessor.$inject = ['config.preprocessors', 'config.preprocessor_priority', 'config.basePath', 'injector']
exports.createPriorityPreprocessor = createPriorityPreprocessor
2 changes: 1 addition & 1 deletion lib/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class Server extends KarmaEventEmitter {
watcher: ['value', watcher],
launcher: ['type', Launcher],
config: ['value', config],
preprocess: ['factory', preprocessor.createPreprocessor],
preprocess: ['factory', preprocessor.createPriorityPreprocessor],
fileList: ['factory', FileList.factory],
webServer: ['factory', createWebServer],
serveFile: ['factory', createServeFile],
Expand Down
88 changes: 67 additions & 21 deletions test/unit/preprocessor.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('preprocessor', () => {
'factory', function () { return fakePreprocessor }
]
}, emitterSetting])
pp = m.createPreprocessor({ '**/*.js': ['fake'] }, null, injector)
pp = m.createPriorityPreprocessor({ '**/*.js': ['fake'] }, {}, null, injector)

const file = { originalPath: '/some/a.js', path: 'path' }

Expand All @@ -68,7 +68,7 @@ describe('preprocessor', () => {
const injector = new di.Injector([{
'preprocessor:fake': ['factory', function () { return fakePreprocessor }]
}, emitterSetting])
pp = m.createPreprocessor({ '**/*.js': ['fake'] }, null, injector)
pp = m.createPriorityPreprocessor({ '**/*.js': ['fake'] }, {}, null, injector)

const file = { originalPath: '/some/.dir/a.js', path: 'path' }

Expand All @@ -90,7 +90,7 @@ describe('preprocessor', () => {
'preprocessor:fake': ['factory', function () { return fakePreprocessor }]
}, emitterSetting])
const config = { '**/*.txt': ['fake'] }
pp = m.createPreprocessor(config, null, injector)
pp = m.createPriorityPreprocessor(config, {}, null, injector)

const file = { originalPath: '/some/a.js', path: 'path' }

Expand All @@ -112,7 +112,7 @@ describe('preprocessor', () => {
const injector = new di.Injector([{
'preprocessor:fake': ['factory', function () { return fakePreprocessor }]
}, emitterSetting])
pp = m.createPreprocessor({ '**/*.js': ['fake'] }, null, injector)
pp = m.createPriorityPreprocessor({ '**/*.js': ['fake'] }, {}, null, injector)

const file = { originalPath: '/some/a.txt', path: 'path' }

Expand All @@ -138,7 +138,7 @@ describe('preprocessor', () => {
'preprocessor:fake2': ['factory', function () { return fakePreprocessor2 }]
}, emitterSetting])

pp = m.createPreprocessor({ '**/*.js': ['fake1', 'fake2'] }, null, injector)
pp = m.createPriorityPreprocessor({ '**/*.js': ['fake1', 'fake2'] }, {}, null, injector)

const file = { originalPath: '/some/a.js', path: 'path' }

Expand All @@ -152,7 +152,7 @@ describe('preprocessor', () => {
})

it('should compute SHA', (done) => {
pp = m.createPreprocessor({}, null, new di.Injector([emitterSetting]))
pp = m.createPriorityPreprocessor({}, {}, null, new di.Injector([emitterSetting]))
const file = { originalPath: '/some/a.js', path: 'path' }

pp(file, () => {
Expand Down Expand Up @@ -182,7 +182,7 @@ describe('preprocessor', () => {
'preprocessor:fake': ['factory', function () { return fakePreprocessor }]
}, emitterSetting])

pp = m.createPreprocessor({ '**/a.js': ['fake'] }, null, injector)
pp = m.createPriorityPreprocessor({ '**/a.js': ['fake'] }, {}, null, injector)

const fileProcess = { originalPath: '/some/a.js', path: 'path' }
const fileSkip = { originalPath: '/some/b.js', path: 'path' }
Expand All @@ -208,7 +208,7 @@ describe('preprocessor', () => {
'preprocessor:failing': ['factory', function () { return failingPreprocessor }]
}, emitterSetting])

pp = m.createPreprocessor({ '**/*.js': ['failing'] }, null, injector)
pp = m.createPriorityPreprocessor({ '**/*.js': ['failing'] }, {}, null, injector)

const file = { originalPath: '/some/a.js', path: 'path' }

Expand All @@ -232,7 +232,7 @@ describe('preprocessor', () => {
'preprocessor:fake': ['factory', function () { return fakePreprocessor }]
}, emitterSetting])

pp = m.createPreprocessor({ '**/*.js': ['failing', 'fake'] }, null, injector)
pp = m.createPriorityPreprocessor({ '**/*.js': ['failing', 'fake'] }, {}, null, injector)

const file = { originalPath: '/some/a.js', path: 'path' }

Expand Down Expand Up @@ -261,7 +261,7 @@ describe('preprocessor', () => {
'preprocessor:fake': ['factory', function () { return fakePreprocessor }]
}, emitterSetting])

const pp = m.createPreprocessor({ '**/*.js': ['fake'] }, null, injector)
const pp = m.createPriorityPreprocessor({ '**/*.js': ['fake'] }, {}, null, injector)

pp(file, () => {
expect(fakePreprocessor).to.have.been.called
Expand All @@ -277,7 +277,7 @@ describe('preprocessor', () => {
it('should throw after 3 retries', (done) => {
const injector = new di.Injector([{}, emitterSetting])

const pp = m.createPreprocessor({ '**/*.js': [] }, null, injector)
const pp = m.createPriorityPreprocessor({ '**/*.js': [] }, {}, null, injector)

pp(file, () => { })

Expand All @@ -299,7 +299,7 @@ describe('preprocessor', () => {
'preprocessor:fake': ['factory', function () { return fakePreprocessor }]
}, emitterSetting])

pp = m.createPreprocessor({ '**/*': ['fake'] }, null, injector)
pp = m.createPriorityPreprocessor({ '**/*': ['fake'] }, {}, null, injector)

const file = { originalPath: '/some/photo.png', path: 'path' }

Expand All @@ -322,7 +322,7 @@ describe('preprocessor', () => {
'preprocessor:fake': ['factory', function () { return fakePreprocessor }]
}, emitterSetting])

pp = m.createPreprocessor({ '**/*': ['fake'] }, null, injector)
pp = m.createPriorityPreprocessor({ '**/*': ['fake'] }, {}, null, injector)

const file = { originalPath: '/some/photo.png', path: 'path' }

Expand All @@ -344,7 +344,7 @@ describe('preprocessor', () => {
'preprocessor:fake': ['factory', function () { fakePreprocessor }]
}, emitterSetting])

pp = m.createPreprocessor({ '**/*': ['fake'] }, null, injector)
pp = m.createPriorityPreprocessor({ '**/*': ['fake'] }, {}, null, injector)

const file = { originalPath: '/some/CAM_PHOTO.JPG', path: 'path' }

Expand All @@ -357,7 +357,7 @@ describe('preprocessor', () => {
})
})

it('should merge lists of preprocessors', (done) => {
it('should merge lists of preprocessors using default priority', (done) => {
const callOrder = []
const fakePreprocessorA = sinon.spy((content, file, done) => {
callOrder.push('a')
Expand All @@ -383,11 +383,11 @@ describe('preprocessor', () => {
'preprocessor:fakeD': ['factory', function () { return fakePreprocessorD }]
}, emitterSetting])

pp = m.createPreprocessor({
pp = m.createPriorityPreprocessor({
'/*/a.js': ['fakeA', 'fakeB'],
'/some/*': ['fakeB', 'fakeC'],
'/some/a.js': ['fakeD']
}, null, injector)
}, {}, null, injector)

const file = { originalPath: '/some/a.js', path: 'path' }

Expand All @@ -399,10 +399,56 @@ describe('preprocessor', () => {
expect(fakePreprocessorC).to.have.been.called
expect(fakePreprocessorD).to.have.been.called

expect(callOrder.indexOf('d')).not.to.equal(-1)
expect(callOrder.filter((letter) => {
return letter !== 'd'
})).to.eql(['a', 'b', 'c'])
expect(callOrder).to.eql(['a', 'b', 'c', 'd'])
done()
})
})

it('should merge lists of preprocessors obeying priority', (done) => {
const callOrder = []
const fakePreprocessorA = sinon.spy((content, file, done) => {
callOrder.push('a')
done(null, content)
})
const fakePreprocessorB = sinon.spy((content, file, done) => {
callOrder.push('b')
done(null, content)
})
const fakePreprocessorC = sinon.spy((content, file, done) => {
callOrder.push('c')
done(null, content)
})
const fakePreprocessorD = sinon.spy((content, file, done) => {
callOrder.push('d')
done(null, content)
})

const injector = new di.Injector([{
'preprocessor:fakeA': ['factory', function () { return fakePreprocessorA }],
'preprocessor:fakeB': ['factory', function () { return fakePreprocessorB }],
'preprocessor:fakeC': ['factory', function () { return fakePreprocessorC }],
'preprocessor:fakeD': ['factory', function () { return fakePreprocessorD }]
}, emitterSetting])

const priority = { 'fakeA': -1, 'fakeB': 1, 'fakeD': 100 }

pp = m.createPriorityPreprocessor({
'/*/a.js': ['fakeA', 'fakeB'],
'/some/*': ['fakeB', 'fakeC'],
'/some/a.js': ['fakeD']
}, priority, null, injector)

const file = { originalPath: '/some/a.js', path: 'path' }

pp(file, (err) => {
if (err) throw err

expect(fakePreprocessorA).to.have.been.called
expect(fakePreprocessorB).to.have.been.called
expect(fakePreprocessorC).to.have.been.called
expect(fakePreprocessorD).to.have.been.called

expect(callOrder).to.eql(['d', 'b', 'c', 'a'])
done()
})
})
Expand Down