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.

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

Add instrumentations for containers created with Dockerfile #311

Merged
merged 7 commits into from
May 21, 2021
Merged

Add instrumentations for containers created with Dockerfile #311

merged 7 commits into from
May 21, 2021

Conversation

hey-xico
Copy link
Contributor

@hey-xico hey-xico commented Apr 13, 2021

Add support for ARGS when using Dockerfile to create images.
Also allow users to enable/disable build log.

@codecov
Copy link

codecov bot commented Apr 13, 2021

Codecov Report

Merging #311 (df36248) into master (555d3e6) will increase coverage by 0.10%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #311      +/-   ##
==========================================
+ Coverage   63.18%   63.28%   +0.10%     
==========================================
  Files          13       13              
  Lines         910      918       +8     
==========================================
+ Hits          575      581       +6     
- Misses        247      248       +1     
- Partials       88       89       +1     
Impacted Files Coverage Δ
docker.go 65.00% <66.66%> (+0.02%) ⬆️
container.go 85.29% <100.00%> (+0.91%) ⬆️

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 555d3e6...df36248. Read the comment docs.

@hey-xico
Copy link
Contributor Author

Hey guys! Can you check this PR, please!?

container.go Outdated Show resolved Hide resolved
@hey-xico hey-xico changed the title Add support for build args Add instrumentations for containers created with Dockerfile Apr 20, 2021
@hey-xico
Copy link
Contributor Author

Hey @gianarb how's it going? I've added a way to print the build log of Dockerfile. Can you validade if, please?

@gianarb
Copy link
Member

gianarb commented Apr 27, 2021

I have to admit that the idea to print to stdout makes me a bit nervous, I saw we do that in other places because the library does not have a logger, but still it is something we should avoid. Can you tell me a bit more about why you need a log? To figure out why a build failed?! I am wondering if we can, at least for now troubleshoot based on just the error without a full log.

Or we can write a TODO: mentioning that at some point we expect a logger (ideally in tests it should be t.Log())

@gianarb
Copy link
Member

gianarb commented May 3, 2021

@xicoalmeida can you rebease? I am happy to merge and release a new version of testcontainers

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.

As @gianarb mentioned, this PR will be ready to go after a rebase 👍

Thanks for your contribution!!

@hey-xico
Copy link
Contributor Author

Hey @mdelapenya, I've just rebased.

@gianarb gianarb merged commit 0d6a586 into testcontainers:master May 21, 2021
@gianarb
Copy link
Member

gianarb commented May 21, 2021

Thanks a lot!

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.

3 participants