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.

Sign up for GitHub

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

Jump to bottom

LoggingOutputStreamTests failing on Windows #51838

Closed
mark-vieira opened this issue Feb 3, 2020 · 5 comments · Fixed by #52162 or #52253
Closed

LoggingOutputStreamTests failing on Windows #51838

mark-vieira opened this issue Feb 3, 2020 · 5 comments · Fixed by #52162 or #52253
Assignees
@rjernst
Labels
:Core/Infra/Logging Log management and logging utilities >test-failure Triaged test failures from CI

Comments

@mark-vieira
Copy link
Contributor

Some recent changes were made to LoggingOutputStream in #51779 which might be responsible for these failures:

https://gradle-enterprise.elastic.co/s/vokapgu3ogfry/tests/failed
@mark-vieira mark-vieira added >test-failure Triaged test failures from CI :Core/Infra/Logging Log management and logging utilities labels Feb 3, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Logging)

@mark-vieira
Copy link
Contributor Author

@rjernst I'm assigning this to you for now.

@ywelsch ywelsch mentioned this issue Feb 5, 2020
Merged
@ywelsch
Copy link
Contributor

ywelsch commented Feb 5, 2020

I'm muting the tests in #51917

ywelsch added a commit that referenced this issue Feb 5, 2020
@ywelsch
Mute LoggingOutputStreamTests (#51917)
2b8e0f6 
Relates #51838
ywelsch added a commit that referenced this issue Feb 5, 2020
@ywelsch
Mute LoggingOutputStreamTests (#51917)
b4480bb 
Relates #51838
rjernst added a commit to rjernst/elasticsearch that referenced this issue Feb 10, 2020
@rjernst
Fix windows empty line in logging capture
dbee517 
This commit fixes another edge case in handling windows newlines in our
capture of stdout/stderr to log4j. The case is that the \r appears at
the beginning of the buffer when flushing, which would unintentionally
be emitted as an empty string. This commit skips the flush if only a \r
was found.

closes elastic#51838
@rjernst rjernst mentioned this issue Feb 10, 2020
Merged
rjernst added a commit that referenced this issue Feb 10, 2020
@rjernst
Fix windows empty line in logging capture (#52162)
efb35b0 
This commit fixes another edge case in handling windows newlines in our
capture of stdout/stderr to log4j. The case is that the \r appears at
the beginning of the buffer when flushing, which would unintentionally
be emitted as an empty string. This commit skips the flush if only a \r
was found.

closes #51838
rjernst added a commit that referenced this issue Feb 10, 2020
@rjernst
Fix windows empty line in logging capture (#52162)
88cf8ac 
This commit fixes another edge case in handling windows newlines in our
capture of stdout/stderr to log4j. The case is that the \r appears at
the beginning of the buffer when flushing, which would unintentionally
be emitted as an empty string. This commit skips the flush if only a \r
was found.

closes #51838
@davidkyle
Copy link
Member

Reopening as there were 3 recent failures all based on commit 0b493e5 which comes after the fix in 88cf8ac

https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+multijob-windows-compatibility/os=windows-2016/439/console
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+multijob-windows-compatibility/os=windows-2012-r2/439/console
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+multijob-windows-compatibility/os=windows-2019/439/console

Looks like LoggingOutputStreamTests.testMaxBuffer also needs the \r fix.

@davidkyle davidkyle reopened this Feb 11, 2020
davidkyle added a commit to davidkyle/elasticsearch that referenced this issue Feb 11, 2020
@davidkyle
Mute LoggingOutputStreamTests.testMaxBuffer
95cffa2 
Relates to elastic#51838
@davidkyle davidkyle mentioned this issue Feb 11, 2020
Merged
@davidkyle
Copy link
Member

davidkyle commented Feb 11, 2020 •
edited
Loading

Muting again in #52193

Master 3ba5566
7.x 343ced4

davidkyle added a commit that referenced this issue Feb 11, 2020
@davidkyle
Mute LoggingOutputStreamTests.testMaxBuffer (#52193)
3ba5566 
Relates to #51838
davidkyle added a commit that referenced this issue Feb 11, 2020
@davidkyle
Mute LoggingOutputStreamTests.testMaxBuffer (#52193)
343ced4 
Relates to #51838
rjernst added a commit to rjernst/elasticsearch that referenced this issue Feb 12, 2020
@rjernst
Fix single newline in logging output stream buffer
ff5b9ec 
The buffer in LoggingOutputStream skips flushing when only a newline
appears. However, if a windows newline appeared, the buffer length was
not reset. This commit resets the length so the \r does not appear in
the next logging message.

closes elastic#51838
@rjernst rjernst mentioned this issue Feb 12, 2020
Merged
rjernst added a commit that referenced this issue Feb 12, 2020
@rjernst
Fix single newline in logging output stream buffer (#52253)
c99210d 
The buffer in LoggingOutputStream skips flushing when only a newline
appears. However, if a windows newline appeared, the buffer length was
not reset. This commit resets the length so the \r does not appear in
the next logging message.

closes #51838
rjernst added a commit that referenced this issue Feb 12, 2020
@rjernst
Fix single newline in logging output stream buffer (#52253)
c07f464 
The buffer in LoggingOutputStream skips flushing when only a newline
appears. However, if a windows newline appeared, the buffer length was
not reset. This commit resets the length so the \r does not appear in
the next logging message.

closes #51838
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rjernst rjernst

Labels
:Core/Infra/Logging Log management and logging utilities >test-failure Triaged test failures from CI
Projects
None yet
Milestone
No milestone
5 participants
@rjernst @davidkyle @ywelsch @mark-vieira @elasticmachine