Make test result doubles accept event log

[jml]

I was looking around at the test result code in preparation for using it with flocker. While I was there, I noticed a heap of lint in flake8, so I've fixed it.

There's a small semantic change also. Add an event_log parameter to all of the test result doubles, and use that if it's provided. This spares code that uses them from having to know about _log.

[thomir]

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

^{testtools/testresult/real.py, line 387 [r1] (raw file):}
It's not clear to me what 'xs' stands for? Perhaps consider changing this name to be more descriptive?

---

Comments from the review on Reviewable.io

[jml]

Review status: 2 of 3 files reviewed at latest revision, 1 unresolved discussion.

---

^{testtools/testresult/real.py, line 387 [r1] (raw file):}
Changed.

FWIW, xs is a Haskell idiom for a list of arbitrary things.

---

Comments from the review on Reviewable.io

[thomir]

LGTM - ship it ;)

[rbtcollins]

Review status: 2 of 3 files reviewed at latest revision, 8 unresolved discussions.

---

^{testtools/testresult/real.py, line 358 [r2] (raw file):}
I'm confused by these reflowings - they're not harmful, but why are you doing them?

---

^{testtools/testresult/real.py, line 387 [r2] (raw file):}
This should be _strict_map, since you're not intending it to be in the public API AFAICT.

And please don't call it SOMETHING_map if it doesn't abide by maps' signature (which it doesn't).

---

^{testtools/testresult/real.py, line 389 [r2] (raw file):}
renaming this is possibly going to break people - and its not clearly marked as private, so I'd like to keep an alias to it around.

The change from _args + *_kwargs to function, items is making your new thing incompatible with actual map (which can take many sequences), so the name change does needs to happen if you're making that change, so please keep an unaltered version around - perhaps with a deprecation warning or something.

---

^{testtools/testresult/real.py, line 431 [r2] (raw file):}
I don't super object, but I also don't understand these reformattings. If the function was nested I would object because it becomes very cramped on the right hand side. I'm guessing you ran a naive pep-8 thingy over the code?

---

^{testtools/testresult/real.py, line 702 [r2] (raw file):}
This is an example of where I'd start to object: this openning bracket aligned style quickly becomes unmanagable. Hanging indents don't, and are PEP8 (if that is whats driving this...)

---

^{testtools/tests/test_testresult.py, line 227 [r2] (raw file):}
I find the application of top level VWS rules around nested classes impedes readability - it conflicts with the VWS rules for inside functions which advise sparing use. I'd rather we turned the rule off entirely for that rather than blindly follow it.

---

^{testtools/tests/test_testresult.py, line 533 [r2] (raw file):}
Similarly here, the massive use of height impedes comprehension.

---

Comments from the review on Reviewable.io

[rbtcollins]

Ah, I just noticed - flake8. So my experience with flake8 is that you have to turn a bunch of its checks off to have clean code. Otherwise you get consistently bad code. pep8 was *never intended for use outside the standard library, and while I do subscribe to the gofmt philosophy, flake8 ain't that: because a) it doesn't pretty print for you, so when the rules are in the way you can't issue new rules and have it Just Work, and b) the rules haven't been hammered out in the expectation that everyone will blindly apply them.

[jml]

Hi Rob,

I know that PEP 8 was not written with the intent of being used outside the standard library. I've read the darn thing several times over.

Nevertheless,

• it is used very widely outside the stdlib
• it's the documented coding standard for testtools

My experience with flake8 is almost directly contrary to yours.

jml

---

Review status: 1 of 3 files reviewed at latest revision, 8 unresolved discussions.

---

^{testtools/testresult/real.py, line 358 [r2] (raw file):}
Standard column limits. See summary comment.

---

^{testtools/testresult/real.py, line 387 [r2] (raw file):}
Agree re _ prefix. Have changed to _strict_map and changed items to *sequences, which matches the map signature.

---

^{testtools/testresult/real.py, line 389 [r2] (raw file):}
Added domap alias with deprecation warning, since it's an inappropriate function to be providing publicly from this module, and arguably out-of-scope for our public API altogether. Maybe we can add something to extras if we care enough?

---

^{testtools/testresult/real.py, line 431 [r2] (raw file):}
Yes, flake8.

---

^{testtools/testresult/real.py, line 702 [r2] (raw file):}
Yeah, I don't like this version either. Changed to putting the arguments as hanging indent with first arg on a newline, which is PEP 8 compliant and addresses that issue.

---

^{testtools/tests/test_testresult.py, line 227 [r2] (raw file):}
I genuinely find the version with vertical whitespace easier to read—I'm not blindly following anything. However, I think the readability gain is marginal, so I've reverted.

---

^{testtools/tests/test_testresult.py, line 533 [r2] (raw file):}
Ah really? It's the opposite for me. Also makes it less likely to miss something, esp. if they're alpha sorted.

---

Comments from the review on Reviewable.io