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

How to disable debug tools? #4214

Closed
pomek opened this issue Nov 28, 2017 · 5 comments · Fixed by ckeditor/ckeditor5-engine#1195
Closed

How to disable debug tools? #4214

pomek opened this issue Nov 28, 2017 · 5 comments · Fixed by ckeditor/ckeditor5-engine#1195
Assignees
Labels
package:engine type:task This issue reports a chore (non-production change) and other types of "todos".
Milestone

Comments

@pomek
Copy link
Member

pomek commented Nov 28, 2017

When tests are executed as a single entry point, two tests fail:

https://github.com/ckeditor/ckeditor5-engine/blob/82335aee6426a7418416927fbe0db5d471c71465/tests/model/document/document.js#L113-L141

and

https://github.com/ckeditor/ckeditor5-engine/blob/82335aee6426a7418416927fbe0db5d471c71465/tests/model/document/document.js#L143-L153

image

In order to fix these tests, I can add a method toJSON into operation object (toJSON: sinon.spy()). But I'm not sure whether this solution is proper. I would like to disable engine debug tools but I guess it's tricky.

This ticket is a last blocker for https://github.com/ckeditor/ckeditor5-dev/issues/290.

@Reinmar
Copy link
Member

Reinmar commented Nov 28, 2017

Do you mean that running all tests together means unintentionally enabling debug tools? That should be fixed in general, not just for broken tests.

@szymonkups
Copy link
Contributor

Do you mean that running all tests together means unintentionally enabling debug tools? That should be fixed in general, not just for broken tests.

When debug tools' tests are executed they're enabled for testing purposes but there is no functionality to disable them. We should create disableEngineDebug() function to revert changes made by enabling them.

@jodator
Copy link
Contributor

jodator commented Nov 28, 2017

I think that those tests are broken as they use Object instead of Operation instance for checking if something is done on operation.
Also this should be changed:
https://github.com/ckeditor/ckeditor5-engine/blob/ddb1c9f5a2017d2af3a57e39192f3b896452e186/src/dev-utils/enableenginedebug.js#L550
to JSON.stringify( operation ) as the toJSON() method will be called by JSON.stringify and rather should not be called directly.

@szymonkups
Copy link
Contributor

I think that those tests are broken as they use Object instead of Operation instance for checking if something is done on operation.

Yes, but the main problem is that engine debug tools are enabled and they shouldn't be. Please look at my comment here: https://github.com/ckeditor/ckeditor5-dev/issues/290#issuecomment-346858435.

@pomek pomek self-assigned this Nov 28, 2017
@pomek
Copy link
Member Author

pomek commented Nov 29, 2017

I decided to create a function which allows disabling engine debug functions.

ma2ciek referenced this issue in ckeditor/ckeditor5-engine Nov 30, 2017
Feature: Engine debug tools can be easily disabled using disableEngineDebug() function. Closes #1193.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the iteration 14 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:task This issue reports a chore (non-production change) and other types of "todos". package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants