Code review comment for lp:~justin-fathomdb/nova/bug724623

> Thanks for outlining the process, but I think this only applies when I'm
> fixing a bug at the same time / am able to fix it myself.

I said the opposite, actually:

* If it is a bug that is reproducible in trunk (i.e. not *just* the
topic branch you're working in):
  (1) File the bug in Launchpad
  (2) Do not fix the bug in your topic branch
  (3) If you want to assign yourself to the bug you just filed, then:
    a. Branch a bugfix branch from your local *trunk* branch (NOT your topic branch)
    b. Add a test case to your bugfix branch that will trigger the bug
    c. Patch code to fix the bug and pass the test case
    d. Push to LP with --fixes=lp:XXXX where XXX is the bug number
    e. Propose for merging your bugfix branch into trunk

> The goal of the
> known_bugs file is to cope when I'm not fixing a bug at the same time. Some
> examples:
>
> * When I'm doing some of my 'advanced' unit tests, I'd rather use fake_carrot
> than RabbitMQ, but there was an issue in it which turned out to be way beyond
> my knowledge of the queuing stuff to solve (Vish fixed it). Though Vish fixed
> it quickly, I had to fall back to using RabbitMQ while the fix was in-
> progress.

Then you should simply file a bug. There is no reason to have the known_bugs.py file IMO. That is what Launchpad and all of its tracking goodness is for...

> * The authentication for OpenStack is currently 'unusual', and not the
> username and password you might expect. By controlling that with a known_bugs
> flag, once this is fixed the flag can be removed and then the tests will
> continue to pass. I can't just fix this bug, because it has to go through
> "the process".

The known_bugs flag doesn't "control" anything. The above scenario just means that your work depends on a fix for a bug. If you don't want that to delay you, you have the option of either fixing the bug if possible or nagging someone to fix it. Disabling a test case that happens to fail when you change code behaviour to depend on a potential bug fix or community-decided change to, say, the API, is not an option. This only leads to long and painful lists of dependent branches that all come crumbling down when the bug is fixed in a way that you didn't foresee. Or at least, that's been my experience. Having a file that disables tests is just not a good future-proof way to "push through" the inconvenience of a bug fix holding up code you want to get merged. I agree it's a pain, but it's a necessary pain and I believe adding the known_bugs.py file makes the source tree vulnerable to bugs via procrastination since the test is disabled and "hidden" from impacting automated testing.

Believe me, Justin, I understand the frustration that comes with having to wait for bug fixes. I do. Based on my experience in other projects, I just feel this particular patch is not a workable solution. But, it's just my opinion. I'll respect the wishes of other contributors if they feel the known_bugs solution is one that should be promoted.

> * Some of my unit tests bring up a WSGI server, but I haven't currently
> figured out how to shut it down cleanly, so I need to recycle the same WSGI
> instance across tests. If someone that knows WSGI fixes this, I'd like to
> shut it down cleanly etc. My unit test code could also be a head start for
> the bug fixer in terms of diagnosing the problem (once the flag is removed,
> they'll get the failing tests immediately)

Again, I think the solution is to file a bug, and work with/nag the bug-fixer to merge a fix in as short a time as possible.

> I agree that known_bugs is a bit ugly, but I think it has some upsides in
> terms of keeping this information neatly in one place in the code. I
> definitely expect that we'd file bugs alongside adding known_bugs flags - this
> isn't intended to be an end-run around that. I'm very open to better
> alternatives!

I feel you, man :) I'm just going to stick to my point on this one and listen to what others have to say. :) I'm in jury duty all this week (writing this on lunch break) so if I don't immediately respond to further comments, that's what is going on. Just a heads up.

-jay