Promises

[benjamingr]

Hey,

At the moment promises won't get called in tests using lolex. I think it would be cool if promises ran under lolex too.

Basically:

• Override Promise.prototype.then (which always defers running callbacks to the next tick).
• If the clock progresses (in any way) - run pending promises as well.
• On uninstall - run all promise callbacks remaining in nextTick and resume standard operation.

Since promises resolve synchronously (but only are acted on asynchronously) that should work.

We also need to patch Promise.resolve since that is called implicitly whenever a function is awaited in an async function.

Do you think this is in scope for lolex? If it is - can start working on it. If not - where would this belong?

[fatso83]

This sounds a bit hairy ... and possibly a bit out of scope. Prototype modification of global types always sends chills down my spine due to potential side effects not expected by the clients. You also have no guarantee that promises are made using global.Promise. How would you handle promises made using pinkySwear (as an example of a promise lib)?

[benjamingr]

Prototype modification of global types always sends chills down my spine due to potential side effects not expected by the clients.

I agree, there is an explicit hook here though (Promise.prototype.then) that is an additional "timer entry point".

How would you handle promises made using pinkySwear (as an example of a promise lib)?

The same way you'd handle timers that come from an external package and no the platform - you don't really. You can instrument their scheduling mechanism and they can opt in.

For example - it's fairly easy to get bluebird to work with lolex by calling setScheduler with a timer parameter. In fact - I'd guess most smaller/slower libs probably schedule with setTimeout and would likely work.

[fatso83]

@mroderick, any thoughts on this or #105?

[mroderick]

lolex is already a bit on the crazy side of things, with replacing setTimeout and friends, we might as well commit to covering more async things. I agree that it's not entirely overlapping with the scope of the rest of the methods here, but I think lolex is a good place for this functionality to start out.

The proposal makes sense to me. We might see issues, that are difficult to anticipate before getting into the details, which may make it difficult to craft a good solution.

It would probably be a good idea to outline how the solution would be used in this issue, before construction starts.

[rogierschouten]

There is a problem with ES2017 async/await and replacing the global Promise object.

With ES2016, you can replace the global Promise object and then you can run fake timer tests just fine. However, in ES2017, if someone uses async, then the promise returned from the async function will NOT be the mocked Promise.

I found a solution, but it basically requires a new lolex module:
Fake the macroqueue items (Interval, Timeout) but don't fake the microqueue ones (nextTick, Promise)

To do this, lolex' clock.tick() function needs to become async, and in your tests you await on it.
What it does, is process the intervals and timeouts as usual (in fake time), but between every timer call, allow Node.JS to process the microqueue items unfaked, by awaiting Promise.resolve().

This ensures a close simulation of real-world behaviour and still makes it possible to fake the time.

