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

Get rid of sleep time in Docker tests #447

Open
c0c0n3 opened this issue Feb 4, 2021 · 6 comments
Open

Get rid of sleep time in Docker tests #447

c0c0n3 opened this issue Feb 4, 2021 · 6 comments

Comments

@c0c0n3
Copy link
Member

c0c0n3 commented Feb 4, 2021

Is your feature request related to a problem? Please describe.

Alot of our tests depend on external Docker processes and can't be run until those processes are ready. At the moment tests that depend on Docker services get run by Bash scripts that bring up the services and then sleep a number of seconds before starting the pytest testing session. While using long delays in practice works most of the time, in general it is a non-solution to the problem of syncing processes. Also that means long waits everywhere, even in environments where dependant services start up in the blink of an eye.

Describe the solution you'd like

@amotl suggested a nice way to get rid of those nasty sleep commands by using lovely-pytest-docker. That's a nice lib, but I'd be even nicer if docker_services.wait_for_service let you:

  • set a max time past which it should give up polling the service
  • use a backoff strategy, even better if you could specify polling intervals, e.g. first poll after 1 sec, 2nd poll after 1 sec, 3rd poll after 10 secs, etc.
  • catch a given set of exceptions E and retry the call if an exception gets raised that's in E, otherwise bail out

You can do all of these things with the backoff library, but backoff doesn't know how to start/stop Docker processes. So is there a way to get the convenience of lovely-pytest-docker and the flexibility of backoff? I think it could be as easy as wrapping this call:

with a backoff decorator. Perhaps we could contribute that to the lovely-pytest-docker project and then use the new lovely-pytest-docker, even lovelier version in our code.

Describe alternatives you've considered

Just use backoff if there's no need to start/stop Docker services? For an example of using it have a look at reporter.tests.embedded_server.py.

Additional context

See @amotl's comments to #441.

Notes

I think one question we should ask ourselves is why the majority of our tests depend on external services? Ideally, those tests should be the minority with unit tests taking the lion share. If most of the tests we cared about were unit tests, all this wouldn't be much of a problem. Lack of modularity and separation of concerns is what's brought us here I reckon. While this is the fate of most code bases that get developed over many years, we should still try writing clean code if we can---easier said than done given the huge pressure we're under, but trying doesn't hurt :-)

@amotl
Copy link

amotl commented Feb 4, 2021

Hi,

I am happy that you liked my suggestion. As @burnes' reaction to my post also was positive, he might be willing to collaborate with you on adding more of those features to lovely-pytest-docker. Also note that @jeromecremers already asked about the possibility to add parameters timeout and pause to the wait_for_service() workhorse, see lovelysystems/lovely-pytest-docker#19.

Extending that to an exponential backoff feature would be super nice, indeed. Actually using @bgreen-litl's / @litl's backoff module for that job would be a blast.

With kind regards,
Andreas.

@c0c0n3
Copy link
Member Author

c0c0n3 commented Feb 4, 2021

Hi @amotl, yea it'd be great if someone could do that! Like I said, in principle using a decorator from the backoff library could do the trick...

@amotl
Copy link

amotl commented Feb 4, 2021

in principle using a decorator from the backoff library could do the trick...

Indeed. If that would work together nicely, there would be no need to put it into lovely-pytest-docker and it would be just a matter of appropriately documenting that as an "addon feature".

@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2021

Stale issue message

@chicco785
Copy link
Contributor

in progress see #524

@github-actions
Copy link
Contributor

Stale issue message

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

No branches or pull requests

3 participants