-
Notifications
You must be signed in to change notification settings - Fork 22.9k
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
Disable transitions API to support unit testing #1789
Comments
+1 |
D3 has 2,585 tests at the current count, many of which involve transitions. I chose Vows.js for D3’s testing framework partly because it handles asynchronous tests well; tests are run in parallel so even hundreds of tests that involve short animations complete very quickly. It’s also easy to isolate the state of each test using JSDOM. (Vows no longer appears to be under active development, which is a shame.) I’m not sure how JS Unit handles asynchronous tests, but if it doesn’t handle asynchronous tests well, then that’s a shortcoming of the test framework. Blanket disabling of transitions for testing may not be a good idea; although your tests become simpler, you’re no longer testing your code as it would run in a browser. And the asynchronous nature of transitions can easily be a source of bugs. That said, the nature of unit tests is that you seek to isolate what each test is responsible for testing, so as long as you’re testing transitions some other way, of course it’s reasonable for some unit tests to mock out transitions and ignore their asynchronous nature for simplicity. There are a variety of ways you could mock out transitions, such as overriding d3.selection.prototype (and perhaps d3.transition.prototype) as mentioned in the Stack Overflow question you linked. My first inclination was to mock out the d3.timer API, but that’s not currently possible because of the internal var callbacks = [];
requestAnimationFrame = function(callback) {
callbacks.push(callback);
};
flushAnimationFrames = function() {
var now = Date.now;
Date.now = function() { return Infinity; };
callbacks.forEach(function(callback) { try { callback(); } catch (e) { console.error(e); } });
callbacks = [];
Date.now = now;
}; Example at bl.ocks.org/9644751. |
Asynchronous test execution makes it a lot harder to debug test failures, as it is not easy to step through the code as a synchronous execution is. However, I don't want to debate test framework architectures with you. Most of d3 is usable without asynchronous execution, and my intention is to make the remaining parts usable within a synchronous test as well (where possible). The point you make about a unit test limiting itself is actually what I tried to achieve: As a D3 user, I trust the API to perform a transition properly and usually want to make sure my first and last state are in order. So mocking out transitions seems a sensible path for a piece of code using D3 (obviously, testing transitions within D3 is a completely different story). Considering the impact of mocking requestAnimationFrame and Date.now, I'd still like to go with a mock of D3's transitions that short circuit to the final state immediately. The stackoverflows only work for limited purposes, as they do not deal with subsequent transition() calls such as ease, duration or attrTween properly. |
I was envisioning transition bugs in “user land” (not D3), such as when one transition unexpectedly interrupts another. I’ll probably go ahead with #1790 for 3.5, in which case mocking d3.timer might look something like this: var timers = [];
d3.timer = function(timer) {
timers.push(timer);
};
function flushTimers() {
timers.forEach(function(timer) { timer(Infinity); });
timers = [];
} You’re welcome to mock out the full transitions API, but my guess is that will be more challenging than simply changing the low-level timer callbacks to make them temporarily synchronous (either by mocking d3.timer or requestAnimationFrame). Note that in the mock requestAnimationFrame example, Date.now is only overridden temporarily while the timers are flushed. |
Agreed, keeping transitions and simply making them execute synchronously is the way to go. Bugs in relation to interaction between transitions is something we haven't been experiencing yet, so I don't know how to test for. Maybe we'll need it when we get there. For now, we're doing this to finish up all transitions before accessing the SVG for assertions. var flushAllD3Transitions = function() {
var now = Date.now;
Date.now = function() { return Infinity; };
d3.timer.flush();
Date.now = now;
} |
@mbostock, could you suggest how to skip the animation in d3 v4? None of the solutions above seem to work for me. |
Same observation… winding time forward to Infinity no longer works to flush D3 transitions in version 4. I'm using asynchronous Mocha tests and using a setTimeout to wait for it to complete, but that does have the potential to slow down our testing environment. |
@skhilko @cvkline Version 4 moved to performance.now, so you need to use |
@larsenmtl @skhilko @cvkline I tried that and was running into problems with clockTick(milliseconds) {
const currentNow = performance.now();
performance.now = () => currentNow + milliseconds;
// The new animation frame means d3's timers will check performance.now() again
return new Promise(resolve => requestAnimationFrame(resolve));
} |
That's a good solution; we make heavy use of promises in our Mocha tests as well and there's a lot of scaffolding to support that. Thanks! |
Transitions with their unpredictable timing make it hard for us to test changes in an SVG with JS unit tests.
We've tried mocking the wall clock and calling d3.timer.flush(), but for some reason not even that helps (unless you flush the timer a few ten thousand times).
Looking at https://stackoverflow.com/questions/20068497/d3-transition-in-unit-testing and https://stackoverflow.com/questions/14443724/disabling-all-d3-animations-for-testing I suppose I'm not the only one who has this problem, so I propose:
There should be a way to globally disable all transitions and animations and move to the final state immediately, similar to http://api.jquery.com/jQuery.fx.off/
The text was updated successfully, but these errors were encountered: