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

Code that uses DateTime.now is untestable #28985

Open
Hixie opened this issue Mar 8, 2017 · 17 comments
Open

Code that uses DateTime.now is untestable #28985

Hixie opened this issue Mar 8, 2017 · 17 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-n library-async library-core type-enhancement A request for a change that isn't a bug

Comments

@Hixie
Copy link
Contributor

Hixie commented Mar 8, 2017

For testing purposes it would be useful if there was a way to drive the dart:core clock manually, so that DateTime.now returned predictable times. One solution would be to provide a hook in the Zone to drive it; this would let FakeAsync drive it in a manner consistent with the rest of the FakeAsync clocks.

Currently there is code in Flutter's framework that is untested and basically untestable because it depends on the value returned from DateTime.now (for example, the code that highlights "today" in the date picker).

@lrhn lrhn added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-async library-core type-enhancement A request for a change that isn't a bug labels Mar 8, 2017
@lrhn
Copy link
Member

lrhn commented Mar 8, 2017

To be precise, DateTime.now is perfectly testable, but code that depends on it is hard to do coverage testing for since you can't control the values going into that code. Changing the behavior of DateTime.now means that you would no longer be testing DateTime.now.

Making every system integration point user configurable (using zones or similar in-program methods) isn't really viable. It's a never ending story, especially when the dart:io library is considered. The logical conclusion would be to make every static method or constructor mockable - and I'm sure some people would like that, but I think it's already been done too much.
I'm also concerned about adding overhead to DateTime.now - it's not really a method that suffers latency well. And changing DateTime.now probably also means changing the behavior of every clock in the system (or risk being inconsistent with StopWatch and Timer, even though those might use different system calls to do their ticking). All in all, it's adding overhead to the most timing critical part of the system, just for testing.

I recommend writing the code that actually uses DateTime.now to abstract over the now function, then you can instantiate it with either DateTime.now or, for testing, your own function, instead of making it a static hard-coded dependency on the time provider. If it's not your code, that's obviously harder to change.
For example, have the actual code in a helper library:

public library:
  abstract class MyTimeBasedClass {
    factory MyTimeBaseClass() : GeneralTimeBaseClass(DateTime.now);
    // interface...
  }
private library:
  class GeneralTimeBaseClass implements MyTimeBasedClass {
    DateTime Function() _now;
    GeneralTimeBaseClass(DateTime this._now());
    // impl
  }

Then your test can import the private library (from src/ somewhere) and test it using a custom DateTime-returning function of your choice.
Or have a static field where you can store the function, or check a Zone variable for an override. Again, that only affects your code.

That is, write your code for testing (reduce static dependencies) instead of requiring the entire system to be configurable for something you only do during testing anyway.

@floitschG
Copy link
Contributor

Alternatively, we can investigate making mocking of static functionality easier in a test-setup.

For example, I find it more scalable if it was possible another isolate that uses the debug-protocol to intercept a constructor-call to new DateTime.now() and returns the mocked object. I'm not sure if this is already fully possible, but this approach would make it possible to intercept every static call.

Obviously this approach would have its limitations:

  • it wouldn't work with dart2js.
  • a full AOT compilation with tree-shaking might not have enough code anymore to create a mock object.

@persimmons
Copy link

You can use the Clock class from quiver.time instead of DateTime to get a fakeable Clock: https://github.com/google/quiver-dart#quivertime

@floitschG floitschG added core-n and removed core-m labels Aug 21, 2017
@Hixie Hixie changed the title DateTime.now is untestable Code that uses DateTime.now is untestable Oct 6, 2017
@adamsmaka
Copy link

adamsmaka commented May 15, 2020

create extension like this

  static DateTime customTime;
  static DateTime get current {
    if (customTime != null)
      return customTime;
    else
      return DateTime.now();
  }
}

use getter in production code.
in tests set expected value like this:
CustomizableDateTime.customTime = DateTime.parse('2012-02-19 13:27:00');

@roughike
Copy link

