Use the current time for NewFakeClock's initial value

[DPJacques]

This prevents callers from building tests that rely on implementation details. Typically by expecting a time or string value computed from NewFakeClock().Now().

This philosophy of preventing users from relying on implementation internals is similar to Go's random map iteration behavior. For users who depend on the initial clock time, NewFakeClockAt() provides the behavior they need, and makes their expectations clear in tests.

This also ensures that future changes that might need to modify the default time for whatever reason won't cause mass breakage, as this author learned about the hard way.

[sagikazarmark]

This also ensures that future changes that might need to modify the default time for whatever reason won't cause mass breakage

Well, this change will. :)

Frankly, I agree with your reasoning, but I'm also worried about the potential mass new issue opening that it may cause.

One thing I would certainly do is expand in the function documentation a bit, telling people that they shouldn't rely on the time being deterministic when using NewFakeClock in their tests. I know it's somewhat redundant with mentioning randomness, but I feel like people need to be explicitly warned.

[DPJacques]

No changes made yet, pending discussion.

I'm also worried about the potential mass new issue opening that it may cause.

I would be too, until I realized the cat is already out of the bag on that one with #52.

I made this CL because I changed the default in that update, thinking .oO(Surely people wouldn't rely on the default time from NewFakeClock. That would be poor form, and NewFakeClockAt is like right there.).

Well, Hyrum's law had something to say about that. There were people in our code base depending on it. However, I went through the process of upgrading our code base to use NewFakeClockAt where people needed a specific time. I viewed it as a reasonable code clean up, although maybe it was premature.

All that is to say, which direction would you like to go with this?

1. We can close this PR and add a warning to existing code that says "please don't rely on the default value", but I think Hyrum's Law would apply anyway :(
2. Since merging #52, we haven't seen any issues/bugs on this yet. Although breakage is a concern, it hasn't been brought up yet. I admit I secretly hope it encourages people to migrate to NewFakeClockAt :)
3. We could revert the default change from #52. Theoretically there could be breakage from reverting the default as well, although reasonably less than keeping it I suppose. I don't know how one reasons about this in open source.

I'm open to whatever you decide, just let me know.

[sagikazarmark]

As you said, the cat is already out of the bag, so let's at least try to help people by adding a more explicit warning to NewFakeClock's documentation.

Other than that (and a rebase), LGTM.

[DPJacques]

Done. Let me know if that works. Happy to modify it as needed.

[DPJacques]

After we get this merged, would it be possible to upgrade the version from 0.3.0 to 0.4.0?

[sagikazarmark]

After we get this merged, would it be possible to upgrade the version from 0.3.0 to 0.4.0?

Sure, no problem.

I can see that you changed the fake clock initial value to the current time. Any particular reason why?

I can't say why, but it sounds somewhat better than an arbitrary random value.

[DPJacques]

I was motivated by what I thought was being requested in #61, although I now understand that issue to be asking for something else. I pondered it a bit this morning, and using time.Now() seems less complex while still accomplishing the desired goal.

Sorry for the late change with no comment beforehand. I was responding to #61. Maybe I should have commented first before pushing. My apologies for that.

[sagikazarmark]

No worries!

I believe this is a better solution, we just needed some time to find it.