jonboulle / clockwork

a fake clock for golang
Apache License 2.0
625 stars 57 forks source link

Ticket cannot be tested using fake clockwork #79

Closed Season0518 closed 6 months ago

Season0518 commented 7 months ago

I wrote a simple function that executes a certain piece of code at a specified interval.

func ScheduledTask(c clockwork.Clock) {
    ticker := c.NewTicker(1 * time.Hour)
    go func() {
        defer ticker.Stop()

        for range ticker.Chan() {
            fmt.Printf("responsed\n")
        }
    }()
}

I referred to the Readme and wrote a unit test like this.

func TestScheduledTask(t *testing.T) {
    fakeClock := clockwork.NewFakeClock()

    var wg sync.WaitGroup
    wg.Add(1)

    go func() {
        ScheduledTask(fakeClock)
        wg.Done()
    }()
    fakeClock.BlockUntil(1)
    for i := 0; i < 24; i++ {
        fakeClock.Advance(1*time.Hour + 1*time.Second)
        //time.Sleep(1 * time.Millisecond)
    }
    wg.Wait()
}

The program cannot produce the expected output.

=== RUN   TestScheduledTask
--- PASS: TestScheduledTask (0.00s)
PASS

But when I set a delay of one millisecond for each loop, the output of the program is as follows

=== RUN   TestScheduledTask
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
responsed
--- PASS: TestScheduledTask (0.03s)
PASS

Is this a bug or a usage issue?

DPJacques commented 7 months ago

Wanted to ACK this and say this does feel like a bug. Things are a tad overloaded right now but I will debug as soon as I am able.

I welcome others to look into it as well.

DPJacques commented 6 months ago

This is not a bug. It is caused by a race condition caused by the goroutine created in ScheduledTask.

Without the time.Sleep, your program exits before the goroutine created in ScheduledTask is able to start. There are a number of different ways you can address this, but I'd need to know more about what you are trying to accomplish. I would probably use a channel to validate the goroutine created in ScheduledTask is running before I enter the for loop.