Alissa Bonas has posted comments on this change.

Change subject: core: add validation non empty connection field
......................................................................


Patch Set 2:

Sergey - thanks for your comment.

If someone changes the error message content (its text) - the test won't fail. 
If someone changes the type of error message (use a completely different error 
from VdcBllMessages) then probably the test will fail and will need fixing. I 
think it's good, because changing the type of error means a potential change in 
flow and perhaps in functionality. Having tests failed will be a good 
indication that it needs either fixing due to new functionality, or that a 
regression was introduced.  
I see nothing wrong in correcting unitests when needed. 
And though validator is good in some cases (and IMO the validator itself will 
also need unitests to see it works, so I don't see how it matters), some cases 
in CanDoAction check very different things (like checking connection validity, 
host validity, if something exists in db, etc) - it will require several 
validators anyway.

-- 
To view, visit http://gerrit.ovirt.org/15560
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d9d133faa54e74a9c4c14a93edb6bd16a9ee796
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to