Fixes watch unit #2367
Merged
Fixes watch unit #2367
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci-robot
added
kind/bug
mik-dass
force-pushed
the
fix_watch_unit
branch
from
b60ed00
to
20e130a
Compare
openshift-ci-robot
added
the
do-not-merge/work-in-progress
mik-dass
force-pushed
the
fix_watch_unit
branch
from
20e130a
to
015fde3
Compare
mik-dass
force-pushed
the
fix_watch_unit
branch
from
015fde3
to
31488a5
Compare
openshift-ci-robot
added
needs-rebase
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
mik-dass
force-pushed
the
fix_watch_unit
branch
from
31488a5
to
aca19fe
Compare
openshift-ci-robot
removed
the
needs-rebase
mik-dass
force-pushed
the
fix_watch_unit
branch
2 times, most recently
from
5032dc8
to
fe267b4
Compare
mik-dass
force-pushed
the
fix_watch_unit
branch
3 times, most recently
from
99b71ad
to
ad82fc0
Compare
|
As of now it looks good to me. Recently @dharmit and @girishramnani has spent good amount of time on researching UTs cases. So i would recommend atleast anyone of them to have a look into it. |
/kind bug What does does this PR do / why we need it: Fixes the watch unit test and adds more Which issue(s) this PR fixes: Fixes redhat-developer#2365 How to test changes / Special notes to the reviewer: run make test and check if it passes Signed-off-by: mik-dass <mrinald7@gmail.com>
mik-dass
force-pushed
the
fix_watch_unit
branch
from
ad82fc0
to
c5227f2
Compare
kadel
reviewed
Dec 2, 2019
| fmt.Printf("some of the push parameters are different, wanted: %v, got: %v", mockPush, []string{ | ||
| componentName, applicationName, "isPushForce:" + strconv.FormatBool(isPushForce), "show:" + strconv.FormatBool(show), | ||
| }) | ||
| os.Exit(1) |
There was a problem hiding this comment.
Shouldn't this return error, rather than calling Exit?
There was a problem hiding this comment.
we are ignoring the error later on in the code, so the test passes if I don't exit
cdrage
approved these changes
Dec 10, 2019
openshift-ci-robot
added
the
lgtm
cdrage
added
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cdrage 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 |
openshift-ci-robot
added
the
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. |
/retest |
rm3l
added
the
estimated-size/XL (40-60)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What kind of PR is this?
/kind bug
What does does this PR do / why we need it:
Fixes the watch unit test and adds more
Which issue(s) this PR fixes:
Fixes #2365
How to test changes / Special notes to the reviewer:
run
make testand check if it passes