[00:08] 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] (Chrome exploded when I tried to paste it into a pastebin) [00:09] failed errors are towards the end [00:10] teward: That looks fine [00:10] - Subject: [Bug ...] subject [00:10] ? ^^^ [00:10] + Subject: [Bug 18] subject [00:10] ? ^^ [00:10] Bug #18: RFE: I'd like to be CC:d automatically to bugs I report [00:10] teward: Yep, as noted, this is doctest output being surprising to the uninitiated [00:10] That's the full diff, not necessarily the diff that caused the failure. [00:10] It doesn't include whitespace normalisation, and it doesn't include ellipsis. [00:11] 👍 thanks cjwatson [00:11] the only other failure cases are the From addresses [00:11] which *are* what changed in my diff [00:11] You should find that if you fix just the From: bits then the rest will match [00:11] that's the tricky part in *that* doctest [00:12] unless it's always using 18 [00:12] <...@bugs.launchpad.net> [00:12] ack [00:12] That's what ellipses are for [00:12] those doctest errors are confuzling to me :) [00:12] Sometimes I wonder if that was one of the design goals [00:12] They can be remarkably obtuse. [00:16] that i don't doubt xD [00:16] but ODDLY no other bug notification tests errored heh [00:16] That is a little remarkable to me, but OK [00:18] 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] 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] Well, it evidently is being tested in that doctest [00:20] yep [00:20] Unexpectedly light coverage, but we are actually testing it [00:21] Albeit slightly in passing [00:23] Though, I don't understand how your changes didn't cause bugnotification-email.txt to fail [00:23] it did [00:23] i fixed those [00:23] Oh, I see [00:23] those were the straightforward fixes [00:23] OK, good [00:24] That's where get_bugmail_from_address is unit-tested (albeit in a ridiculous format, but they're conceptually unit tests) [00:24] yeah and those (as expected) failed [00:25] 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] And hopefully the surrounding prose commentary [00:25] wanted to get the tests passing before I rewrite the commentary :) [14:13] 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] tests pass here, which was... surprising heh. [14:18] 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] but that's a given :( [14:18] :) * [14:24] Will look at it next week, thanks (public holiday here today) [14:28] yep just putting it on the radar :) [14:28] (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] cjwatson: do you know where Merge Proposal emails are constructed in the codebase? [18:45] i see some in docs but I need to find the constructor (they're also DMARC-noncompliant) [18:48] teward: Somewhere in lib/lp/code/mail/ [18:51] 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] (instead of lp.bugs like before) [18:53] 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] Should be rather faster [18:54] +1 [18:55] 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] We try to name tests at least somewhat regularly so that things like that have a better chance of matching [18:55] Doing this in pieces is absolutely correct and appropriate [18:55] No need to rush it [18:58] yep [18:58] i'm bored again though right now so :P Need something to pass time :P