Skip to content

Commit d972f3d

Browse files
jamestalmagejamestalmage
committed
feat: Allow frameworks to inject middleware
The Server#_start function was injected with `webServer` and `socketServer`. This prevents frameworks from being able to manipulate `config.middleware` before those servers are created. The solution is simply to defer injection until after all frameworks have loaded, and then fetch those values with `injector.get`. There were no e2e tests involving middleware. I have added two, including one that verifies the new capabilities.
1 parent eb94739 commit d972f3d

7 files changed

Lines changed: 99 additions & 11 deletions

File tree

‎lib/server.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,8 @@ Server.prototype.refreshFiles = function () {
129129
// Private Methods
130130
// ---------------
131131

132-
Server.prototype._start = function (config, launcher, preprocess, fileList, webServer,
133-
capturedBrowsers, socketServer, executor, done) {
132+
Server.prototype._start = function (config, launcher, preprocess, fileList,
133+
capturedBrowsers, executor, done) {
134134
var self = this
135135
if (config.detached) {
136136
this._detach(config, done)
@@ -143,6 +143,9 @@ Server.prototype._start = function (config, launcher, preprocess, fileList, webS
143143
self._injector.get('framework:' + framework)
144144
})
145145

146+
var webServer = self._injector.get('webServer')
147+
var socketServer = self._injector.get('socketServer')
148+
146149
// A map of launched browsers.
147150
var singleRunDoneBrowsers = Object.create(null)
148151

‎test/e2e/middleware.feature‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
Feature: Middleware
2+
In order to use Karma
3+
As a person who wants to write great tests
4+
I want to use custom middleware with Karma.
5+
6+
Scenario: Simple middleware
7+
Given a configuration with:
8+
"""
9+
files = ['middleware/test.js'];
10+
browsers = ['PhantomJS'];
11+
plugins = [
12+
'karma-jasmine',
13+
'karma-phantomjs-launcher',
14+
_resolve('middleware/middleware')
15+
];
16+
middleware = [
17+
'foo'
18+
]
19+
"""
20+
When I start Karma
21+
Then it passes with:
22+
"""
23+
.
24+
PhantomJS
25+
"""
26+
27+
Scenario: Frameworks can add middleware
28+
Given a configuration with:
29+
"""
30+
files = ['middleware/test.js'];
31+
browsers = ['PhantomJS'];
32+
plugins = [
33+
'karma-jasmine',
34+
'karma-phantomjs-launcher',
35+
_resolve('middleware/middleware')
36+
];
37+
frameworks = ['jasmine', 'foo']
38+
"""
39+
When I start Karma
40+
Then it passes with:
41+
"""
42+
.
43+
PhantomJS
44+
"""

‎test/e2e/pass-opts.feature‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,4 @@ Feature: Passing Options
1919
Then it passes with no debug:
2020
"""
2121
.
22-
PhantomJS
2322
"""

‎test/e2e/support/middleware/middleware.js‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
function middleware (request, response, next) {
2+
if (/\/foo\.js/.test(request.normalizedUrl)) {
3+
response.setHeader('Content-Type', 'text/plain')
4+
response.writeHead(200)
5+
response.end('this is the middleware response')
6+
return
7+
}
8+
next()
9+
}
10+
11+
function framework (config) {
12+
config.middleware = config.middleware || []
13+
config.middleware.push('foo')
14+
}
15+
16+
framework.$inject = ['config']
17+
18+
module.exports = {
19+
'framework:foo': ['factory', framework],
20+
'middleware:foo': ['value', middleware]
21+
}

‎test/e2e/support/middleware/test.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
function httpGet (url) {
2+
var xmlHttp = new XMLHttpRequest()
3+
4+
xmlHttp.open('GET', url, false)
5+
xmlHttp.send(null)
6+
7+
return xmlHttp.responseText
8+
}
9+
10+
describe('foo', function () {
11+
it('should should serve /foo.js', function () {
12+
expect(httpGet('/foo.js')).toBe('this is the middleware response')
13+
})
14+
})

‎test/e2e/support/world.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ exports.World = function World () {
1414
frameworks: ['jasmine'],
1515
basePath: __dirname,
1616
colors: false,
17-
__dirname: __dirname
17+
__dirname: __dirname,
18+
_resolve: function (name) {
19+
return path.resolve(__dirname, '..', 'support', name)
20+
}
1821
}
1922

2023
this.addConfigContent = (function (_this) {

‎test/unit/server.spec.js‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ describe('server', () => {
8484
close: () => {}
8585
}
8686

87+
sinon.stub(server._injector, 'get')
88+
.withArgs('webServer').returns(mockWebServer)
89+
.withArgs('socketServer').returns(mockSocketServer)
90+
8791
webServerOnError = null
8892
})
8993

@@ -92,7 +96,7 @@ describe('server', () => {
9296
// ============================================================================
9397
describe('_start', () => {
9498
it('should start the web server after all files have been preprocessed successfully', () => {
95-
server._start(mockConfig, mockLauncher, null, mockFileList, mockWebServer, browserCollection, mockSocketServer, mockExecutor, doneSpy)
99+
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
96100

97101
expect(mockFileList.refresh).to.have.been.called
98102
expect(fileListOnResolve).not.to.be.null
@@ -106,7 +110,7 @@ describe('server', () => {
106110
})
107111

108112
it('should start the web server after all files have been preprocessed with an error', () => {
109-
server._start(mockConfig, mockLauncher, null, mockFileList, mockWebServer, browserCollection, mockSocketServer, mockExecutor, doneSpy)
113+
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
110114

111115
expect(mockFileList.refresh).to.have.been.called
112116
expect(fileListOnReject).not.to.be.null
@@ -120,7 +124,7 @@ describe('server', () => {
120124
})
121125

122126
it('should launch browsers after the web server has started', () => {
123-
server._start(mockConfig, mockLauncher, null, mockFileList, mockWebServer, browserCollection, mockSocketServer, mockExecutor, doneSpy)
127+
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
124128

125129
expect(mockWebServer.listen).not.to.have.been.called
126130
expect(server._injector.invoke).not.to.have.been.calledWith(mockLauncher.launch, mockLauncher)
@@ -132,7 +136,7 @@ describe('server', () => {
132136
})
133137

134138
it('should try next port if already in use', () => {
135-
server._start(mockConfig, mockLauncher, null, mockFileList, mockWebServer, browserCollection, mockSocketServer, mockExecutor, doneSpy)
139+
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
136140

137141
expect(mockWebServer.listen).not.to.have.been.called
138142
expect(webServerOnError).not.to.be.null
@@ -150,7 +154,7 @@ describe('server', () => {
150154
})
151155

152156
it('should emit a listening event once server begin accepting connections', () => {
153-
server._start(mockConfig, mockLauncher, null, mockFileList, mockWebServer, browserCollection, mockSocketServer, mockExecutor, doneSpy)
157+
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
154158

155159
var listening = sinon.spy()
156160
server.on('listening', listening)
@@ -163,7 +167,7 @@ describe('server', () => {
163167
})
164168

165169
it('should emit a browsers_ready event once all the browsers are captured', () => {
166-
server._start(mockConfig, mockLauncher, null, mockFileList, mockWebServer, browserCollection, mockSocketServer, mockExecutor, doneSpy)
170+
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
167171

168172
var browsersReady = sinon.spy()
169173
server.on('browsers_ready', browsersReady)
@@ -178,7 +182,7 @@ describe('server', () => {
178182
})
179183

180184
it('should emit a browser_register event for each browser added', () => {
181-
server._start(mockConfig, mockLauncher, null, mockFileList, mockWebServer, browserCollection, mockSocketServer, mockExecutor, doneSpy)
185+
server._start(mockConfig, mockLauncher, null, mockFileList, browserCollection, mockExecutor, doneSpy)
182186

183187
var browsersReady = sinon.spy()
184188
server.on('browsers_ready', browsersReady)

0 commit comments

Comments
 (0)