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

Introduce a way to easily override current time in tests #4705

Closed
ma2ciek opened this issue Aug 5, 2019 · 0 comments · Fixed by ckeditor/ckeditor5-watchdog#18
Closed

Introduce a way to easily override current time in tests #4705

ma2ciek opened this issue Aug 5, 2019 · 0 comments · Fixed by ckeditor/ckeditor5-watchdog#18
Assignees
Labels
package:watchdog type:task This issue reports a chore (non-production change) and other types of "todos".
Milestone

Comments

@ma2ciek
Copy link
Contributor

ma2ciek commented Aug 5, 2019

Testing error handling depending on time is pretty complex because sinon.useFakeTimers() make async functions synchronous, which makes promises crashing instantly.

E.g. the following test will fail instantly when the CKEditorError error is thrown:

const watchdog = Watchdog.for( ClassicTestEditor );
const originalErrorHandler = window.onerror;
const clock = sinon.useFakeTimers();

window.onerror = undefined;

return watchdog.create( element ).then( () => {
	setTimeout( () => throwCKEditorError( 'foo', watchdog.editor ), 1000 );

	return new Promise( res => {
		watchdog.on( 'restart', () => {
			window.onerror = originalErrorHandler;

			watchdog.destroy().then( res );
		} );

		clock.tick( 1000 );
	} );
} );

as it boils down to something like this:

// some code
return new Promise( res => {
	// Some code.
	throwCKEditorError( 'foo', watchdog.editor );

	// Some code.
	res();
} );

Code after the error isn't executed, thus the test fails instantly. If the error would be in the setTimeout, even with timeout 0, the rest of the code would execute and such error wouldn't have an impact on the test behavior.

Thus I'm proposing adding the protected _now() method to the watchdog, which would return the Date.now(), so it could be overridden later in tests without affecting other mechanisms depending on time.

@ma2ciek ma2ciek self-assigned this Aug 7, 2019
pjasiun referenced this issue in ckeditor/ckeditor5-watchdog Aug 8, 2019
Fix: The editor data will be saved correctly after the `destroy()` method is called. Added the protected `Watchdog#_now()` method that allows for time-based testing of the error handling mechanism. Closes #17. Closes #19.
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-watchdog Oct 9, 2019
@mlewand mlewand added this to the iteration 26 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:watchdog labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:watchdog 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.

2 participants