karma breaks when singleRun === false and javascript code is invalid #1783
Comments
|
Ok so digged even more into the issue and this is what I found: When there is an invalid code (like the if without closed parenthesis ... Code 1instrument: function (code, filename, callback) {
if (!callback && typeof filename === 'function') {
callback = filename;
filename = null;
}
try {
callback(null, this.instrumentSync(code, filename));
} catch (ex) {
callback(ex);//CODE REACHES THIS CATCH WHEN THERE IS INVALID CODE
}
}The Code 2instrumenter.instrument(content, jsPath, function (err, instrumentedCode) {
if (err) {
log.error('%s\n at %s', err.message, file.originalPath)
}
if (file.sourceMap && instrumenter.lastSourceMap()) {
log.debug('Adding source map to instrumented file for "%s".', file.originalPath)
var generator = SourceMapGenerator.fromSourceMap(new SourceMapConsumer(instrumenter.lastSourceMap().toString()))
generator.applySourceMap(new SourceMapConsumer(file.sourceMap))
file.sourceMap = JSON.parse(generator.toString())
instrumentedCode += '\n//# sourceMappingURL=data:application/json;charset=utf-8;base64,'
instrumentedCode += new Buffer(JSON.stringify(file.sourceMap)).toString('base64') + '\n'
}
// remember the actual immediate instrumented JS for given original path
sourceCache[jsPath] = content
if (includeAllSources) {
// reset stateful regex
coverageObjRegex.lastIndex = 0
var coverageObjMatch = coverageObjRegex.exec(instrumentedCode)
if (coverageObjMatch !== null) {
var coverageObj = JSON.parse(coverageObjMatch[0])
coverageMap.add(coverageObj)
}
}
// RequireJS expects JavaScript files to end with `.js`
if (useJSExtensionForCoffeeScript && instrumenterLiteral === 'ibrik') {
file.path = file.path.replace(/\.coffee$/, '.js')
}
done(instrumentedCode)
})If you notice, when the now, since no second parameter is being passed in this Code 3var nextPreprocessor = function (error, content) {
// normalize B-C
if (arguments.length === 1 && typeof error === 'string') {
content = error
error = null
}
if (error) {
file.content = null
file.contentPath = null
return done(error)
}
if (!preprocessors.length) {
file.contentPath = null
file.content = content
file.sha = sha1(content)
return done()
}
preprocessors.shift()(content, file, nextPreprocessor)
}Now, if you notice, when So, I think my first solution still works, but another solution would be to modify the Proposed solution for https://github.com/karma-runner/karma-coverage/blob/master/lib/preprocessor.js#L112 instrumenter.instrument(content, jsPath, function (err, instrumentedCode) {
if (err) {
log.error('%s\n at %s', err.message, file.originalPath);
done(err.message)
} else {
if (file.sourceMap && instrumenter.lastSourceMap()) {
log.debug('Adding source map to instrumented file for "%s".', file.originalPath)
var generator = SourceMapGenerator.fromSourceMap(new SourceMapConsumer(instrumenter.lastSourceMap().toString()))
generator.applySourceMap(new SourceMapConsumer(file.sourceMap))
file.sourceMap = JSON.parse(generator.toString())
instrumentedCode += '\n//# sourceMappingURL=data:application/json;charset=utf-8;base64,'
instrumentedCode += new Buffer(JSON.stringify(file.sourceMap)).toString('base64') + '\n'
}
// remember the actual immediate instrumented JS for given original path
sourceCache[jsPath] = content
if (includeAllSources) {
// reset stateful regex
coverageObjRegex.lastIndex = 0
var coverageObjMatch = coverageObjRegex.exec(instrumentedCode)
if (coverageObjMatch !== null) {
var coverageObj = JSON.parse(coverageObjMatch[0])
coverageMap.add(coverageObj)
}
}
done(instrumentedCode)
}
})I know the proposed soltion is not part of this repo, but I think my first solution is still valid. Will also open an isue in the karma-coverage repo. @dignifiedquire or anyone else, let me know if what I say makes sense. I think I will create a PR in karma-coverage. Thanks!! |
|
Thanks for the detailed analysis and debugging and report! I really appreciate it (please tell everybody else to report more issues like this 😄) |
|
Yes please open a PR to karma-coverage-reporter. (Yes I'm maintaining pretty much everything under the karma-runner organization) Should get this merged and fixed pretty quick I think. |
|
I'll try to tell people but wont promise any results 😄 😄 😄 |
|
PR Created . karma-runner/karma-coverage#204 |
|
Sounds like this issue can be closed then? |
I'm in the process of upgrading karma form 0.10.x to 0.13.x. Everything is working except for one thing (that used to work before the upgrade), when I want to continuously run my tests for any code change(singleRun === false), whenever my code is invalid (lets say I delete a closing parentheses in an
ifstatement something like...if(x{ return false}and I save the file ), karma breaks with the following error :After digging the whole day in this issue, I discovered that the break happens from
v0.11.5tov0.11.6. Specifically this change: 6e31cb2#diff-1006b0ebc64f4a0530e73029eed686b0R56 is the one responsible for the break.I'm not familiar with the karma codebase, but after some debugging I noticed that, when my javascript code is invalid and the coverage preprocessor processes my js code, the
errorparameter changes fromnulltoundefined(null when there is no errors,undefinedwhen there is errors). Changing https://github.com/karma-runner/karma/blob/master/lib/preprocessor.js#L64 fromif (error) {toif (error || typeof error === "undefined") {seems to solve the error and I'm able to watch continuously for changes.Not sure if this is the right solution, but if it is, I could create a PR with this simple change if you want me to.
This is related with #520 and might be related with the issue closed recently due to inactivity #847
The text was updated successfully, but these errors were encountered: