Unstubbed console.log/error/warn() should throw in tests

[Reinmar]

Extracted from #1970

Make sure we throw on console.log/error() in the tests. Not sure how, but I guess we can load some file which overrides them as the very first test (just add it in the karma's config). There may be a more "standard" way to do so too.

Then:

Make sure all tests pass after the above.

[Reinmar]

After @ma2ciek:

Make sure we throw on console.log/error() in the tests. Not sure how, but I guess we can load some file which overrides them as the very first test (just add it in the karma's config). There may be a more "standard" way to do so too.

I've tried the following added to the entry file of the automated tests and it's been working well:

beforeEach( () => {
	console.log = log => { originalWarn( 'Detected \`console.log()\`:' ); throw new Error( log ); };
	console.warn = warn => { originalWarn( 'Detected \`console.warn()\`:' ); throw new Error( warn ); };
	console.error = error => { originalWarn( 'Detected \`console.error()\`:' ); throw new Error( error ); };
} );

Although, shouldn't this behavior be configurable, e.g. consoleAllowed defaulted to false? I can imagine someone crying when debugging a broken test 😄

[Reinmar]

Although, shouldn't this behavior be configurable, e.g. consoleAllowed defaulted to false? I can imagine someone crying when debugging a broken test 😄

Doh, I didn't think about this ;O

Let's ask the team.

[Reinmar]

I'm actually thinking that perhaps we should have --disallow-console-us and use it on CI. When running tests locally most of the time you're actually working on something and will use the console. At least, that's how I work.

[jodator]

I was going to write that I like the --allow-console more as it will fail faster locally (so before submitting a PR) but the --disallow-console-use would be less troublesome when working locally. We might need to remember about running tests with --disallow-console-use before submitting a PR.

[Reinmar]

To be clear, the final result should be resolving the issue described in #2038 once and for all.

[ma2ciek]

Actually, I wasn't 100% sure if I should test these console warns or not. In the --no-debug environment these tests would break because the console.warn() code would be still commented out.

The other thing is the weird Travis behavior - take a look at the latest master ckeditor5-engine test logs - https://travis-ci.org/ckeditor/ckeditor5-engine/builds/586428795 - there's no warning in the Chrome part, while they appear in the FF tests and on local Chrome as well.

[ma2ciek]

The above PRs fix the issue.