roughike commented Nov 14, 2020

One option is to do it with package:clock, which is maintained by the Dart team:

import 'package:clock/clock.dart';

void main() {
  // prints current date and time
  print(clock.now());
}
import 'package:clock/clock.dart';

void main() {
  withClock(
    Clock.fixed(DateTime(2000)),
    () {
      // always prints 2000-01-01 00:00:00.
      print(clock.now());
    },
  );
}

(I also wrote about it on my blog)

@lukepighetti
Copy link

lukepighetti commented Dec 19, 2020

The problem extends to TimeOfDay.now(). It would be ideal if we could set the clock millis upstream of DateTime.now() as there are many things that depend on it.

@lukepighetti
Copy link

lukepighetti commented Dec 19, 2020

Here's a mixin that ChangeNotifier users might appreciate.

import 'package:flutter/foundation.dart';

/// A getter for [DateTime.now] that can be stubbed for testing.
mixin StubbableNow on ChangeNotifier {
  /// The current date.
  DateTime get now => _stubNow ?? DateTime.now();
  DateTime _stubNow;

  /// Set a stub value for [now], used for testing.
  set stubNow(DateTime date) {
    _stubNow = date;
    notifyListeners();
  }
}

Example test

void main() {
  group('HoursStore', () {
    test('Correctly determines if it is legal hours', () {
      final hours = HoursStore(store);

      final saturdayDay = DateTime(2020, 9, 5, 10, 10);
      final saturdayNight = DateTime(2020, 9, 5, 20, 10);

      /// Saturday day
      hours.stubNow = saturdayDay;
      expect(hours.isLegalHuntingHours, isTrue);

      /// Saturday night
      hours.stubNow = saturdayNight;
      expect(hours.isLegalHuntingHours, isFalse);
    });
  });
}

@neiljaywarner
Copy link

@Hixie any final comments?

@Hixie
Copy link
Contributor Author

Hixie commented Dec 11, 2021

Final comments?

@neiljaywarner
Copy link

Yes. Like should we close this issue.
What do you think of the two different ideas etc...

@Hixie
Copy link
Contributor Author

Hixie commented Dec 11, 2021

As far as I can tell the initial report is still valid.

If someone writes a library that uses DateTime.now, they have to explicitly wrap the API for their code to be testable. This is in contrast, to, say, dart:io's HttpClient API, where we provide a way to test libraries that use such APIs. Has anything changed in this regard?

@neiljaywarner
Copy link

I apologize. If this is fixable someday like your initial description mentioned - great!

@lukepighetti
Copy link

I agree that this issue is not resolved, and that it probably should be at some point, even though there are alternatives available.

@idreesVenD
Copy link

Any update on this? 👀

@matthew-carroll
Copy link

Here's a use-case for this ticket.

I implemented custom velocity tracking for pan and scale gestures. Velocity calculations depend upon changes in time. Flutter's gesture system doesn't include time information in the various GestureDetector callbacks. So I need to track time myself.

I can use a Ticker without issue to track the time during a gesture.

However, I also need to track the time between gestures. Flutter reports a new scale gesture every time the user adds or removes a finger. I need to de-dup these gestures to avoid crazy velocities in the time it takes a user to remove two fingers from the screen.

Using a Ticker to track the time between gestures isn't great. Either, I keep a Ticker running (possibly) forever, or I need to add behavior that understands when and why to stop a Ticker, specifically in the case of measuring time after a gesture completes.

If DateTime could be controlled by Flutter's test pipeline, all of this Ticker behavior could be replaced by a handful of DateTime comparisons.

@lrhn
Copy link
Member

lrhn commented Nov 21, 2022

@matthew-carroll
I'd recommend using Stopwatch to count time between two events, not computing DateTime differences. It's the canonical way to count time in plain Dart. There is no need to stop the stopwatch when you're done using it.
(It won't help with testing, the Stopwatch class does not have an override either.)

@idreesVenD
Copy link

Any update on this? 👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-n library-async library-core type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests