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

Revision history for this message
Jamu Kakar (jkakar) wrote :

> .. you've probably thought about this much more than me, but it seems to me
> you'd want
>
> 1- interfaces that can potentially be used against either a real launchpad or
> a fake one, assuming the code calling them makes no assumptions about the
> existence or nonexistence of particular data - for instance making a person
> and setting some attributes on them should be in that class

Yes. If I understand correctly, that's basically what is in place,
except that the interface is some XML in a WADL file.

> 2- interfaces that simulate behaviour or state we could assume would already
> be present on the other end of a remote launchpad, like the existence of 'me'
> or searchTasks

Yes, these are basically defined in the WADL file.

> 3- interfaces that populate sample data more quickly/efficiently than talking
> to Launchpad

I don't think I really understand this...?

> Why do I think I want this?
>
> + self.launchpad.me = dict(getBranches=lambda statuses: branches)
> + branches = self.launchpad.me.getBranches([])
>
> This seems oddly nonpythonic. I can guess there is a getattr hack there, and
> that's fine for simulating the behaviour on 'me.getBranches', but it seems odd
> to also load things into it by assigning to a dict.
>
> Secondly, I feel that if just assigning to things works, there is a risk
> people will have code that accidentally passes, when assigning to lplib proxy
> objects won't behave the same way. Maybe this isn't a real problem, and we
> just need to write some actual tests using this to see what happens.

As above, the WADL validation should help. The real point of the
FakeLaunchpad instance is just to create a place to hang fake data
off. You then hand the fake into your application, as though it were
a real Launchpad instance, and, if you've provided all the right bits,
everything should work.

It's a bit tricky because launchpadlib doesn't have a set of
pre-defined interfaces. Almost everything it exposes is built based
on the WADL file.

« Back to merge proposal