Here is the modified clock.tick() implementation:

    // do an actual un-faked NodeJS cycle
    async function nodeCycle(clock) {
         return new Promise((resolve) => clock._setTimeout(resolve, 0));
   }

    clock.tick = async function tick(ms) {
        ms = typeof ms === "number" ? ms : parseTime(ms);
        var tickFrom = clock.now;
        var tickTo = clock.now + ms;
        var previous = clock.now;
        var timer, firstException, oldNow;

        clock.duringTick = true;

        // perform process.nextTick()s
        oldNow = clock.now;
        runJobs(clock);
        await nodeCycle();
        if (oldNow !== clock.now) {
            // compensate for any setSystemTime() call during process.nextTick() callback
            tickFrom += clock.now - oldNow;
            tickTo += clock.now - oldNow;
        }

        // perform each timer in the requested range
        timer = firstTimerInRange(clock, tickFrom, tickTo);
        while (timer && tickFrom <= tickTo) {
            if (clock.timers[timer.id]) {
                updateHrTime(timer.callAt);
                tickFrom = timer.callAt;
                clock.now = timer.callAt;
                oldNow = clock.now;
                try {
                    runJobs(clock);
                    await nodeCycle();
                    callTimer(clock, timer);
                } catch (e) {
                    firstException = firstException || e;
                }

                // compensate for any setSystemTime() call during timer callback
                if (oldNow !== clock.now) {
                    tickFrom += clock.now - oldNow;
                    tickTo += clock.now - oldNow;
                    previous += clock.now - oldNow;
                }
            }

            timer = firstTimerInRange(clock, previous, tickTo);
            previous = tickFrom;
        }

        // perform process.nextTick()s again
        oldNow = clock.now;
        runJobs(clock);
        await nodeCycle();
        if (oldNow !== clock.now) {
            // compensate for any setSystemTime() call during process.nextTick() callback
            tickFrom += clock.now - oldNow;
            tickTo += clock.now - oldNow;
        }
        clock.duringTick = false;

        // corner case: during runJobs, new timers were scheduled which could be in the range [clock.now, tickTo]
        timer = firstTimerInRange(clock, tickFrom, tickTo);
        if (timer) {
            try {
                await clock.tick(tickTo - clock.now); // do it all again - for the remainder of the requested range
            } catch (e) {
                firstException = firstException || e;
            }
        } else {
            // no timers remaining in the requested range: move the clock all the way to the end
            updateHrTime(tickTo);
            clock.now = tickTo;
        }
        if (firstException) {
            throw firstException;
        }
        return clock.now;
    };

And here is an example of a sinon test with fake time (in TypeScript)

		it("should resolve in 10 milliseconds", async (): Promise<void> => {
			const p: Promise<void> = myAsyncFunctionThatShouldReturnAfter10Ms();
			let thenSeen = false;
			let errorSeen = false;
			p.then(() => thenSeen = true, () => errorSeen = true);
			await clock.tick(1);
			expect(thenSeen).to.equal(false);
			expect(errorSeen).to.equal(false);
			await clock.tick(10);
			expect(thenSeen).to.equal(false);
			expect(errorSeen).to.equal(false);
		});

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[rogierschouten]

Keepalive

[benjamingr]

I'm going to bring this up as a use case when discussing promise hooks and async/await at the Node summit at the end of the month with the V8 team.

Posting this as a reminder for myself to post an update.

[bertho-zero]

My workaround fix, i ignore mocking of setImmediate:

/* lock time */
const now = new Date( _.round( new Date(), -3 ) );
let clock;

beforeEach( () => {
  clock = sinon.useFakeTimers( {
    now,
    toFake: Object.keys( sinon.timers ).filter( v => ![ 'nextTick', 'setImmediate' ].includes( v ) )
  } );
} );

afterEach( () => {
  clock.restore();
} );
/*************/

[benjamingr]

Use case https://github.com/nodejs/promise-use-cases/blob/master/use-cases/testing/testing-1/testing-1.md for discussion. Would appreciate feedback and criticism either there or in private if you prefer.

[benjamingr]

People still asking about it https://stackoverflow.com/questions/50783013/how-to-timeout-promises-in-jest cc @SimenB

[benjamingr]

I'll make a PoC

[benjamingr]

If anyone wants to play with this, can run with --allow-natives-syntax:

diff --git a/src/lolex-src.js b/src/lolex-src.js
index 5398254..b825a20 100644
--- a/src/lolex-src.js
+++ b/src/lolex-src.js
@@ -33,6 +33,15 @@ function withGlobal(_global) {
     var nextTickPresent = (_global.process && typeof _global.process.nextTick === "function");
     var performancePresent = (_global.performance && typeof _global.performance.now === "function");
     var performanceConstructorExists = (_global.Performance && typeof _global.Performance === "function");
+    var platformRunMicrotasks = (function hasV8Natives() {
+        try {
+            // this is in an eval so that eslint won't die on a syntax error
+            return eval("%RunMicrotasks"); // eslint-disable-line - this is a V8 thing
+        } catch (e) {
+            return NOOP; // noop, no access to platform microtasks
+        }
+    })();
+
     var requestAnimationFramePresent = (
         _global.requestAnimationFrame && typeof _global.requestAnimationFrame === "function"
     );
@@ -207,6 +216,12 @@ function withGlobal(_global) {
             }
         }
         clock.jobs = [];
