-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Make TestFluentdCommand
tests stable on Windows
#4096
Make TestFluentdCommand
tests stable on Windows
#4096
Conversation
The surviving processes would make the tests unstable on Windows. Signed-off-by: Daijiro Fukuda <[email protected]>
Signed-off-by: Daijiro Fukuda <[email protected]>
TestFluentdCommand
tests stable on WindowsTestFluentdCommand
tests stable on Windows
Still fails... 😢 |
Thanks, it seems netter than before, but still sometimes went wrong. https://github.com/fluent/fluentd/actions/runs/4407552329/jobs/7721373310
|
It looks like the same phenomenon. I made this fix as I thought it was caused by other surviving processes that were not killed, |
Last night, I can't reproduce it on my local in this branch, but now it reproduces on my local. |
Sorry, this log is from the worker process, so it could take worker logs too,,,
Hmm, now I think this is caused by short timeout interval |
On Windows, sometimes 10s is too short and can't take some logs to assert. Signed-off-by: Daijiro Fukuda <[email protected]>
Trying timeout: 20s ... |
Now looks stable! |
I'll rerun CI a few times to confirm. |
The failing test is not related to this. https://github.com/fluent/fluentd/actions/runs/4411233957/jobs/7730096945
|
Not related to this again. https://github.com/fluent/fluentd/actions/runs/4411233957/jobs/7731373094
https://github.com/fluent/fluentd/actions/runs/4411233957/jobs/7731373003
|
For |
Thanks! |
Thanks for your review! |
Which issue(s) this PR fixes:
Fixes #4095
What this PR does / why we need it:
Make
TestFluentdCommand
tests stable on Windows.(Please see #4095 for details.)
Docs Changes:
Not needed.
Release Note:
Not needed.