Skip to content

Conversation

@funvit
Copy link
Contributor

@funvit funvit commented Jun 13, 2022

Closes #365

DockerContainer func Logs now strips header for each line.

This fix cannot be used with TTY mode.

Why strip header? Due to setting TTY breaks some tests (ex: Test_ShouldRecognizeLogTypes) ...

@codecov
Copy link

codecov bot commented Jun 13, 2022

Codecov Report

Merging #454 (29d70b4) into main (977ff3b) will increase coverage by 0.18%.
The diff coverage is 78.04%.

@@            Coverage Diff             @@
##             main     #454      +/-   ##
==========================================
+ Coverage   68.44%   68.63%   +0.18%     
==========================================
  Files          21       21              
  Lines        1892     1932      +40     
==========================================
+ Hits         1295     1326      +31     
- Misses        480      486       +6     
- Partials      117      120       +3     
Impacted Files Coverage Δ
docker.go 69.93% <78.04%> (+0.37%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 977ff3b...29d70b4. Read the comment docs.

Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@funvit thanks for taking the time to contribute this PR! It LGTM and I'd like to bring @gianarb 's 👀 to the table for the final review.

Thanks in advance!

@mdelapenya mdelapenya requested a review from gianarb June 28, 2022 08:11
@mdelapenya mdelapenya merged commit e70bc70 into testcontainers:main Jun 28, 2022
@mdelapenya
Copy link
Member

Good job here @funvit! 👏👏👏

@funvit funvit deleted the fix/strip-header-from-logs branch June 28, 2022 15:54
LaurentGoderre added a commit to LaurentGoderre/testcontainers-go that referenced this pull request Jul 2, 2025
LaurentGoderre added a commit to LaurentGoderre/testcontainers-go that referenced this pull request Jul 2, 2025
mdelapenya added a commit that referenced this pull request Jul 9, 2025
* modify log consumer test to show how stripping logs sometimes fails

bug introduced by #454

* fix: implement the docker log stream specification to strip headers from logs

* chore: lint

* chore: use require

---------

Co-authored-by: Manuel de la Peña <[email protected]>
Co-authored-by: Manuel de la Peña <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Docker container Logs prints stream format for each log line

2 participants