Code review comment for lp:~jkakar/launchpadlib/testing-support

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

Thanks for the comments! I have some opinions/ideas about the
branch:

- I agree that keeping FakeLaunchpad's behaviour in sync with a real
  Launchpad instance is tricky. I haven't done anything about it in
  this branch mostly because of time, but writing a set of tests to
  ensure that FakeLaunchpad correctly mirrors Launchpad is not
  impossible.

- The benefit of being able to write unit tests is massive. For
  example, it's particularly difficult to write a robust
  launchpadlib-using application because accessing (many) attributes
  can result in HTTP exceptions being raised. Dealing with such a
  fragile environment without tests is pretty much impossible. I
  would rather deal with problems that come up due to fake/real
  mismatches and be able to write tests than not.

- Running Launchpad locally is painful. Asking users of the API to
  setup a local Launchpad installation creates a barrier to entry
  and an ongoing maintenance headache. If this turns out to be the
  answer I suspect users will opt not to write non-trivial
  applications with launchpadlib. I will be much less excited about
  using the library if I have to maintain a local Launchpad setup.

- I included a WADL file in the branch because it was the easiest
  way for me to get going with the idea. I've envisioned making it
  possible to download a WADL file as part of a test run from the
  beginning. Adding a network dependency to a test suite is an
  annoyance, but one I'd be happy to live with.

- The note about validating method parameters was not about it being
  impossible, just about it not being done yet. The WADL file
  includes enough data to do some validation. It's true that the
  WADL file doesn't provide all the information necessary to do a
  perfect job, but I feel it provides enough to be useful.

Obviously I feel that this branch (or the idea, at least) is useful,
otherwise I wouldn't have implemented it. I'm even curious about
taking it farther and seeing if something like this can be pushed
into lazr.restful[,client], but I don't know enough about either of
those things to know how realistic that might be.

Regardless of whether or not this branch is accepted, I feel that
the testing story for launchpadlib is rather poor at present and
that something needs to be done about it. I want to write
applications using launchpadlib that I can depend on and, at
present, I can't do that.

« Back to merge proposal