+        // run actual "platform" microtasks like async functions and promises: note that this can in turn
+        // enqueue more lolex-side microtasks so we have to check the status afterwards
+        platformRunMicrotasks();
+        if (clock.jobs.length > 0) {
+            runJobs(clock);// TODO loopLimit once this is no longer experimental
+        }
     }

[rakeshramakrishnan]

This would be a good feature to have.

When we are testing intermediate events being generated, which are based on time.now, and we tick the clock by a large amount, we generate a lot of events.

But, most importantly, this event generation process is blocking the event thread, and the large amount of messages collected increases the memory, without allowing garbage collection to collect the intermediate events.

If each intermediate tick was scheduled in the micro-task queue, or even as part of the next tick, garbage collection can clear these events.

[benjamingr]

I want to bring this up to the Node promises team in the next meeting once everyone is back on schedule.

[SimenB]

Sorta related, there's a PR in jest adding support for
faking promises (which'll try to transpile async-await): jestjs/jest#6876

Would love proper support for this in node, feels super brittle!

[benjamingr]

@SimenB would that work with async/await?
Also, wouldn't a much simpler approach be to use a promise library (like bluebird) that enables configurable ticking/scheduling (with setScheduler?).

That looks like a full blown promise implementation and is guaranteed to miss things (for example, if we go forward with GC based exiting on unhandled promise rejection - it breaks that).

It would be great to get feedback from Jest on exact use cases and requirements to support (but totally understand if this is time the project does not want to spend)

[Berzeg]

@SimenB @benjamingr

The idea behind fake promises is to run all promise callbacks synchronously. The goal was never to rewrite promises. When using fake timers this allows us to run nextTick callbacks, promise callbacks, and timer callbacks in the same order they would run in the Node.js environment.

As far as I can tell, there is no way to run promise callbacks synchronously out of the box with bluebird. That would make fake promises much simpler!

The fake promises PR introduces a jest option to compile async/await syntax to generators with explicit promises. Using fake promises without that option will produce a warning in the console.

[benjamingr]

@Berzeg

As far as I can tell, there is no way to run promise callbacks synchronously out of the box with bluebird. That would make fake promises much simpler!

Promise.setScheduler(fn => fn()) does exactly that. You can also do something better and push the function somewhere and call it whenever you want promises to resolve (when microticks run for instance) If that doesn't work for you let me know and we'll fix it in bluebird.

I want to be clear on something: I really don't want to take the wind out of your sails here - rather I think it's an interesting take and I think it makes sense to write down all the cases it doesn't work for and try to figure out what we can do to address those.

[Berzeg]

Promise.setScheduler(fn => fn())

Yip. That would do it.

I don't want to take the wind out of your sails

Understood! I'm not taking any of this personally 😄. The simpler solution should be used, and, well... I think this is perfect.

[PFight]

If someone looking for solution right now, here you go:

function waitTimeout(ms: number): Promise<void> {
    return new Promise((resolve) => {
        setTimeout(resolve, ms);
    });
}

export async function waitTicks(ticksCount: number = 10) {
    for (let i = 0; i < ticksCount; i++) {
        await waitTimeout(0);
    }
}

Use like this:

it('should stuck', async () => {
        let stuckPromise = doSomeStuckWork();

        let resolve = sinon.fake();
        stuckPromise.then(resolve);
        await waitTicks();

        expect(resolve.called).to.be.false;
});

[septagram]

In my case, this issue caused my promises to be invoked in the wrong order. I have encountered this issue both in AngluarJS testing with Karma + Mocha + Sinon and NodeJS testing with Jest (which is using Sinon's FakeTimers under the hood). The only semi-reliable (albeit ugly) workaround I found so far is:

async function tick (ms) {
	for (let i = 0; i < ms; i ++) {
		jest.advanceTimersByTime (1)
		// or sandbox.tick (1)
		// or sinon.tick (1)
		await Promise.resolve()
	}
}

...essentially advancing time by 1ms per turn. And then in your tests (TypeScript):

async function delay (ms: number) {
	return new Promise (resolve => setTimeout (resolve, ms))
}

async function outputOneTwoAndFour (output: string[]) {
	await delay (100)
	output.push ('one')
	await delay (100)
	output.push ('two')
	await delay (200)
	output.push ('four')
}

async function outputThree (output: string[]) {
	await delay (300)
	output.push ('three')
}

test ('with workaround -- passes', async () => {
	const output: string[] = []
	outputOneTwoAndFour (output)
	outputThree (output)
	await tick (500)
	expect (output).toEqual (['one', 'two', 'three', 'four'])
})

test ('without workaround -- fails', async () => {
	const output: string[] = []
	const finished = Promise.all ([
		outputOneTwoAndFour (output),
		outputThree (output),
	])
	jest.runAllTimers()
	await finished
	expect (output).toEqual (['one', 'two', 'three', 'four'])
	// This test times out because of the promises
})

Source: https://stackoverflow.com/a/52196951/521032

Unfortunately the caveat to this is that long promise chains may unwind over several virtual milliseconds, so this solution is imprecise and can backfire if your promise chains are long and delays are short (would be hell to debug that).

I've also tried to use jest.advanceTimersToNextTimer() and then use the real timers API to flush the microtasks queue (which would have its own caveats), but couldn't get it working for now -- my tests time out. Could be a mistake on my part, might take another look into it later.

I'm currently puzzled as to how do FakeTimers.runAllAsync(), FakeTimers.tickAsync() and sinon.tickAsync() come into play. Are they supposed to completely flush microtask queue? Would appreciate if someone would clarify. Jest doesn't expose async FakeTimers functions AFAIK.

[septagram]

Unfortunately the caveat to this is that long promise chains may unwind over several virtual milliseconds, so this solution is imprecise and can backfire if your promise chains are long and delays are short (would be hell to debug that).

I've managed to solve this problem by using setTimeout (..., 0) / setImmediate(), or the Flush Promises library, which does just that. Turns out that timeouts won't execute until the microtask queue is finished. So the solution is pretty easy in the end:

import flushPromises from 'flush-promises'

async function tick (ms: number) {
	for (let i = 0; i < ms; i ++) {
		jest.advanceTimersByTime (1)
		// or clock.tick (1)
		// or sandbox.tick (1)
		// or sinon.tick (1)
		await flushPromises()
	}
}

Drawbacks:

1. This is pretty ugly (wish I could come up with something better).
2. It will be off by one millisecond if you perform setTimeout (..., 0) / setImmediate() in your code under tests.

Still, it's the best I've go so far and it works like a charm in Jest. I'll be testing it in the Karma+Sinon+AngularJS setup at some point as well.

[benjamingr]

flushPromises just does setTimeout(0) @septagram :D

You can "cheat" by calling process._tickCallback or using the chrome devtools protocol (with require('inspector') to run stuff using the debugger)

We can do much much better (call but unfortunately I don't have time since I started a new job recently and am expecting a baby :)

[fatso83]

So ... just poking around a bit: what is the next step here? I see Benji talking "someting much better", but I am not sure this relates to implementing something in the Node core or elsewhere. If it is something we can do in this project, it would be cool if the ideas were fleshed out a bit, so someone could have a go.

[benjamingr]

I think this can be closed since async tick methods cover this (waiting for microtasks) in practice and it's been very hard to do consistently at the platform level.

[fatso83]

What is lacking (as always) is some good tutorials/docs on how to do this in practice. We could stuff it in the How-To section of the main Sinon homepage. https://sinonjs.org/how-to/