sinonjs / sinon

Test spies, stubs and mocks for JavaScript.
https://sinonjs.org/
Other
9.66k stars 771 forks source link

Install fake timer before calling setup() #455

Closed dharkness closed 10 years ago

dharkness commented 10 years ago

When using automatic fake timers, accessing the current time in setup() returns the real time. This breaks tests that depend on starting at 0.

mantoni commented 10 years ago

Could you be a little more specific?

dharkness commented 10 years ago

Yes, sorry. Got pulled away to lunch before I could post a test case.

function Ttl(ms) {
    this.expireAt = Date.now() + ms;
}

Ttl.prototype.isExpired = function () {
    return Date.now() >= this.expireAt;
};

module("simple TTL expiration", {
    setup: function () {
        this.ttl = new Ttl(10);
    }
});

test("starts not expired", function () {
    assertThat(this.ttl.isExpired(), is(false));
});

test("expires after configured TTL", function () {
    this.clock.tick(10);
    assertThat(this.ttl.isExpired(), is(true));
});

Running produces a failure:

>> simple TTL expiration - expires after configured TTL
>> Message: Expected is equal to true but was false

Copying the setup code to the beginning of each test fixes the problem but is obviously not a good work-around. Sinon should install the fake timers before calling setup.

mantoni commented 10 years ago

Still can't follow. I don't know where module and test come from and why Sinon should call setup.

dharkness commented 10 years ago

Those functions come from QUnit, and Sinon-QUnit provides a wrapper--but only for test. I see the problem now. That integration package (also by @cjohansen) wraps the test method (the function passed to test) with a Sinon version that sets up and tears down the sandbox, including fake timers.

(function () {
    var qTest = QUnit.test;
    QUnit.test = test = function (testName, expected, callback, async) {
        ...
        return qTest(testName, expected, sinon.test(callback), async);
    };
}());

But QUnit's module allows you to specify setup code to run before each test method, and I need Sinon to setup the sandbox before that happens. Fixing this will take quite a bit more work than I thought and will require work in both Sinon and the Sinon-QUnit integration package.

For the few tests that need the fake timer in setup I can perhaps temporarily suspend the automatic fake timers, manually install one, and then restore it in teardown to clean up. In any case, this issue probably belongs to the integration package so I'll close it for now. Thanks!

dharkness commented 10 years ago

Disabling automatic fake timers during setup works, but it got me thinking. My first attempted workaround was to call sinon.useFakeTimers() from setup, but this broke because the automatic fake timer overrides it.

What if automatic fake timers were skipped if already installed? Still have them restored automatically, but allow setup code to install them manually before the sandbox is created. Would that work without breaking other code?