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

Implement jasmine.clock().asyncTick() #1762

Closed
wants to merge 5 commits into from

Conversation

stephenfarrar
Copy link

@stephenfarrar stephenfarrar commented Oct 30, 2019

Description

Adds an .asyncTick() method to jasmine.Clock, and to the internal DelayedFunctionScheduler. These are just a slight variation on the existing .tick() methods, but they allow microtasks to run.

There is an existing loop in DelayedFunctionScheduler for running the scheduled timeouts/intervals.
I just want to make that an async loop, and call clearStack() to run any pending microtasks.
Simple.

Unfortunately, it requires transforming the code.
I changed the existing loop into an async loop, using a state machine with a switch statement. (This is how babel transpiles async-await, although I did this manually.)

Motivation and Context

Closes #1725

This makes it possible to test code that uses timeouts in combination with Promises or async-await.

How Has This Been Tested?

I'm fairly confident that the existing tests verify the transformed loop with the switch statement.
Thus, I've only added a handful of tests that verify microtasks get run.

  • one for the initial queue pumping.
  • one for the subsequent queue pumping.
  • one to verify that setTimeout and asyncTick interact as expected.

Problems:

  • At the moment the tests use async-await, but once the review is close to the end, I'll manually convert them from async-await to ES5.
  • I have only run this on node.js. I will have to do something different in the browser, and I'm not sure how to handle IE which doesn't even have Promises.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

I had to make the existing loop into an async loop, using a state
machine with a switch statement. (This is how babel transpiles
async-await, although I did this manually.)
Use (the real) setTimeout for emptying the microtask queue.
I haven't figured out where the docs are to draft an update yet
@SLaks
Copy link

SLaks commented May 17, 2020

Is there any update on this?
I need this functionality too.

@sgravrock
Copy link
Member

Thank you for the PR. I'm sorry you didn't get a response much sooner.

This is impressive work, but I'm not planning to merge it. The async loop transformation adds considerable complexity to what was already one of the harder parts of Jasmine to work with. That's not always a deal-breaker, but I have to weigh the maintainability cost against the value of the feature. In this case I think the cost outweighs the benefit.

Clock and DelayedFunctionScheduler are mostly decoupled from the rest of Jasmine, so those of you who want this functionality could probably pull the versions of those classes from this PR into your own codebases. I'm also going to keep this on a branch so that we can refer to it if anything changes in the future that affects the cost/benefit analysis.

@sgravrock sgravrock closed this Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: await jasmine.clock().asyncTick()
3 participants