-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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: Fill IMAGE_TAG,etc on Docker builds #7788
Conversation
Thanks for the PR here! Can you rebase this PR, I think recently one of the issues seen in the tests:
was fixed - Additionally I believe there are some legitimate test failures where some expected values in tests might have to be changed in relation to the change made here. To run the full set of unit tests locally you can use |
Resolves GoogleContainerTools#4295 Docs have long stated that these fields are available when using the local docker builder, now they actually are.
Right - so finally got a chance to get back around to this. Looks like it was just "used to be mocking It does feel like there should be a more elegant way of doing this, particularly with the caching bits (perhaps using In the mean while, I believe the tests should be passing, and the docs match the actual behaviour |
Codecov Report
@@ Coverage Diff @@
## main #7788 +/- ##
==========================================
- Coverage 70.48% 66.57% -3.92%
==========================================
Files 515 593 +78
Lines 23150 28696 +5546
==========================================
+ Hits 16317 19104 +2787
- Misses 5776 8177 +2401
- Partials 1057 1415 +358
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Thanks so much for your work here @0xADD1E! Re:
I've created an issue - #7824 to explore refactoring this as I agree. Tests look good now, enabling auto-merge! Thanks again! |
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.
LGTM!
Fixes: #4295
Description
Docs have long stated that these fields are available when using the local docker builder. This makes is to they actually are.