4

I'm writing a Unit Test class in C# (.NET 4.5). In one of the tests I'm checking the values of various properties after an instance of our class FeedbackDao is constructed. On construction, the FeedbackDate property of FeedbackDao is set to DateTime.Now.

FeedbackDao feedbackDao = new FeedbackDao();
// a couple of lines go here then I set up  this test:
Assert.IsTrue(feedbackDao.FeedbackDate.CompareTo(DateTime.Now) < 0);

My assumption is that feedbackDao.FeedbackDate should always be just a little earlier than the current time returned by DateTime.Now, even if it's only by a millisecond, and my IsTrue test should always pass, but sometimes it passes and sometimes it fails. When I add a message like this:

Assert.IsTrue(feedbackDao.FeedbackDate.CompareTo(DateTime.Now) < 0,
                feedbackDao.FeedbackDate.CompareTo(DateTime.Now).ToString());

the message sometimes reads -1 (meaning that FeedbackDate is earlier than Now) and sometimes reads 0 (meaning that the DateTime instances are equal).

Why is FeedbackDate not always earlier than Now? And, if I can't trust that comparison, how can I write a rigorous test to check the value of FeedbackDate when FeedbackDao is constructed?

DavidHyogo
  • 2,838
  • 4
  • 31
  • 48
  • 3
    Why on *earth* don't just check `feedbackDao.FeedbackDate > DateTime.Now` or whatever ? – Marc Gravell Aug 09 '12 at 11:52
  • Why on _earth_ would I ask the question if I could have thought of that myself. That's why Stackoverflow is here, isn't it? – DavidHyogo Aug 09 '12 at 11:55
  • @Paddy Thanks. That answered the question about a rigorous test that satisfies me. I've run it several times and it always passes, and gives me the confidence that the property is properly set. – DavidHyogo Aug 09 '12 at 11:56
  • 1
    Beware, <=0 is *another test entirely* in which you allow the dates to be *equal*. Jon Skeet's answer also points out a major flaw in your test design you should definitely check out. – Alex Aug 09 '12 at 11:58
  • As a sidenote: I'd use `UtcNow` in this context. – CodesInChaos Aug 09 '12 at 12:03
  • 1
    The test with `<` or `<=` should not give you any confidence at all. If no one initialized the field its default value will be `DateTime.MinValue` which always passes the test. See my answer for an alternative while still using `DateTime.Now` to initialize it. – João Angelo Aug 09 '12 at 12:03
  • @JoãoAngelo Thanks João. I hadn't thought of the possibility that the field might have the default value. And, after all, the whole point of my test is to check that the field is correctly initialised. – DavidHyogo Aug 09 '12 at 12:05

7 Answers7

8

My assumption is that feedbackDao.FeebackDate should always be just a little earlier than the current time returned by DateTime.Now, even if it's only by a millisecond.

What makes you think that? That would suggest that 1000 calls would have to take at least 1 second which seems unlikely.

Add to that the fact that DateTime.Now only has a practical granularity of about 10-15ms IIRC, and very often if you call DateTime.Now twice in quick succession you'll get the same value twice.

For the purpose of testability - and clean expression of dependencies - I like to use a "clock" interface (IClock) which is always used to extract the current system time. You can then write a fake implementation to control time however you see fit.

Additionally, this assertion is flawed:

Assert.IsTrue(feedbackDao.FeebackDate.CompareTo(DateTime.Now) < 0,
                feedbackDao.FeebackDate.CompareTo(DateTime.Now).ToString());

It's flawed because it evaluates DateTime.Now twice... so the value that it reports isn't necessarily the same one that it checks. It would be better as:

DateTime now = DateTime.Now;
Assert.IsTrue(feedbackDao.FeebackDate.CompareTo(now) < 0,
                feedbackDao.FeebackDate.CompareTo(now).ToString());

Or even better:

DateTime now = DateTime.Now;
DateTime feedbackDate = feedbackDao.FeebackDate;
Assert.IsTrue(now < feedbackDate,
              feedbackDate + " should be earlier than " + now);
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Sorry, Jon, I didn't mean literally a millisecond. I really didn't know what the practical tick would be. Thanks for tightening up my code fragments, too. This will give me confidence for the next time I have to compare DateTime instances. – DavidHyogo Aug 09 '12 at 12:01
  • Obviously, I gave you an upvote for such a (characteristically) useful answer, but João Angelo's point about checking that the `FeedbackDate` field is later than the default minimum value of `DateTime.Now` gave me a lot of food for thought and I think I've got the test I wanted. That's why I accepted his answer. – DavidHyogo Aug 09 '12 at 12:54
2

Your test is not that useful as it is, you're asserting that the value is less than DateTime.Now but that does not mean it was correctly set to the expected value. If the date time is not initialized it will have the DateTime.MinValue and that value will always pass the test.

This test is as valid as testing for feedbackDao.FeebackDate.CompareTo(DateTime.Now) <= 0 and with that you would not have the problem that motivated you to write this question.

