franela / goblin

Minimal and Beautiful Go testing framework
MIT License
884 stars 79 forks source link

Added Timeout() for Describe and Top Level #95

Open Neirpyc opened 3 years ago

Neirpyc commented 3 years ago

What was the previous behavior

Previously, calling G.Timeout() inside a Describe or in the top level test function resulted in the following error: panic: runtime error: invalid memory address or nil pointer dereference [recovered] This was because of this piece of code:

func (g *G) Timeout(time time.Duration) {
    g.timeout = time
    g.timer.Reset(time) // <- g.timer not defined outside of an It block
}

Which behavior is this PR bringing

g.Timeout() can be called multiple times in which case the given value applies until a new g.Timeout() is encountered in the same or an higher scope.

What would have been another valid behavior ?

It could be coherent that calling g.Timeout inside a Describe sets a maximum execution time for the whole Describe, and not for every It inside it. The same way, setting the timeout in the top level function could set a global time for all of the Describes instead of a time per It.

Which improvements could be made ?

The test suite for the timeouts is pretty bad, as it calls time.Sleep() many times, which makes the testing slow, and it could be interesting to think of a way to mock the Sleep calls.