Nir Soffer has posted comments on this change.
Change subject: tests: Add simple mocking library
......................................................................
Patch Set 1:
> I don't understand why one would need to check if things are called with
> certain paramters.
Because this is how you verify that an object is doing what it supposed to do.
You have object A that should call object B.start(True). Calling B.start(True)
in the tests is not possible (B.start is very slow or depends on external
resource not available during the test). So you create a mock for B, and tell
it what to expect. If B.start was not called, or called with wrong parameter,
your test fail.
> This makes the test assume things about the method being called that are not
> part of it's declared interface.
It does not.
> You should never check if something calls something else or if it uses
> certain parameters.
You should check that when needed.
> The algorithm of the method is malleable and tests should not fail if you
> change it.
Not clear what you mean.
> When you test things like that you are actually just making it so that the
> person changing the algorithm needs to change the test as well even when they
> fix things.
> Lets say we need to change a certain parameter to a called function so that
> the actual interface is fixed. In that case, even though we fix the method
> the test would fail.
If the interface between your objects changes, the tests should change.
> Functions are black boxes as far as unit tests are concerned. By ignoring
> that you make the developer confused whether they actually broke something or
> if the test is just there to give him extra work.
There are different types of tests. When you can tests something as black box,
it is nice, but sometimes you need to test is as white box. It depends on the
code tested.
> For the use case of making stubs easier. Stubs usually need to have logic and
> state. So you'll probably implement you own classes.
I don't want to create stubs.
> Further more, making a function that makes for functions is needlessly
> convoluted.
> for instance to make a function that returns a value you can do
> def foo():
> return "abc"
> This syntax though a bit more verbose is clear and well understood by all
> programmers. It also uses python which means you can add extra logic as
> needed.
> So instead of c = mock.Callable().expect("foo").result("bar") you can de def
> c(s): self.assertEquals("foo", s); return "bar" which is only 3 chars longer
> > (If I remove the optional space chars it is actually shorter) but doesn't
> require someone to learn your DSL and is endlessly extensible.
I'm not trying to make a function that return a value. I want to function that
return different values or raise an exception, simulating a real flow when a
function is called multiple times and return different values, or fails.
Check the example tests here to understand why a mock is useful:
http://gerrit.ovirt.org/#/c/20720/17/tests/toolLvmDeactivateLvsTests.py
To create a stub that that this, I have to either create a stub for each test,
or create completed stub which needs its own tests.
--
To view, visit http://gerrit.ovirt.org/21155
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5d874f553b6a983652ed745d7d8554716e7a15e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: mooli tayer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
