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

Request: allow use of already previous test session Docker containers #691

Open
devuo opened this issue Oct 17, 2022 · 7 comments
Open

Request: allow use of already previous test session Docker containers #691

devuo opened this issue Oct 17, 2022 · 7 comments

Comments

@devuo
Copy link
Contributor

devuo commented Oct 17, 2022

Context:
Starting Docker containers can take from a few seconds to some minutes depending on the Docker image being used.
The current implementation forcefully kills containers if there is already one running with the same container name. This impedes the ability of setting up a faster test workflow for developers to do so: the re-use of the same container in different test runs.

Proposal:
Allow developers to pass an option like .WithContainerNameReuse("some-container-name") (or other some other option name) that would make Gnomock re-use the container with the given name, instead of killing it and starting a new.

@devuo
Copy link
Contributor Author

devuo commented Oct 17, 2022

I'm more than happy to implement this myself and open a PR, but first would need to know if this functionality is something that you'd find OK to be added to gnomock @orlangure

@orlangure
Copy link
Owner

Hi @devuo, I like the idea!

I can already see two scenarios that we might need to support:

  1. WithContainerReuse or WithExistingContainer that simply translates container information into gnomock.Container object that can be used in tests (with all the c.DefaultAddress goodies). The setup in this case is basically zero.
  2. WithContainerReset or something like this for cases where the container already exists, but needs to be cleaned up before using in a new test. For example, a database needs to be dropped and created again between tests inside the same container. In this case, we should think through the desired API. What would be the clean up code? Maybe per preset? Like WithContainerReset(postgres.Reset()), and each preset will implement Reset function that drops everything somehow. Can be tricky.

We can start with the first option of course and see how it goes, but I would like to eventually support both. What do you think?

@devuo
Copy link
Contributor Author

devuo commented Oct 17, 2022

That's great to hear 😀! Do you want me to take a hand at it, or will you implement it?

@orlangure
Copy link
Owner

If you can, please go ahead. 😼
Thank you.

@orlangure
Copy link
Owner

The first step is now merged, and I'll create a new release soon, thanks to @devuo 😻

I really liked the second, much more complex feature mentioned here, I think it would be great to start thinking about it as well. I'll give it a thought in the background, and I hope somebody can offer some help with defining the feature a bit better so we can proceed to actually implementing per-preset cleanups.

@orlangure
Copy link
Owner

I submitted a first draft of how the reset feature (continuation of reuse) can look like.
Here is the postgres reset implementation as an example: #699

The naive way is to provide a single cleanup function, that can do any work

type ResetFunc func(*Container) error

It works good for postgres since there should be an easy way to drop almost everything and restore with init function provided by the preset. A single function for the entire postgres package works.

When I tried to implement the same thing for localstack package to clean-up s3 buckets, I understood that this way might be wrong, and maybe the function should sit on the Preset instance instead, and Start will accept a Resetter instance instead of a single function.

type Resetter interface {
    Reset(*Container) error
}

The advantage is that the Reset function eventually must have access to the preset configuration, such as "which AWS services were requested in this localstack container?", and this will allow to delete buckets only if s3 exists (otherwise there might be an error).

The downside is that a single interface will make the Reset function unconfigurable beyond the underlying preset configuration. I can think of a few options that I might want to send to Reset in the future, such as WithKeepSchemas in postgres or WithKeepIAM in localstack).

@devuo do you have any thoughts on the issue? I think your input is very valuable, because you built the ability to reuse containers, and this feels like the next step in the same direction.

Of course if anybody else sees the issue and has something to say about it, please go ahead 😼

@devuo
Copy link
Contributor Author

devuo commented Oct 24, 2022

Hey @orlangure that's great, will give feedback in the next day or two 🙏

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