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

Remove obsolete done callbacks from the unit tests #13216

Merged
merged 1 commit into from
Apr 10, 2021

Conversation

timvandermeij
Copy link
Contributor

The done callbacks are an outdated mechanism to signal Jasmine that a unit test is done, mostly in cases where a unit test needed to wait for an asynchronous operation to complete before doing its assertions. Nowadays a much better mechanism is in place for that, namely simply passing an asynchronous function to Jasmine, so we don't need callbacks anymore (which require more code and may be more difficult to reason about).

In these particular cases though the done callbacks never had any real use since nothing asynchronous happens in these places. Synchronous functions don't need to use done callbacks since Jasmine simply knows it's done when the function reaches its normal end, so we can safely get rid of these callbacks. The telltale sign is if the done callback is used unconditionally at the end of the function.

This is all done in an effort to over time get rid of all callbacks in the unit test code.

The done callbacks are an outdated mechanism to signal Jasmine that a
unit test is done, mostly in cases where a unit test needed to wait for
an asynchronous operation to complete before doing its assertions.
Nowadays a much better mechanism is in place for that, namely simply
passing an asynchronous function to Jasmine, so we don't need callbacks
anymore (which require more code and may be more difficult to reason
about).

In these particular cases though the done callbacks never had any real
use since nothing asynchronous happens in these places. Synchronous
functions don't need to use done callbacks since Jasmine simply knows
it's done when the function reaches its normal end, so we can safely get
rid of these callbacks. The telltale sign is if the done callback is
used unconditionally at the end of the function.

This is all done in an effort to over time get rid of all callbacks in
the unit test code.
@timvandermeij
Copy link
Contributor Author

/botio unittest

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_unittest from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/2e243f927c004b4/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_unittest from @timvandermeij received. Current queue size: 0

Live output at: http://3.101.106.178:8877/a5a0338f5970d01/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/2e243f927c004b4/output.txt

Total script time: 3.76 mins

  • Unit Tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://3.101.106.178:8877/a5a0338f5970d01/output.txt

Total script time: 5.31 mins

  • Unit Tests: Passed

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you for doing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants