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

Fake timers not used in tests #349

Open
andreaspalsson opened this issue Aug 19, 2024 · 0 comments · May be fixed by #351
Open

Fake timers not used in tests #349

andreaspalsson opened this issue Aug 19, 2024 · 0 comments · May be fixed by #351

Comments

@andreaspalsson
Copy link

andreaspalsson commented Aug 19, 2024

We are using @sinonjs/fake-timers and have gotten an issue where items with a ttl doesn’t expire as we progress the time. It seems like lru-cache will not use the fake timers unless they are installed before the first require/import of lru-cache. Since it can be quite hard to control the order of how code is loaded/imported in larger projects this can create some subtle bugs within tests.

Digging into the code I assume the issue stems from lru-cache saving a reference to performance or Date when you load the module the first time and this will reference the real functions if you haven’t installed the fakers yet.

This issue might just be related to our setup where we install and uninstall the fake timers depending on if the test needs them and keeping the servers alive between tests to save time.

Reproduction repository: https://github.com/Milkywire/node-lru-cache-test-issue

A possible solution could be an function that allows you to update or refresh the internal reference something like #refreshTTLTimerRefrence that just reruns the code to find the correct timer.

#refreshTTLTimerReference() {
    	perf = typeof performance === 'object' &&
    	performance &&
    	typeof performance.now === 'function'
    	? performance
    	: Date;
}

This could then be part of unsafeExposeInternals and used if needed.

What do you think?

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 a pull request may close this issue.

1 participant