You need to extract the dependency on DateTime.Now or use a mocking framework that supports mocking DateTime.Now and assert that the value is initialized to the correct one. You can check Microsoft Moles, now renamed to Fakes in VS 2012, which is the only mocking framework that I know that is free (kind of for the latest version, since it ships with VS and don't know if it is available on the express editions) and that will let you replace a call to DateTime.Now.

Update:

Without resorting to a mocking framework you could improve your test by doing something like this:

var lowerBoundary = DateTime.Now;
var dao = new FeedbackDao();
var upperBoundary = DateTime.Now;

Assert.IsTrue(dao.Date >= lowerBoundary && dao.Date <= upperBoundary);
João Angelo
  • 56,552
  • 12
  • 145
  • 147
  • There have been so many useful answers and comments, but your point about the possibility that the field was initialised at all, really got me thinking. I'm afraid I don't really understand what a "mocking" framework is and I'm not clear what you mean by "extract the dependency on DateTime". Is there no way of writing my test rigorously without resorting to "Fakes"? Paddy's comment or NPSF3000's answer combined with a test that FeedbackDate is greater than MinValue would be enough, wouldn't it? – DavidHyogo Aug 09 '12 at 12:13
  • I update my answer with an alternative that is slightly better in my opinion. – João Angelo Aug 09 '12 at 12:23
  • Why did you use operators like `>=` instead of `CompareTo`? Are they more reliable? – DavidHyogo Aug 09 '12 at 12:25
  • They are more concise and find them more readable and in this case I just need a boolean result so `CompareTo` is not required. `CompareTo` is more useful in sorting scenarios. – João Angelo Aug 09 '12 at 12:30
  • Well you've got my vote. It passes every time, as it should. I was a bit surprised by `var`. I've been sticking to .NET 2.0 for years and just recently upgraded to .NET 4.5, so I hadn't noticed `var`. – DavidHyogo Aug 09 '12 at 12:37
1

When unit testing, I consider DateTime.Now to be an external dependency, and thus something needing to be mocked. What I've done in the past when testing scenarios involving DateTime.Now, I've just passed a Func<DateTime> in via the constructor of the class, which allows me to mock DateTime.Now during testing.

I prefer this over Jon Skeet's suggestion of using something like an IClock interface to wrap around the DateTime properties, just because the last time I did this, I felt silly making a new interface and class to wrap around a single property. If you're going to need to test around more than one of the static DateTime properties, I definitely agree with the IClock suggestion.

For example,

public class Foo
{
    private readonly Func<DateTime> timeStampProvider;
    public Foo(Func<DateTime> timeStampProvider)
    {
        this.timeStampProvider = timeStampProvider;
    }

    public Foo() : this(() => DateTime.Now)
    {
    }

    public bool CompareDate(DateTime comparisonDate)
    {
        // Get my timestamp
        return comparisonDate > timeStampProvider();
    }
}

Then, during testing,

var testFoo = new Foo(() => new DateTime(1, 1, 2010));
Daniel Mann
  • 57,011
  • 13
  • 100
  • 120
  • Thanks DBM. I'm new to Unit Testing and didn't know what "mocking" meant in this context, and the way you've explained it, I now understand much better what dependencies mean. (I know what "dependency" means - I mean I now understand much better what Jon and João were getting at in this context.) Thanks. – DavidHyogo Aug 09 '12 at 12:40
1

I generally use a mock data to validate my logic. I evolve my test scenarios around the mock data. As suggested by DBM. Mock data is a set of known data that is generally static or configurable. Common practice is to have a XML file with all the test data and load them as and when required. I can give you an example in our Project.

Mrinmoy Das
  • 35
  • 1
  • 7
  • Thanks. This whole concept of mock data is exactly what was missing from the way I was thinking about Unit Tests. This is my first attempt so the answers here have been really useful. – DavidHyogo Aug 10 '12 at 14:21
0

Try

Assert.IsTrue(feedbackDao.FeebackDate.CompareTo(DateTime.Now) < 1);

Or

Assert.IsTrue(feedbackDao.FeebackDate - DateTime.Now < someMarginOfError);

Time is generally fairly granular - often 10's of milliseconds IIRC.

NPSF3000
  • 2,421
  • 15
  • 20
0

Depending on your system, DateTime.Now is not updated every millisecond or tick, it is only updated periodically. Typically 10 ms or so. See here: How frequent is DateTime.Now updated ? or is there a more precise API to get the current time?

Community
  • 1
  • 1
CodingWithSpike
  • 42,906
  • 18
  • 101
  • 138
0

DateTime.Now isn't 100% accurate. It increases by around 130 ms(from personal experience per tick). So it's verry likely that if your method is fast enough the date will be equal to datetime.now and not smaller.
If you want a 100% accurate timer you should use the StopWatch class.
Msdn link to stopwatch

Kristof
  • 3,267
  • 1
  • 20
  • 30