-
Notifications
You must be signed in to change notification settings - Fork 244
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
Fixes watch unit tests for windows #2692
Fixes watch unit tests for windows #2692
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2692 +/- ##
==========================================
+ Coverage 43.87% 43.92% +0.04%
==========================================
Files 82 86 +4
Lines 7581 7757 +176
==========================================
+ Hits 3326 3407 +81
- Misses 3931 4015 +84
- Partials 324 335 +11
Continue to review full report at Codecov.
|
@mrinal i do not see any unit test failure even after executing
Also can you please update the windows thing just we discussed in triage call so that i can test it on windows too. |
Please try it at least 100 times, just to be extra sure :D
Done |
It was passing because i was passing wrong package name :(. After correcting the script i hit the same flake. Also surprisingly i am also hitting same flake through
|
@mik-dass seems your fix does not work properly on Windows. Note, i used cygwin terminal See the failure logs
|
#2562 got merged and I ran watch test on windows locally. I am not able to see any failure from watch test but as I stated earlier we were getting some failure in debug file because of watch, which is still there I guess. I have mentioned the logs output below:
AFAIR when the watch test was skipped we were not getting this failure anymore on windows. So there is a strong probability of this failure reason to be watch test. @mik-dass can you please have a look on this and correct me if I am wrong :) |
@prietyc123 That's a different flake error occurring in debug tests. I have a raised a separate PR for this flake #2702
I am not seeing this PR on the Travis CI. Thus I think this is a flake in the debug test. |
/retest |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: girishramnani 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 |
/lgtm |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest Please review the full test history for this PR and help us cut down flakes. |
Removes the unit tests requiring the survey package from windows runtime. Signed-off-by: mik-dass <mrinald7@gmail.com>
/retest |
/lgtm |
What type of PR is this?
/kind bug
What does does this PR do / why we need it:
It removes the unit tests requiring the survey package and fixes the watch unit tests for windows.
Which issue(s) this PR fixes:
Fixes #2570 (windows part)
How to test changes / Special notes to the reviewer:
// +build !windows
totesting\survey_ui.go
andodo\cli\service\service_test.go