-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Bug-compatible refactor of docker-env + output tests #6527
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tstromberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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.
Ah. Sorry about this mistake. I need to make windows test failures look like others ASAP
/ok-to-test |
Error: running mkcmp: exit status 1 |
All Times minikube: [ 93.982657 95.527866 92.615017] Average minikube: 94.041847 Averages Time Per Log
|
PTAL, I've repurposed this PR for cleanup-only work. |
All Times Minikube (PR 6527): [ 95.758353 96.993859 96.684549] Average minikube: 96.066804 Averages Time Per Log
|
Codecov Report
@@ Coverage Diff @@
## master #6527 +/- ##
==========================================
- Coverage 37.95% 37.91% -0.05%
==========================================
Files 138 137 -1
Lines 8708 8683 -25
==========================================
- Hits 3305 3292 -13
+ Misses 4986 4978 -8
+ Partials 417 413 -4
|
Read half of it, looks very nice ! reading the rest |
All Times minikube: [ 94.496072 94.742602 90.992936] Average minikube: 93.410537 Averages Time Per Log
|
The previous tests for
docker-env
did not provide much value, causing issues like #6524 to be both easy to introduce, and difficult to confidently fix.This PR refactors
docker-env
to be trivially tested, by allowing it take only strings and a writer as inputs. This means we no longer have to mock out parts of libmachine, and we can now compare outputs.This PR introduces no behavior changes or new tests. A follow-up PR will fix the following bugs I discovered: