[14:59] me [15:00] #startmeeting [15:00] Meeting started at 09:00. The chair is bac. [15:00] Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] [15:00] hi, whose here? [15:00] me [15:00] me [15:00] me [15:00] me [15:00] who's here, too :) [15:00] :) [15:00] me [15:00] nice cascade we had there [15:00] me [15:00] me [15:00] bigjools: no, i found a very nice "here" and was wondering who'd claim it [15:01] me [15:01] me [15:01] I'll take it [15:01] me [15:01] well, it's probably not mine [15:02] well, let's get started then [15:02] me [15:02] not much on the agenda so we should zip right through [15:02] * Roll call [15:02] * Agenda [15:02] * Outstanding actions [15:02] * New topics [15:02] * PQM commit messages: I don't f***ing care what you're currently listening to. See also https://dev.launchpad.net/PQMCommitMessages [bigjools] [15:02] * Peanut gallery [15:02] * jml(?) to set a policy on what can live in lib/lp, lib/services, and lib/coop [15:03] since bjornt left us, this item was assigned to jml...but i forgot to tell him about it, so clearly nothing was done. [15:03] my bad [15:03] me [15:03] sinzui: how goes your lint replacement? good, no? [15:03] lint is queued for landing [15:03] \o/ [15:04] I will send an email announcing this today [15:04] + instructions about embedding it in your editor [15:04] very nice, thank you. [15:05] as to new items we only have one [15:05] [topic] PQM commit messages: I don't f***ing care what you're currently listening to. See also https://dev.launchpad.net/PQMCommitMessages [bigjools] [15:05] New Topic: PQM commit messages: I don't f***ing care what you're currently listening to. See also https://dev.launchpad.net/PQMCommitMessages [bigjools] [15:05] bigjools: the floor is yours. [15:05] well first, I was in a bad mood when I added that topic title, but I think you get the gist [15:06] the commit messages should follow the style guide [15:06] EOT [15:06] okay [15:06] * sinzui stops work to change editor [15:06] any other thoughts? [15:07] Is that spec up to date? [15:07] e.g. Include bugs fixed in this landing. For each bug, include the bug number and the bug summary next to it. e.g. "Fixes: bug 12345: Mozilla Firefox lacks SVG support; bug 54321: Lack of spacing in new account form." [15:07] Launchpad bug 12345 in isdnutils (Ubuntu) "isdn does not work, fritz avm (pnp?) (affected: 0, heat: 3)" [Medium,Fix released] https://launchpad.net/bugs/12345 [15:07] Launchpad bug 54321 in zope-archetypes (Ubuntu) "Please sync zope-archetypes 1.3.9-2 from Debian unstable (affected: 0, heat: 2)" [Undecided,Fix released] https://launchpad.net/bugs/54321 [15:08] I never do that, I thought that's what [bug=xxx] is for [15:08] same here [15:08] henninge: you are correct [15:08] the guide is out of date [15:08] henninge, agreed. ec2 land and lp-land use your syntax. [15:08] bigjools, fixed [15:08] i'll fix it this week [15:09] thanks sinzui [15:09] sinzui: sorry, I didn't meant to pick on you specifically. I've just seen some poor commit messages lately. [15:09] [action] bac to update https://dev.launchpad.net/PQMCommitMessages [15:09] ACTION received: bac to update https://dev.launchpad.net/PQMCommitMessages [15:10] bigjools, I agree about the poor commit messages...I brought the issue up with Urshina when I was RM. There were 30 bugs un QAed and I could not say what they changed. [15:10] we're working on that and will have a report at epic [15:11] totally [15:11] oh cool [15:11] merge proposals have a slot for the commit message. Maybe we should encourage people to fill it out as part of the review. [15:11] [topic] other stuff [15:11] New Topic: other stuff [15:11] abentley: +1 [15:11] +1 [15:11] anything else to discuss today [15:11] ? [15:12] benji is a new launchpadder [15:12] he's doing new launchpaddy things now [15:12] but he will be asking for reviews [15:12] great gary_poster [15:12] and will eventually want to go through the mentat process [15:12] I had a few issues come up in a recent review, let me pick one. [15:12] everyone should make an effort to say hi to benji this week [15:13] +1, was hoping I could get him at this meeting [15:13] Anybody ever heard a guideline "one assert per test" in functional test cases? [15:14] i have not [15:14] henninge, no. [15:14] The problem with multiple asserts is that the test will abort and the remaing asserts will not be executed. === Ursinha is now known as Ursinha-brb [15:14] henninge, the problem with one assert per test is that set-up time increases. [15:14] henninge, not to mention much more finger-typing. [15:15] abentley: it's either that or smarter asserts. [15:15] and probably a less readable test [15:15] henninge, I'm not clear on how the abort is really a problem. For a start, it means you get to fix one test failure at a time rather than having to dig through many. [15:15] henninge, what's the disadvantage of having the test abort and not executing the remaining asserts? [15:16] hang on [15:16] Code like this: [15:16] > + self.assertEquals(1, len(result_set)) [15:16] > >>> + self.assertEquals(self.ppa, result_set[0].archive) [15:16] > >>> + self.assertEquals(self.cell_proc, result_set[0].processorfamily) [15:16] >> [15:17] You can test all three conditions at the same time. You can see how many rows [15:17] were returned instead and what their values are. Much more useful. [15:17] The problem with multiple assert statements in one test method is that not all [15:17] will be executed if one fails and you are missing valuable information that [15:17] you can only gain by changing the test code and running the test again. [15:17] For example, if result_set contained 2 instead of the expected 1 elements, [15:17] And how often would the test have failed anyway, because the remaining asserts assume the first assert passed? [15:17] you'd want to know what the two contain but this test would not even tell you [15:17] what the first contains because the asserts don't get called. [15:17] abentley: point [15:18] henninge, so in your example, what if len==0? [15:18] abentley: sure, we should just make sure that is really the case. The dependency i mean. [15:18] The example is missing my proposed solution which would be comparing lists. [15:18] henninge, and since you're getting results you're not expecting, how do you know that result_set[0] is testing the right thing? [15:18] if len == 0, the list would be empty. [15:19] you can have a test for "method X changes A into B". The test may need something like "assert(A); do_something(); assert(B)". "assert(A)" may not be strictly needed, but it can help to ensure that you have a proper test setup. [15:19] henninge, right, and the rest of the asserts will raise IndexError. Which seems like meaningless exceptions to me. [15:20] My suggestion: [15:20] result = [(row.archive, row.processorfamily) for row in result_set] [15:20] self.assertEqual([(self.ppa, self.cell_proc)], result) [15:20] From the review^ [15:20] abentley: there are no other asserts to fail. [15:21] I thought self.assertEquals(self.ppa, result_set[0].archive) is an assert that would fail [15:22] abentley: with len== 0? yes [15:22] henninge: i like your suggestion in this instance. i don't think we'd want to generalize to a restriction of one assert per test case as other situations may not lend themselves so nicely. [15:22] henninge, If we followed a "one assert per test" rule, we might have the same test three times, with the different asserts. [15:22] I was not proposing a rule [15:23] ok, i see you said 'guideline'. [15:23] just to have people think about smart asserts. [15:23] henninge, sorry, *guideline* [15:23] henninge, I like your solution. [15:23] henninge, but if you hadn't thought of it, I don't think applying the guideline would make sense. [15:23] I recall the reason for "one assert per test" was not 10 asserts in a test, but that the test was actually several tests. [15:24] henninge, because it would multiply the number of tests and failures in ways that are not useful. [15:24] just make sure that functional tests suffer not from similar dependency problems like doctest might do. [15:24] When an object is transitioned to a new state, it may be legitimate to very 3 aspects of the state, but not to then do another state change [15:24] We had several tests that did that a few years ago [15:25] sinzui: jtv and I just looked at one this morning ... ;) [15:25] anyway, I have made my point. [15:25] henninge: I think that one did go through multiple state changes [15:25] ...with separate assertions [15:25] sinzui, I'm very much in favour of "test only one thing" as a guideline. [15:26] abentley, I have read your tests, and I know you are [15:26] I had it mentioned to me in a review when I first started and just wanted to check if that is the general opinion. [15:26] the exception to that is where the setup is very expensive [15:26] jtv: right. [15:27] bigjools, though often those are integration tests, so what you're testing is the integration, not the units. [15:28] possibly [15:28] bigjools, YMMV [15:28] henninge: thanks for bringing this up. i think reducing the asserts, as you did in your example, is a good idea where practical. [15:28] Another form of testing-too-many-things is where the test sets up a large set of data, and then either performs the operation once and checks all the outputs; or several times with separate checks but without a clear reason why the output should be as expected. [15:28] abentley: especially in Soyuz :) [15:29] any other topics? [15:29] 3 [15:29] 2 [15:29] 1 [15:29] thanks everyone [15:29] thanks [15:29] #endmeeting [15:29] Meeting finished at 09:29. [15:30] go read http://elliotmurphy.com/2010/07/06/teambuilding-and-culture-in-a-distributed-workplace/ for a canonical warm-n-fuzzy [15:47] bac: that blog post is awesome, I read it yesterday and had a huge smile on my face === leonardr is now known as leonardr-afk === salgado is now known as salgado-lunch === gary_poster is now known as gary-lunch === salgado-lunch is now known as salgado === Ursinha-brb is now known as Ursinha === gary-lunch is now known as gary_poster === EdwinGrubbs is now known as Edwin-lunch === Edwin-lunch is now known as EdwinGrubbs [22:33] thumper, rockstar: reviewer meeting? [22:34] bac: rockstar just went for a walk [22:34] thumper: ok. we didn't get up to much in the earlier meeting. [22:34] thumper: main item was bigjools requesting that we follow the PQM submit message guidelines [22:35] thumper: by that he meant cut out extraneous stuff like what music is playing, etc. [22:35] everyone agreed [22:35] henninge brought up the topic of a single assert per test case in functional tests which led to a pretty long discussion [22:36] he was concerned that after the first assertion fails the test stops and you don't see the results of the later assertions [22:36] hmm [22:36] we agreed that in the cases where you can do something clever to avoid multiple assertions then that was ok but we didn't want to codify such a thing [22:36] personally I don't care about the commit message thing [22:37] I'm happy to go either way [22:37] henning agreed it should be a guildeline [22:37] me too [22:37] I don't like the single assert in a test thing [22:37] if you want to see the others, I think there is a flag [22:37] really? [22:38] * thumper thinks [22:38] maybe? [22:38] dunno [22:38] lifeless would probably know [22:38] if there isn't one, we could add one [22:38] it isn't rocket science [22:38] i'll look into it [22:38] ta [22:39] WRT unit tests [22:39] often if an earlier assert dies then those following don't have much chance [22:39] the commit message thing was more about writing something that actually explained the fix properly [22:39] I would say that the test should be short [22:39] and primarily test one thing [22:39] but that one thing may well need multiple asserts [22:40] thumper: yes, you're channeling the gist of our discussion [22:40] ok [22:40] we're on the the same page then [22:40] yeppers === salgado is now known as salgado-afk [22:41] that was it. [22:41] thumper: you have anything to pass on? [22:41] ok, I guess we are done then [22:41] right. have a good day. [22:41] not, not really [22:41] see you next week