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

Enable line terminator matching for LogMessageWaitStrategy #982

Merged
merged 4 commits into from Dec 23, 2018
Merged

Enable line terminator matching for LogMessageWaitStrategy #982

merged 4 commits into from Dec 23, 2018

Conversation

ghost
Copy link

@ghost ghost commented Nov 19, 2018

Enable line terminator matching to make expressions like ".*" matching even the end of a log line e.g. "waiting for connections on port 20017\n"

@ghost ghost requested review from bsideup, kiview and rnorth as code owners November 19, 2018 10:18
@ghost
Copy link
Author

ghost commented Dec 20, 2018

Any thoughts on this?

@bsideup
Copy link
Member

bsideup commented Dec 21, 2018

Hi @bengtbrodersen0042,

Could you please add tests for it?

@ghost
Copy link
Author

ghost commented Dec 21, 2018

I had to adjust a test only.

@rnorth
Copy link
Member

rnorth commented Dec 23, 2018

I guess I'd just like to check that people won't have problems if they still have the line terminator in their patterns. What happens if people don't update their tests?

It would be sad if this were a breaking change.

@rnorth rnorth added this to the next milestone Dec 23, 2018
@rnorth
Copy link
Member

rnorth commented Dec 23, 2018

I'm just going to add an additional test to make sure both cases are covered - looking good though.

@rnorth
Copy link
Member

rnorth commented Dec 23, 2018

I've pushed a commit with additional test cases.

@qoomon
Copy link
Contributor

qoomon commented Dec 23, 2018

Thx, I was quite busy

@rnorth
Copy link
Member

rnorth commented Dec 23, 2018

I'm going to ignore the failing Travis test - it looks like Travis has gotten confused due to me mistakenly pushing a copy of this PR into a branch in the main repo.

@rnorth rnorth merged commit dd88880 into testcontainers:master Dec 23, 2018
@rnorth
Copy link
Member

rnorth commented Dec 28, 2018

Released in 1.10.4!

Thanks for the contribution!

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

Successfully merging this pull request may close these issues.

None yet

3 participants