Skip to content

Commit

Permalink
fix: filter browser logging by level of LOG
Browse files Browse the repository at this point in the history
Additional fix to the bug, "filter browser logging by level" ([35965d9](35965d9)), closes [#2228](#2228)

Otherwise console.log ignore doesn't work with 
```
browserConsoleLogOptions: {
  level: 'debug',
  terminal: true
}
```
  • Loading branch information
blackswanny authored and dignifiedquire committed Feb 19, 2017
1 parent eaeffe9 commit 89a7a1c
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 4 deletions.
3 changes: 2 additions & 1 deletion lib/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@ exports.LOG_ERROR = 'ERROR'
exports.LOG_WARN = 'WARN'
exports.LOG_INFO = 'INFO'
exports.LOG_DEBUG = 'DEBUG'
exports.LOG_LOG = 'LOG'
exports.LOG_PRIORITIES = [
exports.LOG_DISABLE,
exports.LOG_ERROR,
exports.LOG_WARN,
exports.LOG_INFO,
exports.LOG_DEBUG,
exports.LOG_DISABLE
exports.LOG_LOG
]

// Default patterns for the pattern layout.
Expand Down
5 changes: 4 additions & 1 deletion test/e2e/browser_console.feature
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ Feature: Browser Console Configuration
plugins = [
'karma-jasmine',
'karma-phantomjs-launcher'
];
],
browserConsoleLogOptions = {
level: 'log'
};
"""
When I start Karma
Then it passes with like:
Expand Down
40 changes: 38 additions & 2 deletions test/unit/reporters/base.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,45 @@ describe('reporter', function () {
return expect(writeSpy).to.have.been.calledWith('LOG: Message\n')
})

it('should not log if lower priority than browserConsoleLogOptions.level', function () {
it('should not log if lower priority than browserConsoleLogOptions "error"', function () {
var reporter = new m.BaseReporter(null, null, true, {
browserConsoleLogOptions: {level: 'ERROR'}
browserConsoleLogOptions: {level: 'error'}
}, adapter)
var writeSpy = sinon.spy(reporter, 'writeCommonMsg')

reporter._browsers = ['Chrome']
reporter.onBrowserLog('Chrome', 'Message', 'WARN')

return writeSpy.should.have.not.been.called
})

it('should not log if lower priority than browserConsoleLogOptions "warn"', function () {
var reporter = new m.BaseReporter(null, null, true, {
browserConsoleLogOptions: {level: 'warn'}
}, adapter)
var writeSpy = sinon.spy(reporter, 'writeCommonMsg')

reporter._browsers = ['Chrome']
reporter.onBrowserLog('Chrome', 'Message', 'INFO')

return writeSpy.should.have.not.been.called
})

it('should not log if lower priority than browserConsoleLogOptions "info"', function () {
var reporter = new m.BaseReporter(null, null, true, {
browserConsoleLogOptions: {level: 'info'}
}, adapter)
var writeSpy = sinon.spy(reporter, 'writeCommonMsg')

reporter._browsers = ['Chrome']
reporter.onBrowserLog('Chrome', 'Message', 'DEBUG')

return writeSpy.should.have.not.been.called
})

it('should not log if lower priority than browserConsoleLogOptions "debug"', function () {
var reporter = new m.BaseReporter(null, null, true, {
browserConsoleLogOptions: {level: 'debug'}
}, adapter)
var writeSpy = sinon.spy(reporter, 'writeCommonMsg')

Expand Down

3 comments on commit 89a7a1c

@nello
Copy link

@nello nello commented on 89a7a1c Feb 23, 2017

Choose a reason for hiding this comment

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

So, if you don't add the following to your config, console.log() is disabled? Oh well done.

// turn console.log back on
browserConsoleLogOptions: {
  level: 'log',
},

@mgol
Copy link
Contributor

@mgol mgol commented on 89a7a1c Apr 26, 2017

Choose a reason for hiding this comment

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

I think those tests are incorrect - if I change the priority to be higher instead of lower in various tests they still pass... I stumbled upon it trying to fix #2582, see my comment there.

@mgol
Copy link
Contributor

@mgol mgol commented on 89a7a1c Apr 26, 2017

Choose a reason for hiding this comment

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

I fixed those tests in PR #2676.

Please sign in to comment.