From dd3f77af5fa360bd1c2be6c8b5424b84976dc21a Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Wed, 10 Jul 2019 14:32:38 +0200 Subject: [PATCH 1/2] Removed usage of logger. --- src/template.js | 9 +++--- src/toolbar/toolbarview.js | 9 +++--- tests/template.js | 60 +++++++++++++++++------------------- tests/toolbar/toolbarview.js | 13 +++----- 4 files changed, 44 insertions(+), 47 deletions(-) diff --git a/src/template.js b/src/template.js index 621e5227..0dbbfb0d 100644 --- a/src/template.js +++ b/src/template.js @@ -7,15 +7,14 @@ * @module ui/template */ -/* global document */ +/* global document, console */ -import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; +import CKEditorError, { attachLinkToDocumentation } from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import mix from '@ckeditor/ckeditor5-utils/src/mix'; import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin'; import View from './view'; import ViewCollection from './viewcollection'; import isNode from '@ckeditor/ckeditor5-utils/src/dom/isnode'; -import log from '@ckeditor/ckeditor5-utils/src/log'; import { isObject, cloneDeepWith } from 'lodash-es'; const xhtmlNs = 'http://www.w3.org/1999/xhtml'; @@ -381,7 +380,9 @@ export default class Template { * * @error template-extend-render */ - log.warn( 'template-extend-render: Attempting to extend a template which has already been rendered.' ); + console.warn( attachLinkToDocumentation( + 'template-extend-render: Attempting to extend a template which has already been rendered.' + ) ); } extendTemplate( template, normalize( clone( def ) ) ); diff --git a/src/toolbar/toolbarview.js b/src/toolbar/toolbarview.js index ee95d3a1..287c789d 100644 --- a/src/toolbar/toolbarview.js +++ b/src/toolbar/toolbarview.js @@ -7,15 +7,17 @@ * @module ui/toolbar/toolbarview */ +/* globals console */ + import View from '../view'; import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; import FocusCycler from '../focuscycler'; import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler'; import ToolbarSeparatorView from './toolbarseparatorview'; import preventDefault from '../bindings/preventdefault.js'; -import log from '@ckeditor/ckeditor5-utils/src/log'; import '../../theme/components/toolbar/toolbar.css'; +import { attachLinkToDocumentation } from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; /** * The toolbar view class. @@ -180,10 +182,9 @@ export default class ToolbarView extends View { * @error toolbarview-item-unavailable * @param {String} name The name of the component. */ - log.warn( + console.warn( attachLinkToDocumentation( 'toolbarview-item-unavailable: The requested toolbar item is unavailable.', - { name } - ); + ), { name } ); } } ); } diff --git a/tests/template.js b/tests/template.js index f5d5078f..94919db6 100644 --- a/tests/template.js +++ b/tests/template.js @@ -3,9 +3,8 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* globals HTMLElement, Event, document */ +/* globals HTMLElement, Event, document, console */ -import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import { default as Template, TemplateToBinding, TemplateIfBinding } from '../src/template'; import View from '../src/view'; import ViewCollection from '../src/viewcollection'; @@ -14,7 +13,6 @@ import Model from '../src/model'; import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin'; import DomEmitterMixin from '@ckeditor/ckeditor5-utils/src/dom/emittermixin'; import normalizeHtml from '@ckeditor/ckeditor5-utils/tests/_utils/normalizehtml'; -import log from '@ckeditor/ckeditor5-utils/src/log'; import { expectToThrowCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; @@ -22,10 +20,10 @@ let el, text; const injectedElements = []; describe( 'Template', () => { - testUtils.createSinonSandbox(); - // Clean-up document.body from the rendered elements. afterEach( () => { + sinon.restore(); + for ( const el of injectedElements ) { el.remove(); } @@ -1026,9 +1024,9 @@ describe( 'Template', () => { '
ab
' ); - const spy1 = testUtils.sinon.spy(); - const spy2 = testUtils.sinon.spy(); - const spy3 = testUtils.sinon.spy(); + const spy1 = sinon.spy(); + const spy2 = sinon.spy(); + const spy3 = sinon.spy(); observable.on( 'ku', spy1 ); observable.on( 'kd', spy2 ); @@ -1593,7 +1591,7 @@ describe( 'Template', () => { } ); it( 'accepts plain binding', () => { - const spy = testUtils.sinon.spy(); + const spy = sinon.spy(); setElement( { tag: 'p', @@ -1612,8 +1610,8 @@ describe( 'Template', () => { } ); it( 'accepts an array of event bindings', () => { - const spy1 = testUtils.sinon.spy(); - const spy2 = testUtils.sinon.spy(); + const spy1 = sinon.spy(); + const spy2 = sinon.spy(); setElement( { tag: 'p', @@ -1640,9 +1638,9 @@ describe( 'Template', () => { } ); it( 'accepts DOM selectors', () => { - const spy1 = testUtils.sinon.spy(); - const spy2 = testUtils.sinon.spy(); - const spy3 = testUtils.sinon.spy(); + const spy1 = sinon.spy(); + const spy2 = sinon.spy(); + const spy3 = sinon.spy(); setElement( { tag: 'p', @@ -1721,8 +1719,8 @@ describe( 'Template', () => { } ); it( 'accepts function callbacks', () => { - const spy1 = testUtils.sinon.spy(); - const spy2 = testUtils.sinon.spy(); + const spy1 = sinon.spy(); + const spy2 = sinon.spy(); setElement( { tag: 'p', @@ -1753,7 +1751,7 @@ describe( 'Template', () => { } ); it( 'supports event delegation', () => { - const spy = testUtils.sinon.spy(); + const spy = sinon.spy(); setElement( { tag: 'p', @@ -1777,7 +1775,7 @@ describe( 'Template', () => { } ); it( 'works for future elements', () => { - const spy = testUtils.sinon.spy(); + const spy = sinon.spy(); setElement( { tag: 'p', @@ -1811,7 +1809,7 @@ describe( 'Template', () => { describe( 'to', () => { it( 'returns an instance of TemplateToBinding', () => { - const spy = testUtils.sinon.spy(); + const spy = sinon.spy(); const binding = bind.to( 'foo', spy ); expect( binding ).to.be.instanceof( TemplateToBinding ); @@ -2034,7 +2032,7 @@ describe( 'Template', () => { describe( 'if', () => { it( 'returns an object which describes the binding', () => { - const spy = testUtils.sinon.spy(); + const spy = sinon.spy(); const binding = bind.if( 'foo', 'whenTrue', spy ); expect( binding ).to.be.instanceof( TemplateIfBinding ); @@ -2334,7 +2332,7 @@ describe( 'Template', () => { } ); it( 'logs a warning if an element has already been rendered', () => { - const spy = testUtils.sinon.stub( log, 'warn' ); + const consoleWarnStub = sinon.stub( console, 'warn' ); const tpl = new Template( { tag: 'p' @@ -2354,8 +2352,8 @@ describe( 'Template', () => { } } ); - sinon.assert.calledOnce( spy ); - sinon.assert.calledWithExactly( spy, sinon.match( /^template-extend-render/ ) ); + sinon.assert.calledOnce( consoleWarnStub ); + sinon.assert.calledWithExactly( consoleWarnStub, sinon.match( /^template-extend-render/ ) ); } ); describe( 'attributes', () => { @@ -2918,11 +2916,11 @@ describe( 'Template', () => { describe( 'listeners', () => { it( 'extends existing', () => { - const spy1 = testUtils.sinon.spy(); - const spy2 = testUtils.sinon.spy(); - const spy3 = testUtils.sinon.spy(); - const spy4 = testUtils.sinon.spy(); - const spy5 = testUtils.sinon.spy(); + const spy1 = sinon.spy(); + const spy2 = sinon.spy(); + const spy3 = sinon.spy(); + const spy4 = sinon.spy(); + const spy5 = sinon.spy(); observable.on( 'A', spy1 ); observable.on( 'C', spy2 ); @@ -2973,9 +2971,9 @@ describe( 'Template', () => { } ); it( 'creates new', () => { - const spy1 = testUtils.sinon.spy(); - const spy2 = testUtils.sinon.spy(); - const spy3 = testUtils.sinon.spy(); + const spy1 = sinon.spy(); + const spy2 = sinon.spy(); + const spy3 = sinon.spy(); observable.on( 'A', spy1 ); observable.on( 'B', spy2 ); diff --git a/tests/toolbar/toolbarview.js b/tests/toolbar/toolbarview.js index 98b63a45..eb34e782 100644 --- a/tests/toolbar/toolbarview.js +++ b/tests/toolbar/toolbarview.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* global document, Event */ +/* global document, Event, console */ import ToolbarView from '../../src/toolbar/toolbarview'; import ToolbarSeparatorView from '../../src/toolbar/toolbarseparatorview'; @@ -13,15 +13,11 @@ import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; import FocusCycler from '../../src/focuscycler'; import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; import ViewCollection from '../../src/viewcollection'; -import log from '@ckeditor/ckeditor5-utils/src/log'; import View from '../../src/view'; -import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; describe( 'ToolbarView', () => { let locale, view; - testUtils.createSinonSandbox(); - beforeEach( () => { locale = {}; view = new ToolbarView( locale ); @@ -29,6 +25,7 @@ describe( 'ToolbarView', () => { } ); afterEach( () => { + sinon.restore(); view.destroy(); } ); @@ -306,7 +303,7 @@ describe( 'ToolbarView', () => { it( 'warns if there is no such component in the factory', () => { const items = view.items; - testUtils.sinon.stub( log, 'warn' ); + const consoleWarnStub = sinon.stub( console, 'warn' ); view.fillFromConfig( [ 'foo', 'bar', 'baz' ], factory ); @@ -314,8 +311,8 @@ describe( 'ToolbarView', () => { expect( items.get( 0 ).name ).to.equal( 'foo' ); expect( items.get( 1 ).name ).to.equal( 'bar' ); - sinon.assert.calledOnce( log.warn ); - sinon.assert.calledWithExactly( log.warn, + sinon.assert.calledOnce( consoleWarnStub ); + sinon.assert.calledWithExactly( consoleWarnStub, sinon.match( /^toolbarview-item-unavailable/ ), { name: 'baz' } ); From 3eb5d3fce2d0a27f035cef6a0384839e049858c0 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Fri, 12 Jul 2019 12:30:29 +0200 Subject: [PATCH 2/2] Fixed errors/warnings. --- src/template.js | 11 ++++++----- tests/template.js | 21 +++++++++------------ 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/template.js b/src/template.js index 0dbbfb0d..b1f814ce 100644 --- a/src/template.js +++ b/src/template.js @@ -7,9 +7,9 @@ * @module ui/template */ -/* global document, console */ +/* global document */ -import CKEditorError, { attachLinkToDocumentation } from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; +import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import mix from '@ckeditor/ckeditor5-utils/src/mix'; import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin'; import View from './view'; @@ -380,9 +380,10 @@ export default class Template { * * @error template-extend-render */ - console.warn( attachLinkToDocumentation( - 'template-extend-render: Attempting to extend a template which has already been rendered.' - ) ); + throw new CKEditorError( + 'template-extend-render: Attempting to extend a template which has already been rendered.', + [ this, template ] + ); } extendTemplate( template, normalize( clone( def ) ) ); diff --git a/tests/template.js b/tests/template.js index 94919db6..9db30397 100644 --- a/tests/template.js +++ b/tests/template.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* globals HTMLElement, Event, document, console */ +/* globals HTMLElement, Event, document */ import { default as Template, TemplateToBinding, TemplateIfBinding } from '../src/template'; import View from '../src/view'; @@ -2331,9 +2331,7 @@ describe( 'Template', () => { expect( tpl.attributes.b[ 0 ] ).to.equal( 'bar' ); } ); - it( 'logs a warning if an element has already been rendered', () => { - const consoleWarnStub = sinon.stub( console, 'warn' ); - + it( 'throws an error if an element has already been rendered', () => { const tpl = new Template( { tag: 'p' } ); @@ -2346,14 +2344,13 @@ describe( 'Template', () => { tpl.render(); - Template.extend( tpl, { - attributes: { - class: 'bar' - } - } ); - - sinon.assert.calledOnce( consoleWarnStub ); - sinon.assert.calledWithExactly( consoleWarnStub, sinon.match( /^template-extend-render/ ) ); + expectToThrowCKEditorError( () => { + Template.extend( tpl, { + attributes: { + class: 'bar' + } + } ); + }, /^template-extend-render/ ); } ); describe( 'attributes', () => {