Code review comment for lp:~jtv/launchpad/bug-844550

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Graham, you raise a difficult point. I went with the entire-list-at-once assertion for exactly the considerations you give. I'll elaborate.

When an early assertion in a series of them fails, the first thing I always want to know is “yeah but what did it do in the other cases, that are all similar except in one important respect?” For instance, did all publications just get the same status for some reason, or are they put in the wrong order, or is it just one case that's off, or was nothing updated at all, or is something else going on? Often I find myself re-running tests in the debugger, or with the failing assertion commented out, or with reordered assertions just to find out. Hence the choice of an assertion that shows the full pattern I expected and the full pattern that was observed. The alternative would be a series of similar tests, but in this case I particularly wanted to give a single full panorama view on what happens to a typical string of publications.

Ideally, I'd like to annotate the items in the assertion with what's unique about how each of them went through the process, and why I expect them to end up in the given way. In most cases that can be done with a bit of case-by-case creativity.

In this case, I can think of two ways of doing that:

1. Assert the “before action” state in a similar way.

2. Compare not lists, but dicts that map versions to statuses. So it'll read as “1.0 is Superseded,” etc.

I could do either one of these, or both. For brevity, one component of clarity, I'd prefer to limit it to one.

« Back to merge proposal