Simplify log-wait-strategy

[danielbodart]

This pull request simplifies the log-wait-strategy in a number of ways:

• Replaces all callbacks with async code
• Separates concerns of timeout and log processing
• Starts timeout immediately so that if Docker/dockerode is slow it will still timeout in the correct amount of time
• Removes bylines dependency
• 95% of all consoles will buffer by line anyway
• More flexible as it could allow a regex across multiple lines when need
• Minor performance improvement
• Bonus 20% less code

I'd also argue that it is a lot easier to read!

[netlify]

✅ Deploy Preview for testcontainers-node ready!

Name	Link
🔨 Latest commit	f5231e1
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-node/deploys/67f35f20bcce78000867356e
😎 Deploy Preview	https://deploy-preview-977--testcontainers-node.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[cristianrgreco]

The refactor looks great! Looks like a test is failing just because the LogWaitStrategy failure message has changed.

One Q: I'm pretty sure that removing byline can result in an intermittent issue. E.g let's say we're looking for the message "Started successfully!". It's possible we get that message in 2 chunks, e.g "Started" and " successfully!". Without buffering into a single line we would not match the message.

[danielbodart]

I'll fix the tests when I'm back at my PC later

[danielbodart]

The message should be exactly the same so I'll fix that.

On the bylines side, I'd argue for an actual example/real use case. Even if you do echo -n, the shell will buffer the output by line, same with python and node I believe.

The only real world example I have seen in the wild is with binary outputs and you don't need new lines

Even if you did need it, you don't need a library to do it. with an iterator it's a couple of lines of code at worst, so less lines of code than using that library!

[danielbodart]

Hey @cristianrgreco I've added tests for both the message not being found and also for if the content is not sent in a single line and they all pass (without need to bylines). Please have a look and let me know what you think?

[danielbodart]

So I readded byline even though the test passes with or without it

[cristianrgreco]

The changes LGTM. I had a doubt about removing the stream.destroy(), IIRC it resulted in some open handles reported by Jest, but I cannot reproduce that anymore. As all the tests are passing as well it doesn't seem necessary.

Are there any other changes you wanna make or are we good to merge?

[danielbodart]

Good from me!