[00:08] <teward> cjwatson: this is the test output for the DMARC-changed code.  There was early tests also which were also doctests erroring because the from addresses did actually change and didn't match - but when those were revised the remaining errors are the only ones I see failing AFAICT.  https://people.ubuntu.com/~teward/lpdev-dmarc.txt
[00:09] <teward> (Chrome exploded when I tried to paste it into a pastebin)
[00:09] <teward> failed errors are towards the end
[00:10] <wgrant> teward: That looks fine
[00:10] <wgrant>     - Subject: [Bug ...] subject
[00:10] <wgrant>     ?               ^^^
[00:10] <wgrant>     + Subject: [Bug 18] subject
[00:10] <wgrant>     ?               ^^
[00:10] <mup> Bug #18: RFE: I'd like to be CC:d automatically to bugs I report <feature> <iso-testing> <lp-bugs> <Launchpad itself:Fix Released by bradb> <https://launchpad.net/bugs/18>
[00:10] <cjwatson> teward: Yep, as noted, this is doctest output being surprising to the uninitiated
[00:10] <wgrant> That's the full diff, not necessarily the diff that caused the failure.
[00:10] <wgrant> It doesn't include whitespace normalisation, and it doesn't include ellipsis.
[00:11] <teward> 👍 thanks cjwatson
[00:11] <teward> the only other failure cases are the From addresses
[00:11] <teward> which *are* what changed in my diff
[00:11] <cjwatson> You should find that if you fix just the From: bits then the rest will match
[00:11] <teward> that's the tricky part in *that* doctest
[00:12] <teward> unless it's always using 18

[00:12] <teward> ack
[00:12] <cjwatson> That's what ellipses are for
[00:12] <teward> those doctest errors are confuzling to me :)
[00:12] <wgrant> Sometimes I wonder if that was one of the design goals
[00:12] <wgrant> They can be remarkably obtuse.
[00:16] <teward> that i don't doubt xD
[00:16] <teward> but ODDLY no other bug notification tests errored heh
[00:16] <cjwatson> That is a little remarkable to me, but OK
[00:18] <cjwatson> I guess we don't *have* to have a bazillion tests that interact with everything.  Usually there's more fallout from a change like this though
[00:19] <teward> cjwatson: indeed.  yeah the lack of other things exploding here was also something that made me headscratch.  Unless we don't *actually* test bug delivery?
[00:20] <cjwatson> Well, it evidently is being tested in that doctest
[00:20] <teward> yep
[00:20] <cjwatson> Unexpectedly light coverage, but we are actually testing it
[00:21] <cjwatson> Albeit slightly in passing
[00:23] <cjwatson> Though, I don't understand how your changes didn't cause bugnotification-email.txt to fail
[00:23] <teward> it did
[00:23] <teward> i fixed those
[00:23] <cjwatson> Oh, I see
[00:23] <teward> those were the straightforward fixes
[00:23] <cjwatson> OK, good
[00:24] <cjwatson> That's where get_bugmail_from_address is unit-tested (albeit in a ridiculous format, but they're conceptually unit tests)
[00:24] <teward> yeah and those (as expected) failed
[00:25] <teward> my git tree has two documentation test fixes - one for the latest errors (Ellipses are evil) and one with the earlier two from address mismatches (which were very easy fixes by replacing the email addresses)
[00:25] <cjwatson> And hopefully the surrounding prose commentary
[00:25] <teward> wanted to get the tests passing before I rewrite the commentary :)
[14:13] <teward> literally just filed a short time ago - https://code.launchpad.net/~teward/launchpad/+git/launchpad/+merge/383643 - but this should handle the DMARC compliant bug mail issue.  The documentation needs vetted because I basically rewrote a whole portion of it.  Kept all four test cases for From addresses, but the function is much less verbose and quite straightforward - I kept the test cases to show behavior.
[14:14] <teward> tests pass here, which was... surprising heh.
[14:18] <teward> if this goes into deployment however it would help having a nice document posted somewhere about the change as people might start headscratching when things change majorly.
[14:18] <teward> but that's a given :(
[14:18] <teward> :) *
[14:24] <cjwatson> Will look at it next week, thanks (public holiday here today)
[14:28] <teward> yep just putting it on the radar :)
[14:28] <teward> (wish I had a public holiday today... I don't want to be doing Work from Home today - could use an extra 4 hours sleep xD)
[18:45] <teward> cjwatson: do you know where Merge Proposal emails are constructed in the codebase?
[18:45] <teward> i see some in docs but I need to find the constructor (they're also DMARC-noncompliant)
[18:48] <cjwatson> teward: Somewhere in lib/lp/code/mail/
[18:51] <teward> thanks i'll dig.  that's going to be the next thing I start poking at.  What's the testsuite for just merge proposals?  (or merges) - lp.code ?
[18:51] <teward> (instead of lp.bugs like before)
[18:53] <cjwatson> Yes.  The -t option takes a regex match applied to test case IDs - you can be more specific if you like, and I'd probably be inclined to do something like -t merge.\*proposal -t code.\*review
[18:53] <cjwatson> Should be rather faster
[18:54] <teward> +1
[18:55] <teward> i'll dig there next, but i'm thinking a more phased approach to DMARC implementation - make sure there's no regressions in one part before implementing the rest.  Unless we want to one-and-done it all but that has more regression risks.
[18:55] <cjwatson> We try to name tests at least somewhat regularly so that things like that have a better chance of matching
[18:55] <cjwatson> Doing this in pieces is absolutely correct and appropriate
[18:55] <cjwatson> No need to rush it
[18:58] <teward> yep
[18:58] <teward> i'm bored again though right now so :P  Need something to pass time :P