-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 StreamType.RAW frame processing #1101
Conversation
public class OutputStreamWithTTYTest { | ||
|
||
@Rule | ||
public GenericContainer container = new GenericContainer("alpine:3.2") |
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.
if you change to GenericContainer<>
, you can remove ((CreateContainerCmd)command)
cast
|
||
@Rule | ||
public GenericContainer container = new GenericContainer("alpine:3.2") | ||
.withCommand("/bin/sh", "-c", "sleep 2; ls -1" + |
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.
- could you please remove
+ ""
- would be better to not use
sleep
here because it adds constant 2 seconds to the overall test execution time
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.
...also, you might want to use OneShotStartupCheckStrategy
here
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.
Sorry, but I was forced to do so because ls command is too fast and container is closes before Testcontainers have time to connect to it.
Solution with Google ping is not the case for me because I behind the firewall and cant ping the Internet. 😄
container.followOutput(consumer, STDOUT); | ||
|
||
consumer.waitUntil(frame -> frame.getType() == STDOUT && frame.getUtf8String().contains("home"), | ||
30, TimeUnit.SECONDS); |
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 would remove the line break
|
||
assertThrows("a TimeoutException should be thrown", TimeoutException.class, () -> { | ||
consumer.waitUntil(frame -> frame.getType() == STDOUT && frame.getUtf8String().contains("qqq"), | ||
4, TimeUnit.SECONDS); |
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.
is it necessary to be 4 seconds?
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.
It's can be 3 seconds 😃 Just should be greater than sleep in running script.
|
||
@Test | ||
public void testFetchStdout() throws TimeoutException { | ||
|
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.
"") | ||
.withCreateContainerCmdModifier(command -> ((CreateContainerCmd)command).withTty(Boolean.TRUE)); | ||
|
||
private static final Logger LOGGER = LoggerFactory.getLogger(OutputStreamTest.class); |
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.
please use @Slf4j
annotation from Lombok
…for faster tests.
core/src/test/java/org/testcontainers/junit/OutputStreamWithTTYTest.java
Outdated
Show resolved
Hide resolved
@vektory79 thanks for your contribution! 👍 |
Released in 1.10.4! Thanks for the contribution! |
Fix for Issue #1100. It's solve my problem.