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

[Edge, Safari] ckeditor5-editor-* CI jobs fail on 15th test #1715

Closed
Reinmar opened this issue Apr 18, 2019 · 7 comments
Closed

[Edge, Safari] ckeditor5-editor-* CI jobs fail on 15th test #1715

Reinmar opened this issue Apr 18, 2019 · 7 comments
Assignees
Labels
type:task This issue reports a chore (non-production change) and other types of "todos".
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Apr 18, 2019

https://travis-ci.org/ckeditor/ckeditor5-editor-inline/jobs/521399249

image

My guess will be the memory leak tests make that browser unresponsive. I'd switch them off on Edge because its engine is dead anyway.

@Reinmar
Copy link
Member Author

Reinmar commented Apr 18, 2019

Those tests should be skipped when window.gc is not present:

function skipIfNoGarbageCollector() {
	before( function() {
		if ( !window.gc ) {
			this.skip();
		}
	} );
}

But maybe this object is available on Edge? :| Or I'm mistaken that this is about that test.

@Reinmar Reinmar added status:confirmed type:task This issue reports a chore (non-production change) and other types of "todos". labels Apr 18, 2019
@Reinmar Reinmar added this to the iteration 24 milestone Apr 18, 2019
@Reinmar
Copy link
Member Author

Reinmar commented Apr 18, 2019

One more job: https://travis-ci.org/ckeditor/ckeditor5-editor-balloon/jobs/521399230

Here, both Safari and Edge failed:

image

I've just checked that this.skip() is either not called or it doesn't make the test abort from being executed (and just skips the results). We need to make sure those tests are not executed at all.

@Reinmar Reinmar changed the title [Edge] ckeditor5-editor-* CI jobs fail on 15th test [Edge, Safari] ckeditor5-editor-* CI jobs fail on 15th test Apr 18, 2019
@pomek
Copy link
Member

pomek commented Apr 18, 2019

It is not about the GC. This test is invalid and breaks everything:

it( 'initializes with config.initialData', () => {
	return InlineEditor.create( editorElement, {
		initialData: '<p>Hello world!</p>',
		plugins: [ Paragraph ]
	} ).then( editor => {
		expect( editor.getData() ).to.equal( '<p>Hello world!</p>' );

		editor.destroy();
	} );
} );

@pomek
Copy link
Member

pomek commented Apr 18, 2019

It should be:

it( 'initializes with config.initialData', () => {
	const editorElement = document.createElement( 'div' );
	document.body.appendChild( editorElement );

	return InlineEditor.create( editorElement, {
		initialData: '<p>Hello world!</p>',
		plugins: [ Paragraph ]
	} ).then( editor => {
		expect( editor.getData() ).to.equal( '<p>Hello world!</p>' );

		editor.destroy();
		editorElement.remove();
	} );
} );

or even more correct solution:

it( 'initializes with config.initialData', () => {
	const editorElement = document.createElement( 'div' );
	document.body.appendChild( editorElement );

	return InlineEditor.create( editorElement, {
		initialData: '<p>Hello world!</p>',
		plugins: [ Paragraph ]
	} ).then( editor => {
		expect( editor.getData() ).to.equal( '<p>Hello world!</p>' );

		return editor.destroy();
	} ).then( () => {
		editorElement.remove();
	} );
} );

@pomek
Copy link
Member

pomek commented Apr 18, 2019

I assume that the editor does not know what to do. We passed initialData and we have HTML code as initial data in editorElement.

https://github.com/ckeditor/ckeditor5-editor-inline/blob/1f47051035b38fe049719fa062c75523d1a466c5/tests/inlineeditor.js#L33-L34

@mlewand mlewand modified the milestones: iteration 24, iteration 25 May 27, 2019
@pomek
Copy link
Member

pomek commented Jun 15, 2019

Because we removed BrowserStack from our CI (#1742 (comment)), could we agree that this ticket is no longer valid?

@pomek
Copy link
Member

pomek commented Jun 24, 2019

After removing BS, all tests pass. I am closing the ticket.

@pomek pomek closed this as completed Jun 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

No branches or pull requests

3 participants