Code review comment for lp:~jkakar/launchpadlib/fake-launchpad

> Thanks for writing this, Jamu. This is such a big gap for anyone writing
> Launchpad clients. I do feel getting some thing into this gap, however
> imperfect, is better than nothing. It's a real shame this has been sitting
> here for 20 months.
>
> Perhaps this should go into a separate project where it can evolve separately
> of lazr.restful/launchpadlib and be perhaps cross a lower barrier? As far as
> I can see it doesn't actually patch anything in launchpadlib; it only adds new
> classes.

That's an interesting idea... though it feels like a compromise, but
maybe one worth making to get the code out and more easily accessible.

> The main thing I would like to see both as a reviewer and would-be user is
> just a README demonstrating how one could use it, and what kind of thing it's
> supposed to help you with.

Good call.

> People trying to catch up on the history of this should read
> https://code.edge.launchpad.net/~jkakar/launchpadlib/testing-
> support/+merge/14444
>
> I'm not very familiar with the internals of the existing implementation but
> the code for this looks reasonable to me, at least to the point where I would
> not be afraid to change it. I would like to see the xml-parsing stuff split
> out a bit from the FakeResource, and perhaps changed to use lxml, but there's
> no reason that needs to be done in the first iteration.
>
> + def test_callable_object_return_type(self):
> + """
> + The result of a fake method is a L{FakeResource}, automatically
> + created from the object used to define the return object.
> + """
> + branches = dict(total_size="8")
> + self.launchpad.me = dict(getBranches=lambda statuses: branches)
> + branches = self.launchpad.me.getBranches([])
> + self.assertTrue(isinstance(branches, FakeResource))
> + self.assertEqual("8", branches.total_size)
> +
>
> I feel I would like this to make a bigger distinction between code that's
> running "behind the scenes" to set up the mock object, vs things being done on
> the front end of the mock object once they're set up. It seems like assigning
> to "me" is a behind-the-scenes setup thing and not something you can do on a
> real Launchpad, whereas accessing its attributes is pretending to be talking
> to a real one.
>
> Maybe this is not a meaningful distinction for this library?

Yeah, so the way it works, and the reason for the weird dict()-based
interface is that you set fake data on the FakeLaunchpad
object... when that happens, it validates it against the WADL file to
make sure that what you're setting is actually possible. Each of
those dict()'s being set on the FakeLaunchpad instance represent
objects that would be exposed on the real Launchpad instance. When
you set an object, say to represent a person, you don't have to
provide all the attributes and methods the real version would have,
just the ones you care about for your test.

It really is a bit of a hack, but the best one I could think of. I
haven't used this code in a real project, so I can only guess that the
WADL validation will prevent silly problems.