Add contexts that use FakeClock rather than the system time.

[DPJacques]

These contexts work similar to the standard library's context.WithTimeout and context.WithDeadline, expect they use a clockwork.FakeClock for deadline determination.

By nature, the generated contexts ignore parent cancelation if the parent's cancelation returned conext.DeadlineExceeded. However, an unfortunate but necessary nuances is that once the parent is cancelled with conext.DeadlineExceeded, future calls to cancel the parent due to otherwise normal calling of the parent's cancel function are not communicated to the child. As far as I can tell, there is no way to cleanly, reliably communicate that.

Once this occurs, and the child has ignored the parent cancellation, the only way to cancel the child is to advance its fake clock past the context's deadline or call the cancel function returned when the context was created.

Other changes that were made in support of this change or as targets of opportunity:

• newFakeTimer was made a standalone function since it needed to be used in 2 places.
• Functions with the pattern new*AtTime were introduced to ensure the FakeClock mutex is held when we need to call time.Time.Sub(FakeClock.Now()) and pass that to a function that also needs to hold the FakeClock mutex. The common use case for this is supporting functions that take an absolute time rather than a duration.
• Use slices.DeleteFunc where we can.

[3vilhamster]

Hey. I'm trying to understand how it supposed to be used.

Case that I have in mind:

func Handler(ctx context.Context) {
   ctx := context.WithTimeout(ctx, 5*time.Second)
   ...
   // do some method with a maximum of 5 seconds timeout.
   select {
      case <-readyChan:
         // finished work
      case ctx.Done():
         // handling timeout
   }
}

That is why in my PR I proposed a generic method that inputs Clock, not *FakeClock.

I wonder which usage you have in mind for these methods.

[DPJacques]

I think this is more of a question on how to inject a FakeClock into a scope.

In your case above, I'd do:

type Handler struct {
  fakeClock *clockwork.FakeClock
}

func (h Handler) Handle(ctx context.Context) {
  if h.fakeClock == nil {
    // Running in prod
    ctx = context.WithTimeout(ctx, 5*time.Second)
  } else {
    // Running in test
    ctx = clockwork.WithTimeout(ctx, h.fakeClock, 5*time.Second)
  }

  ...
  // do some method with a maximum of 5 seconds timeout.
  select {
    case <-readyChan: // finished work 
    case <-ctx.Done(): // handling timeout
  }
}

[3vilhamster]

This means that FakeClock goes to production, which is undesirable. Usually, I pass the Clock interface used in prod and tests. Otherwise, the code path diverges between tests and production.
In your example, h.fakeClock != nil path is impossible to test.

[DPJacques]

This means that FakeClock goes to production, which is undesirable.

Production code will have a clockwork.Clock interface that switches between a RealClock and FakeClock, so a FakeClock is "in production" in either case, it just hides behind an interface most of the time. Here it would be nil in production, so I wouldn't qualify that as "in production"

Otherwise, the code path diverges between tests and production.

This always happens to some non-zero degree when you have to inject a clock. You have to communicate to your code whether to use a RealClock or a FakeClock somehow.

I don't see a single if-else statement as terribly egregious, The newly created context uses the same code path regardless of how it is initialized.

In your example, h.fakeClock != nil path is impossible to test.

The following would work:

// In test
fc := clockwork.NewFakeClock()
h := Handler{fakeClock: fc}
go func() {
  h.Handle( context.Background())
} ()

// do things with fc.

// in prod - for posterity
h := Handler{}
h.Handle(context.Background())

---

In general, is your contention that this PR's implementation of clockwork.WithTimeout and clockwork.WithDeadline should take a clockwork.Clock rather than a FakeClock? If so, that is not possible to do without introducing potential race conditions or type inspection to extract the FakeClock (in which case, you should just pass the FakeClock, IMHO).

We could change this so if clockwork.WithTimeout and clockwork.WithDeadline receive a nil clock, they just fall back to context.WithTimeout and context.WithDeadline. Would that help assuage your concerns?

[3vilhamster]

This does not look like a nice API to me, but that might be just me.
Even typecasting inside the method would be better.

What I usually have is:

Handler{clock Clock}

which is set to a proper one in prod and a fake one in tests. From my experience, it is the best API and separates the library's responsibility from the developer's.

Your proposal will make all library users aware of Clock and Fakeclock, which does not look like a great design.

In my initial proposal, I used a method instead of a function. That will allow separation of implementation without type assertions.