Changes to debounce() causing test failures with mocked timers

[ilikesleeping]

In v1.12.1, it seems the debounce method has changed to use now(), which holds onto a reference to whatever Date.now was when underscore was loaded the first time.

This has caused problems in our jest tests, where we're using jest.useFakeTimers to mock changes in time to test timeouts and suchlike, because any mocked changes to the date functions won't have any effect.

Looking at the tests added at the same time, it seems this was perhaps intentional? If so, I think it's possibly not a suitable change for a .1 patch release, as it'll break anything that was relying on mocking the current time - which I suspect is a reasonably common use case in tests. (Ok, maybe not that common, but not totally out there...)

Thanks!

[jgonggrijp]

Thanks for reaching out @ilikesleeping.

There is no intentional breaking change, but I'm not ruling out an unintentional one. Although I currently don't see yet how that should have happened.

The changed logic in debounce (#2892) and the changed tests for debounce and throttle (#2913) that you mention belong to separate pull requests with different objectives. In #2892, @kritollm restored an optimization that had been inadvertently removed by a past contributor. While I'm certainly not ruling out that it might break things (the past change in which the optimization was removed was also a breaking change), I don't see how that should work for your particular case; while it didn't use now before the change, it also didn't use Date in any other way before the change, so I see no way in which you could tamper with the time before the change, either.

#2913 just restores some tests that have been in Underscore for a very long time, exactly meant to confirm that you can't tamper with the timeout intervals of throttle and debounce. I made those tests meaningless when I modularized everything (#2849) and @ognjenjevremovic just made them meaningful again. It doesn't change anything about the intended semantics of these functions.

Could you show me your code that has broken because of the update? That might help me diagnose what is causing the issue. Thanks in advance.

[ilikesleeping]

Thanks for the quick and detailed reply.

I've added a simplest possible test case here to show the difference between the two versions. Just npm i && npm test.

The jest test code runs through any timers set with setTimeout and setInterval as well as mocking date/time (it uses sinonjs/fake-timers under the hood, which I suspect replaces those references in the global context) which is why it worked before, and the change to check the current time against specifically the native time function is why it stopped working.

Obviously it's not your job to support this kind of odd use case in other libraries, but I wonder if retaining a reference to the Date.now function at the time the debounce is created, rather than when the library is instantiated, would be a reasonable middle ground, as it is at least more predictable that way?

[jgonggrijp]

@ilikesleeping Thank you for the reduced testcase, it's very informative. I think I now understand how #2892 broke your tests. I did some historical research as well and found more answers. I will not change the logic, for reasons that I'll explain below, but I think there is a different way to fix your tests, which I'll also detail below.

In chronological order, here are some relevant things that happened to debounce in the past:

• add in single timeout implementation of debounce #1269 introduced the optimization. The author did mention at the time that it might interfere with overriding Date.
• Allow _.throttle to function correctly after the system time is updated #1473 added the original time-tampering tests, which relied on _.now. Around this time, people had already observed that Sinon.useFakeTimers didn't work with _.now.
• Do not store a direct reference to Date.now #1478 proposed a way to reconcile Sinon.useFakeTimers with _.now, but was rejected. At this point, Underscore had clearly committed to favoring timing consistency over mockability.
• Cleanup the debounce function #2340 was a misguided attempt at "cleaning up" debounce, which more or less reverted add in single timeout implementation of debounce #1269 and also broke the test introduced in Allow _.throttle to function correctly after the system time is updated #1473. The latter however did not actually fail, because it relied specifically on _.now, which Cleanup the debounce function #2340 removed from the implementation. In this way, Cleanup the debounce function #2340 introduced a bug that went unnoticed for years, and which accidentally enabled your tests to work.
• Chopping Underscore into lots of tiny modules #2849 put debounce and now in separate modules.
• Reimplementing timer optimization #1269 #2892 fixed the bug introduced in Cleanup the debounce function #2340, by (again) more or less reverting it. The test from Allow _.throttle to function correctly after the system time is updated #1473 was however still unable to detect regressions, because of the direct import of now in debounce that arised through Chopping Underscore into lots of tiny modules #2849.
• [Tests]: System time tampering effects on _.throttle and _.debounce methods #2913 finally restored the ability of the time tampering tests to detect regressions.

All in all, Underscore 1.12.1 has finally brought debounce back to the point where it should have been all the time, i.e., fast and reliable in real-world situations. Unfortunately, this means that your tests no longer work as-is. I believe that changing the logic in order to re-enable your tests is a breaking change, against the intent of the library and repeating history; those are three reasons not to do it.

However, if you are doing CommonJS imports in your application and your tests, like in the reduced testcase that you sent, then there is hope. You can override _.debounce (and optionally also _.now and _.throttle) to behave in any way that works for you using mixin. A low-effort way to do this is to import Underscore 1.12.0 alongside current Underscore, like you did in your reduced testcase, and import the old (buggy but mockable) debounce from there:

var _ = require('underscore');
var _1_12_0 = require('underscore-1.12.0');
_.mixin({debounce: _1_12_0.debounce});
// all code invoking _.debounce after this point will use the 1.12.0 version

If you don't want to have two complete copies of Underscore in your test runtime, you can also avoid this by using our modular CommonJS build:

var _ = require('underscore');
var _1_12_0_debounce = require('underscore-1.12.0/cjs/debounce.js');
_.mixin({debounce: _1_12_0_debounce});

I have demonstrated this workaround in the context of your reduced testcase over here: ilikesleeping/underscore-debounce-test#1

[ilikesleeping]

Thanks for the response and history of debounce, very informative. It seems our implementation was perhaps working by chance all along.

Of course prioritising the real-world use cases of course makes sense, so we shall find a way to work around it.

Closing this issue out as there's nothing left to do here.

Thanks again!

[jgonggrijp]

@ilikesleeping Thanks for closing. If I can help finding workarounds (for example for module systems other than CommonJS), please let me know.

[Sacharified]

@jgonggrijp Is there a recommended way to test these then? Importing an older version of the library to use a specific function in tests while using a newer version in production doesn't really seem like a very maintainable approach.

Other time-related functions such as defer work as expected with jest's timers but it's not really clear without digging in to the source why these don't work the same way or how to test them differently.

[jgonggrijp]

@Sacharified thanks for reaching out.

@jgonggrijp Is there a recommended way to test these then? Importing an older version of the library to use a specific function in tests while using a newer version in production doesn't really seem like a very maintainable approach.

Please keep in mind that I'm not suggesting that you swap out the entire library for an older version. I'm only suggesting to replace debounce by a different implementation, which happens to be available from an older version. Patching specific library functions (or your own) in order to control particular aspects of a test is a common technique. In fact, tampering with the system clock in the first place is not really different.

That said, I'd be more than happy to suggest an approach that might suit you better. Please tell me more about your particular use case if you'd like that.

[Sacharified]

@jgonggrijp Thanks for the response

I want to understand regardless of my use case: What's the recommended way to test a function which implements debounce in the current version of this library without having to wait for the real timers?

Please keep in mind that I'm not suggesting that you swap out the entire library for an older version. I'm only suggesting to replace debounce by a different implementation, which happens to be available from an older version. Patching specific library functions (or your own) in order to control particular aspects of a test is a common technique. In fact, tampering with the system clock in the first place is not really different.

Here's my issues with this: I don't want to have to include 2 different versions of Underscore in my project. I don't want to have a different implementation in my tests to my production code and I definitely don't want to ship 2 different versions of the library to the browser.

That said, I'd be more than happy to suggest an approach that might suit you better. Please tell me more about your particular use case if you'd like that.

In my test I have a text input component which automatically calls an API endpoint when typed in to. I have a debounce on the search function so that it doesn't call the API whilst the user is still typing. I want to validate that when my test enters a 2+ character query the API is only called once.

I don't think you can really mock that without basically re-implementing the debounce function. I could just allow the real timers to run but I don't want to slow down my test suite.

[jgonggrijp]

@Sacharified So here's the thing. In your application in production, you're debouncing something, which implies a substantial delay. If you don't want to have that delay in your unittests, you're always going to somehow make your unittests different from the application in production. You could fake the time, change logic, lower the debounce interval, or a combination of such measures. In all cases, whatever approach you take to effectively make your unittest less realistic, you won't be shipping those modifications in production.

I want to understand regardless of my use case: What's the recommended way to test a function which implements debounce in the current version of this library without having to wait for the real timers?

Honestly, I don't think I could recommend a single approach that works for everyone. However, bear with me and keep reading.

Please keep in mind that I'm not suggesting that you swap out the entire library for an older version. I'm only suggesting to replace debounce by a different implementation, which happens to be available from an older version. Patching specific library functions (or your own) in order to control particular aspects of a test is a common technique. In fact, tampering with the system clock in the first place is not really different.

Here's my issues with this: I don't want to have to include 2 different versions of Underscore in my project. I don't want to have a different implementation in my tests to my production code and I definitely don't want to ship 2 different versions of the library to the browser.

Again, there is no need to ship two versions of Underscore in production. Unittests always involve dependencies that aren't present in production. Using a second version of Underscore just for monkeypatching debounce in your unittests is not really different in this regard.

In my test I have a text input component which automatically calls an API endpoint when typed in to. I have a debounce on the search function so that it doesn't call the API whilst the user is still typing. I want to validate that when my test enters a 2+ character query the API is only called once.

I don't think you can really mock that without basically re-implementing the debounce function.

If you really want to completely eliminate any debounce delay, then this is your only option. However, you can save yourself the effort of actually re-implementing debounce by patching just the debounce function from version 1.12.0 only in your unittests, like I described here. Also, there are still alternative ways to speed up your tests without having to mock debounce at all.

I could just allow the real timers to run but I don't want to slow down my test suite.

Maybe you can reach an acceptable compromise by adjusting the debounce interval for your unittests. For example, you could have the key presses arrive at 1 ms intervals and delay the API call until 5 ms pass without typing. Depending on how you set the interval in production, this could already be a hundredfold speedup even though it doesn't completely eliminate the delay.

As a final remark, I would like to mention that there is a fine line between unittesting your own logic and unittesting your own glue code that basically just invokes a library function. In the latter case, you end up indirectly unittesting the library function, which in most cases is wasted effort because the library already has unittests of its own. As you describe your use case above, it seems a bit like you're just unittesting whether debounce works as advertised; consider whether it might be sufficient to unittest whether the debounced event handler by itself does the right thing (i.e., send an API call).

(Of course, you also want to test whether all of your units and your external dependencies integrate correctly, which implies testing the glue code as well. This is however the topic of functional testing, and I wouldn't generally recommend messing with the time in functional tests. Functional testing is very different from unittesting.)

[ilikesleeping]

For what it's worth, I ultimately solved my issue by switching to use this debounce instead, which I believe is the implementation from lodash. That one works just fine with mocked timers. (Sorry @jgonggrijp!)

[jgonggrijp]

@ilikesleeping The Readme of that package states that it was extracted from Underscore, but I wouldn't mind either way. It's your project. If you are using Underscore in production and also that alternative debounce implementation, do realize that your users are downloading two implementations of debounce.