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

It is safe NOT to call gnomock.Stop()? #1024

Open
your-diary opened this issue Dec 8, 2023 · 1 comment
Open

It is safe NOT to call gnomock.Stop()? #1024

your-diary opened this issue Dec 8, 2023 · 1 comment

Comments

@your-diary
Copy link

your-diary commented Dec 8, 2023

According to the documentation of Start(),

It is recommended, but not required, to call gnomock.Stop() when the tests complete to cleanup the containers.

But it isn't clear what will happen when you don't explicitly call gnomock.Stop().

  • As far as I tested, even if I don't explicitly call gnomock.Stop(), a container is terminated after a test. Is this behavior guaranteed?

  • During a test, docker ps shows a container for DB and unknown gnomock-cleaner. What is this? Is there any documentation for this?


Edit: I just found Start() internally calls StartCustom() (source) and the documentation of StartCustom()says (emphasis mine):

The returned container must be stopped when no longer needed using its Stop() method.

This contradicts with the documentation of Start() above. Which should I trust?


Edit2: While reading the source code, I found this:

// WithDisableAutoCleanup disables auto-removal of this container when the
// tests complete. Automatic cleanup is a safety net for tests that for some
// reason fail to run `gnomock.Stop()` in the end, for example due to an
// unexpected `os.Exit()` somewhere.
func WithDisableAutoCleanup() Option {
	return func(o *Options) {
		o.DisableAutoCleanup = true
	}
}

Is this related?

@orlangure
Copy link
Owner

Hi @your-diary, thanks for the question and sorry for taking so long to respond.

First, it is safe to not call Stop if everything else is fine (cleaner container started up and nothing broke). I still recommend calling Stop as a best practice and a good habit of closing unused resources once they are not needed anymore.

Second, you should trust the code above the comments 😼 I might have missed some documentation changes when adding the cleaner container at some point, including changing the word most in one place, and adding information about automatic clean up in another place. Would you like to help me out and update the docs so everything becomes clear? You are correct, there is a bit of a mess.

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

No branches or pull requests

2 participants