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 don't work with vue (sinon 7.5.0, @vue/cli 3.12.0) #2155

Closed
AyanamiAkaha opened this issue Nov 7, 2019 · 1 comment
Closed

Comments

@AyanamiAkaha
Copy link

Describe the bug
sinon.useFakeTimers does not affect new Date() when used with vue-cli webpack setup.

To Reproduce

first commit is clean vue cli setup, second is my minimal reproducer.

Expected behavior
Test passes - no matter how much real time passes, new Date().getTime() always returns the same value

Screenshots
n/a
Context (please complete the following information):

Additional context
vue cli version itself is outdated in this setup, but I also briefly tried with newest vue cli, with the same results.
Depending on the machine it might be needed to tweak number of iterations in my reproducer.

@fatso83
Copy link
Contributor

fatso83 commented Nov 7, 2019

Of course, Sinon works fine. It's your setup that doesn't. Now I took 20 minutes to dig into this for you, as I assume there are plenty of people with some combination of JSDOM and Webpack issues that will stumble upon this.

Your setup has a lot of "magic", using a lot of implicit dependencies. The first thing I was thinking about was that you probably had some kind of DOM-thing going on that would shadow the original Date function in some way or mess up what environment Sinon thinks it's running in. I was right and I found out like this, by just printing out some values from your test. I replaced the contents of your test with the following:

    console.log(typeof global);
    console.log(typeof window);
    console.log("window = global", window === global);
    console.log("window.Date = global.Date", window.Date === global.Date);

    const orgWinDate = window.Date;
    const orgGlobalDate = global.Date;

    const clock = sinon.useFakeTimers();

    console.log("window");
    console.dir(window);
    console.log("global");
    console.dir(global);

    console.log("orgWinDate replaced?", orgWinDate !== window.Date);
    console.log("orgGlobalDate replaced?", orgGlobalDate !== global.Date);

The important takeaway are the last two lines, which prints

orgWinDate replaced? true
orgGlobalDate replaced? false

OK, so I know there are two Date objects, one for window and one for global. Originally they point to the same object. This is what your implicit dependency jsdom-global sets up for you.

We also know that since there is a window property Sinon probably thinks it's in a browser environment and will try to target window as the global object to replace properties on, including the Date object and various timers. Since just using Date in a Node environment (which your test is running in) is equal to global.Date, replacing the Date property of window, of course won't do anything to make your test pass.

So the key take away here is that you want to target a different context. This is slightly more exotic than the usual test case, but you already accepted voodoism when mixing Babel, Webpack and JSDOM, so why not add some extra spice 🔥

Lolex, which is the library Sinon is using, have had support for this for ages, but it's not mentioned in the Sinon docs other than "Please visit the lolex.install documentation for the full feature set.".

If you did visit the lolex.install docs you would see that you can "hijack timers in another context" by passing the target property. Let's try that for our Sinon testcase:

const clock = sinon.useFakeTimers({ target: global });

Eh, voila! The test passed.

Actually, while researching this, I found that @LouisBrunner actually added some extra code in #1935 to address an issue (#1852) which is a near duplicate. The problem is that it actually adds code to achieve something that was already possible. It's know possible to use both global and target to achieve the same thing ... not sure that's a good thing. Better to just document the original properly, no? 😄

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

No branches or pull requests

2 participants