Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve stability of package and tests #126

Merged
merged 11 commits into from
Aug 7, 2020
98 changes: 43 additions & 55 deletions src/ckeditor/ckeditor.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@
*/

import waitUntil from 'wait-until-promise';
import { async, ComponentFixture, TestBed } from '@angular/core/testing';
Dumluregn marked this conversation as resolved.
Show resolved Hide resolved
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { CKEditorComponent } from './ckeditor.component';
import {
fireDragEvent,
mockDropEvent,
mockPasteEvent,
Dumluregn marked this conversation as resolved.
Show resolved Hide resolved
setDataMultipleTimes,
whenDataReady,
whenEvent
Expand All @@ -24,11 +23,11 @@ describe( 'CKEditorComponent', () => {
fixture: ComponentFixture<CKEditorComponent>,
config: Object;

beforeEach( async( () => {
TestBed.configureTestingModule( {
beforeEach( () => {
return TestBed.configureTestingModule( {
declarations: [ CKEditorComponent ]
} ).compileComponents();
} ) );
} )

beforeEach( () => {
fixture = TestBed.createComponent( CKEditorComponent );
Expand Down Expand Up @@ -73,9 +72,10 @@ describe( 'CKEditorComponent', () => {
} );

it( 'should have proper editor type', () => {
whenEvent( 'ready', component ).then( () => {
return whenEvent( 'ready', component ).then( () => {
fixture.detectChanges();
expect( component.instance.editable().isInline() ).toBe( component.type !== EditorType.CLASSIC );
expect( component.instance.editable().isInline() )
.toBe( component.type !== EditorType.CLASSIC );
} );
} );

Expand Down Expand Up @@ -141,7 +141,7 @@ describe( 'CKEditorComponent', () => {
warn: isDivarea
} ].forEach( ( { newConfig, msg, warn } ) => {
describe( msg, () => {
beforeAll( () => {
beforeEach( () => {
config = newConfig;
} );

Expand Down Expand Up @@ -170,14 +170,15 @@ describe( 'CKEditorComponent', () => {
} );

describe( 'when set with config', () => {
beforeEach( done => {
beforeEach( () => {
component.config = {
readOnly: true,
width: 1000,
height: 1000
};
fixture.detectChanges();
whenEvent( 'ready', component ).then( done );

return whenEvent( 'ready', component );
} );

it( 'editor should be readOnly', () => {
Expand All @@ -193,84 +194,80 @@ describe( 'CKEditorComponent', () => {
expect( component.instance.plugins.undo ).not.toBeUndefined();
} );

it( 'should register changes', async done => {
it( 'should register changes', () => {
const spy = jasmine.createSpy();

component.registerOnChange( spy );

setDataMultipleTimes( component.instance, [
return setDataMultipleTimes( component.instance, [
'<p>Hello World!</p>',
'<p>I am CKEditor for Angular!</p>'
] ).then( () => {
expect( spy ).toHaveBeenCalledTimes( 2 );

done();
} );
} );
} );

describe( 'when set without undo plugin', () => {
beforeEach( ( done ) => {
beforeEach( () => {
component.config = {
removePlugins: 'undo'
};
fixture.detectChanges();
whenEvent( 'ready', component ).then( done );
return whenEvent( 'ready', component );
} );

it( 'editor should not have undo plugin', () => {
expect( component.instance.plugins.undo ).toBeUndefined();
} );

it( 'should register changes without undo plugin', async done => {
it( 'should register changes without undo plugin', () => {
const spy = jasmine.createSpy();

component.registerOnChange( spy );

setDataMultipleTimes( component.instance, [
return setDataMultipleTimes( component.instance, [
'<p>Hello World!</p>',
'<p>I am CKEditor for Angular!</p>'
] ).then( () => {
expect( spy ).toHaveBeenCalledTimes( 2 );

done();
} );
} );
} );
} );

describe( 'on destroy', () => {
it ( 'should not have call runOutsideAngular when destroy before DOM loaded', done => {
it ( 'should not have call runOutsideAngular when destroy before DOM loaded', () => {
spyOn( fixture.ngZone, 'runOutsideAngular' );

fixture.detectChanges();

waitUntil( () => {
return waitUntil( () => {
fixture.destroy();
return true;
}, 0 ).then( () => {
expect( fixture.ngZone.runOutsideAngular ).toHaveBeenCalledTimes( 1 );
} ).then( done );
} );
} );

it ( 'should not have call runOutsideAngular when destroy before DOM loaded', done => {
it ( 'should not have call runOutsideAngular when destroy before DOM loaded', () => {
spyOn( fixture.ngZone, 'runOutsideAngular' );

fixture.detectChanges();

waitUntil( () => {
return waitUntil( () => {
fixture.destroy();
return true;
}, 200 ).then( () => {
expect( fixture.ngZone.runOutsideAngular ).toHaveBeenCalledTimes( 1 );
} ).then( done );
} );
} );
} );

describe( 'when component is ready', () => {
beforeEach( done => {
beforeEach( () => {
fixture.detectChanges();
whenEvent( 'ready', component ).then( done );
return whenEvent( 'ready', component );
} );

it( 'should be initialized', () => {
Expand Down Expand Up @@ -319,18 +316,16 @@ describe( 'CKEditorComponent', () => {
const data = '<b>foo</b>',
expected = '<p><strong>foo</strong></p>\n';

it( 'should be configurable at the start of the component', async done => {
it( 'should be configurable at the start of the component', async () => {
fixture.detectChanges();

await whenDataReady( component.instance, () => component.data = data );

expect( component.data ).toEqual( expected );
expect( component.instance.getData() ).toEqual( expected );

done();
} );

it( 'should be writeable by ControlValueAccessor', async done => {
it( 'should be writeable by ControlValueAccessor', async () => {
fixture.detectChanges();

const editor = component.instance;
Expand All @@ -342,8 +337,6 @@ describe( 'CKEditorComponent', () => {
await whenDataReady( editor, () => component.writeValue( '<p><i>baz</i></p>' ) );

expect( component.instance.getData() ).toEqual( '<p><em>baz</em></p>\n' );

done();
} );
} );

Expand Down Expand Up @@ -423,7 +416,7 @@ describe( 'CKEditorComponent', () => {
return eventPromise;
} );

it( 'drag/drop events should emit component dragStart, dragEnd and drop', async done => {
it( 'drag/drop events should emit component dragStart, dragEnd and drop', () => {
fixture.detectChanges();

const spyDragStart = jasmine.createSpy( 'dragstart' );
Expand All @@ -435,30 +428,26 @@ describe( 'CKEditorComponent', () => {
const spyDrop = jasmine.createSpy( 'drop' );
component.drop.subscribe( spyDrop );

whenDataReady( component.instance, () => {
const dropEvent = mockDropEvent();
const paragraph = component.instance.editable().findOne( 'p' );
const dropEvent = mockDropEvent();
const paragraph = component.instance.editable().findOne( 'p' );

component.instance.getSelection().selectElement( paragraph );
component.instance.getSelection().selectElement( paragraph );

fireDragEvent( 'dragstart', component.instance, dropEvent );
fireDragEvent( 'dragstart', component.instance, dropEvent );

expect( spyDragStart ).toHaveBeenCalledTimes( 1 );
expect( spyDragStart ).toHaveBeenCalledTimes( 1 );

fireDragEvent( 'dragend', component.instance, dropEvent );
fireDragEvent( 'dragend', component.instance, dropEvent );

expect( spyDragEnd ).toHaveBeenCalledTimes( 1 );
expect( spyDragEnd ).toHaveBeenCalledTimes( 1 );

// There is some issue in Firefox with simulating drag-drop flow. The drop event
// is not fired making this assertion fail. Let's skip it for now.
if ( !CKEDITOR.env.gecko ) {
fireDragEvent( 'drop', component.instance, dropEvent );
// There is some issue in Firefox with simulating drag-drop flow. The drop event
// is not fired making this assertion fail. Let's skip it for now.
if ( !CKEDITOR.env.gecko ) {
fireDragEvent( 'drop', component.instance, dropEvent );

expect( spyDrop ).toHaveBeenCalledTimes( 1 );
}

done();
} );
expect( spyDrop ).toHaveBeenCalledTimes( 1 );
}
} );

it( 'fileUploadRequest should emit component fileUploadRequest', () => {
Expand Down Expand Up @@ -517,17 +506,16 @@ describe( 'CKEditorComponent', () => {
expect( spy ).toHaveBeenCalled();
} );

it( 'onChange callback should be called when editor model changes', async done => {
it( 'onChange callback should be called when editor model changes', () => {
fixture.detectChanges();

const spy = jasmine.createSpy();
component.registerOnChange( spy );

setDataMultipleTimes( component.instance, [
return setDataMultipleTimes( component.instance, [
'initial', 'initial', 'modified'
] ).then( () => {
expect( spy ).toHaveBeenCalledTimes( 2 );
done();
} );
} );
} );
Expand Down
26 changes: 14 additions & 12 deletions src/ckeditor/ckeditor.helpers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ declare let CKEDITOR: any;

describe( 'getEditorNamespace', () => {
const fakeScript = 'data:text/javascript;base64,d2luZG93LkNLRURJVE9SID0ge307';
let isIe10 = navigator.userAgent.toLowerCase().indexOf( 'trident/' ) > -1;
isIe10 = isIe10 && document[ 'documentMode' ] === 10;

beforeEach( () => {
delete window[ 'CKEDITOR' ];
Expand All @@ -21,9 +19,10 @@ describe( 'getEditorNamespace', () => {
} );

it( 'typeError thrown when empty string passed', () => {
expect( () => {
getEditorNamespace( '' );
} ).toThrowError( 'CKEditor URL must be a non-empty string.' );
return getEditorNamespace( '' ).catch( err => {
expect( err instanceof Error );
expect( err.message ).toEqual( 'CKEditor URL must be a non-empty string.' );
} );
} );

it( 'returns promise even if namespace is present', () => {
Expand All @@ -37,13 +36,11 @@ describe( 'getEditorNamespace', () => {
} );
} );

if ( !isIe10 ) { // Ignore unstable test on IE10.
it( 'load script and resolves with loaded namespace', () => {
return getEditorNamespace( fakeScript ).then( namespace => {
expect( namespace ).toBe( CKEDITOR );
} );
it( 'load script and resolves with loaded namespace', () => {
return getEditorNamespace( fakeScript ).then( namespace => {
expect( namespace ).toBe( CKEDITOR );
} );
}
} );

it( 'rejects with error when script cannot be loaded', () => {
return getEditorNamespace( 'non-existent.js' ).catch( err => {
Expand All @@ -52,6 +49,11 @@ describe( 'getEditorNamespace', () => {
} );

it( 'returns the same promise', () => {
expect( getEditorNamespace( fakeScript ) ).toBe( getEditorNamespace( fakeScript ) );
const promise1 = getEditorNamespace( fakeScript );
const promise2 = getEditorNamespace( fakeScript );

expect( promise1 ).toBe( promise2 );

return Promise.all( [ promise1, promise2 ] )
} );
} );
5 changes: 3 additions & 2 deletions src/ckeditor/ckeditor.helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ let promise;

export function getEditorNamespace( editorURL: string ): Promise<{ [ key: string ]: any; }> {
if ( editorURL.length < 1 ) {
throw new TypeError( 'CKEditor URL must be a non-empty string.' );
return Promise.reject( new TypeError( 'CKEditor URL must be a non-empty string.' ) );
}

if ( 'CKEDITOR' in window ) {
Expand All @@ -22,8 +22,9 @@ export function getEditorNamespace( editorURL: string ): Promise<{ [ key: string
scriptReject( err );
} else {
scriptResolve( CKEDITOR );
promise = undefined;
}

promise = undefined;
} );
} );
}
Expand Down