Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Timeout() for Describe and Top Level #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Neirpyc
Copy link
Contributor

@Neirpyc Neirpyc commented Oct 10, 2020

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

  • When inside an It block, nothing changes
  • When inside a Describe block, sets the timeout to the given value, except when overridden inside an It block (only affect the It inside which is the g.Timeout())
  • When outside of any Describe or It it is applied to every It block unless overriden in the It or in the parent Describe.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant