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
Fix intermittent failure of messaging tests #1031
Conversation
.withLogConsumer(new Consumer<OutputFrame>() { | ||
@Override | ||
public void accept(OutputFrame outputFrame) { | ||
System.out.println(outputFrame.getUtf8String()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be replaced by slf4j ? (I think there is a facility from testcontainers)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but most of the content coming back from the container is already log formatted. So it gets messy if you wrap it in more formatting. Hence I just write it as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can remove the logging altogether if you'd prefer. It's just useful for debugging if we hit issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant this one that should properly handle the output frame.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can remove the logging altogether if you'd prefer. It's just useful for debugging if we hit issues.
No worries we can keep it, was just wondering if we can make it gogin to the logger so we can control it from properties, but not a huge issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Latest commit uses Slf4jLogConsumer
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it work as advertised ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works ok. Everything gets prefixed with STDOUT:
, so at least it's clear what's coming from the container and what isn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah, that issue ... I think I did implement my own version some time ago, maybe I'll copy over this repo, if only I could remember where it it
7ce4646
to
57e6a71
Compare
57e6a71
to
f2b1b12
Compare
Fixes #1023