| abentley | me | 14:59 |
|---|---|---|
| bac | #startmeeting | 15:00 |
| MootBot | Meeting started at 09:00. The chair is bac. | 15:00 |
| MootBot | Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] | 15:00 |
| bac | hi, whose here? | 15:00 |
| bigjools | me | 15:00 |
| sinzui | me | 15:00 |
| gmb | me | 15:00 |
| jtv | me | 15:00 |
| bigjools | who's here, too :) | 15:00 |
| bac | :) | 15:00 |
| deryck | me | 15:00 |
| jtv | nice cascade we had there | 15:00 |
| mars | me | 15:00 |
| EdwinGrubbs | me | 15:00 |
| bac | bigjools: no, i found a very nice "here" and was wondering who'd claim it | 15:00 |
| gary_poster | me | 15:01 |
| henninge | me | 15:01 |
| gary_poster | I'll take it | 15:01 |
| noodles775 | me | 15:01 |
| gary_poster | well, it's probably not mine | 15:01 |
| bac | well, let's get started then | 15:02 |
| adeuring | me | 15:02 |
| bac | not much on the agenda so we should zip right through | 15:02 |
| bac | * Roll call | 15:02 |
| bac | * Agenda | 15:02 |
| bac | * Outstanding actions | 15:02 |
| bac | * New topics | 15:02 |
| bac | * 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 |
| bac | * Peanut gallery | 15:02 |
| bac | * jml(?) to set a policy on what can live in lib/lp, lib/services, and lib/coop | 15:02 |
| bac | 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 |
| bac | my bad | 15:03 |
| flacoste | me | 15:03 |
| bac | sinzui: how goes your lint replacement? good, no? | 15:03 |
| sinzui | lint is queued for landing | 15:03 |
| bac | \o/ | 15:03 |
| sinzui | I will send an email announcing this today | 15:04 |
| sinzui | + instructions about embedding it in your editor | 15:04 |
| bac | very nice, thank you. | 15:04 |
| bac | as to new items we only have one | 15:05 |
| bac | [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 |
| MootBot | 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 |
| bac | bigjools: the floor is yours. | 15:05 |
| bigjools | well first, I was in a bad mood when I added that topic title, but I think you get the gist | 15:05 |
| bigjools | the commit messages should follow the style guide | 15:06 |
| bigjools | EOT | 15:06 |
| sinzui | okay | 15:06 |
| * sinzui stops work to change editor | 15:06 | |
| bac | any other thoughts? | 15:06 |
| henninge | Is that spec up to date? | 15:07 |
| henninge | 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 |
| ubot5 | 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 |
| ubot5 | 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:07 |
| henninge | I never do that, I thought that's what [bug=xxx] is for | 15:08 |
| jtv | same here | 15:08 |
| bac | henninge: you are correct | 15:08 |
| bac | the guide is out of date | 15:08 |
| abentley | henninge, agreed. ec2 land and lp-land use your syntax. | 15:08 |
| sinzui | bigjools, fixed | 15:08 |
| bac | i'll fix it this week | 15:08 |
| bac | thanks sinzui | 15:09 |
| bigjools | sinzui: sorry, I didn't meant to pick on you specifically. I've just seen some poor commit messages lately. | 15:09 |
| bac | [action] bac to update https://dev.launchpad.net/PQMCommitMessages | 15:09 |
| MootBot | ACTION received: bac to update https://dev.launchpad.net/PQMCommitMessages | 15:09 |
| sinzui | 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 |
| gary_poster | we're working on that and will have a report at epic | 15:10 |
| bigjools | totally | 15:11 |
| bigjools | oh cool | 15:11 |
| abentley | 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 |
| bac | [topic] other stuff | 15:11 |
| MootBot | New Topic: other stuff | 15:11 |
| bac | abentley: +1 | 15:11 |
| bigjools | +1 | 15:11 |
| bac | anything else to discuss today | 15:11 |
| bac | ? | 15:11 |
| gary_poster | benji is a new launchpadder | 15:12 |
| gary_poster | he's doing new launchpaddy things now | 15:12 |
| gary_poster | but he will be asking for reviews | 15:12 |
| bac | great gary_poster | 15:12 |
| gary_poster | and will eventually want to go through the mentat process | 15:12 |
| henninge | I had a few issues come up in a recent review, let me pick one. | 15:12 |
| bac | everyone should make an effort to say hi to benji this week | 15:12 |
| gary_poster | +1, was hoping I could get him at this meeting | 15:13 |
| henninge | Anybody ever heard a guideline "one assert per test" in functional test cases? | 15:13 |
| bac | i have not | 15:14 |
| abentley | henninge, no. | 15:14 |
| henninge | The problem with multiple asserts is that the test will abort and the remaing asserts will not be executed. | 15:14 |
| === Ursinha is now known as Ursinha-brb | ||
| abentley | henninge, the problem with one assert per test is that set-up time increases. | 15:14 |
| abentley | henninge, not to mention much more finger-typing. | 15:14 |
| henninge | abentley: it's either that or smarter asserts. | 15:15 |
| bac | and probably a less readable test | 15:15 |
| gmb | 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 |
| abentley | henninge, what's the disadvantage of having the test abort and not executing the remaining asserts? | 15:15 |
| henninge | hang on | 15:16 |
| henninge | Code like this: | 15:16 |
| henninge | > + self.assertEquals(1, len(result_set)) | 15:16 |
| henninge | > >>> + self.assertEquals(self.ppa, result_set[0].archive) | 15:16 |
| henninge | > >>> + self.assertEquals(self.cell_proc, result_set[0].processorfamily) | 15:16 |
| henninge | >> | 15:16 |
| henninge | You can test all three conditions at the same time. You can see how many rows | 15:17 |
| henninge | were returned instead and what their values are. Much more useful. | 15:17 |
| henninge | The problem with multiple assert statements in one test method is that not all | 15:17 |
| henninge | will be executed if one fails and you are missing valuable information that | 15:17 |
| henninge | you can only gain by changing the test code and running the test again. | 15:17 |
| henninge | For example, if result_set contained 2 instead of the expected 1 elements, | 15:17 |
| abentley | And how often would the test have failed anyway, because the remaining asserts assume the first assert passed? | 15:17 |
| henninge | you'd want to know what the two contain but this test would not even tell you | 15:17 |
| henninge | what the first contains because the asserts don't get called. | 15:17 |
| bigjools | abentley: point | 15:17 |
| abentley | henninge, so in your example, what if len==0? | 15:18 |
| henninge | abentley: sure, we should just make sure that is really the case. The dependency i mean. | 15:18 |
| henninge | The example is missing my proposed solution which would be comparing lists. | 15:18 |
| abentley | 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 |
| henninge | if len == 0, the list would be empty. | 15:18 |
| adeuring | 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 |
| abentley | henninge, right, and the rest of the asserts will raise IndexError. Which seems like meaningless exceptions to me. | 15:19 |
| henninge | My suggestion: | 15:20 |
| henninge | result = [(row.archive, row.processorfamily) for row in result_set] | 15:20 |
| henninge | self.assertEqual([(self.ppa, self.cell_proc)], result) | 15:20 |
| henninge | From the review^ | 15:20 |
| henninge | abentley: there are no other asserts to fail. | 15:20 |
| abentley | I thought self.assertEquals(self.ppa, result_set[0].archive) is an assert that would fail | 15:21 |
| henninge | abentley: with len== 0? yes | 15:22 |
| bac | 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 |
| abentley | henninge, If we followed a "one assert per test" rule, we might have the same test three times, with the different asserts. | 15:22 |
| henninge | I was not proposing a rule | 15:22 |
| bac | ok, i see you said 'guideline'. | 15:23 |
| henninge | just to have people think about smart asserts. | 15:23 |
| abentley | henninge, sorry, *guideline* | 15:23 |
| abentley | henninge, I like your solution. | 15:23 |
| abentley | henninge, but if you hadn't thought of it, I don't think applying the guideline would make sense. | 15:23 |
| sinzui | 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:23 |
| abentley | henninge, because it would multiply the number of tests and failures in ways that are not useful. | 15:24 |
| henninge | just make sure that functional tests suffer not from similar dependency problems like doctest might do. | 15:24 |
| sinzui | 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 |
| sinzui | We had several tests that did that a few years ago | 15:24 |
| henninge | sinzui: jtv and I just looked at one this morning ... ;) | 15:25 |
| henninge | anyway, I have made my point. | 15:25 |
| jtv | henninge: I think that one did go through multiple state changes | 15:25 |
| jtv | ...with separate assertions | 15:25 |
| abentley | sinzui, I'm very much in favour of "test only one thing" as a guideline. | 15:25 |
| sinzui | abentley, I have read your tests, and I know you are | 15:26 |
| henninge | 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 |
| bigjools | the exception to that is where the setup is very expensive | 15:26 |
| henninge | jtv: right. | 15:26 |
| abentley | bigjools, though often those are integration tests, so what you're testing is the integration, not the units. | 15:27 |
| bigjools | possibly | 15:28 |
| abentley | bigjools, YMMV | 15:28 |
| bac | 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 |
| jtv | 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 |
| bigjools | abentley: especially in Soyuz :) | 15:28 |
| bac | any other topics? | 15:29 |
| bac | 3 | 15:29 |
| bac | 2 | 15:29 |
| bac | 1 | 15:29 |
| bac | thanks everyone | 15:29 |
| bigjools | thanks | 15:29 |
| bac | #endmeeting | 15:29 |
| MootBot | Meeting finished at 09:29. | 15:29 |
| bac | go read http://elliotmurphy.com/2010/07/06/teambuilding-and-culture-in-a-distributed-workplace/ for a canonical warm-n-fuzzy | 15:30 |
| bigjools | bac: that blog post is awesome, I read it yesterday and had a huge smile on my face | 15:47 |
| === 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 | ||
| bac | thumper, rockstar: reviewer meeting? | 22:33 |
| thumper | bac: rockstar just went for a walk | 22:34 |
| bac | thumper: ok. we didn't get up to much in the earlier meeting. | 22:34 |
| bac | thumper: main item was bigjools requesting that we follow the PQM submit message guidelines | 22:34 |
| bac | thumper: by that he meant cut out extraneous stuff like what music is playing, etc. | 22:35 |
| bac | everyone agreed | 22:35 |
| bac | henninge brought up the topic of a single assert per test case in functional tests which led to a pretty long discussion | 22:35 |
| bac | 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 |
| thumper | hmm | 22:36 |
| bac | 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 |
| thumper | personally I don't care about the commit message thing | 22:36 |
| thumper | I'm happy to go either way | 22:37 |
| bac | henning agreed it should be a guildeline | 22:37 |
| bac | me too | 22:37 |
| thumper | I don't like the single assert in a test thing | 22:37 |
| thumper | if you want to see the others, I think there is a flag | 22:37 |
| bac | really? | 22:37 |
| * thumper thinks | 22:38 | |
| thumper | maybe? | 22:38 |
| bac | dunno | 22:38 |
| thumper | lifeless would probably know | 22:38 |
| thumper | if there isn't one, we could add one | 22:38 |
| thumper | it isn't rocket science | 22:38 |
| bac | i'll look into it | 22:38 |
| thumper | ta | 22:38 |
| thumper | WRT unit tests | 22:39 |
| bac | often if an earlier assert dies then those following don't have much chance | 22:39 |
| bigjools | the commit message thing was more about writing something that actually explained the fix properly | 22:39 |
| thumper | I would say that the test should be short | 22:39 |
| thumper | and primarily test one thing | 22:39 |
| thumper | but that one thing may well need multiple asserts | 22:39 |
| bac | thumper: yes, you're channeling the gist of our discussion | 22:40 |
| thumper | ok | 22:40 |
| thumper | we're on the the same page then | 22:40 |
| bac | yeppers | 22:40 |
| === salgado is now known as salgado-afk | ||
| bac | that was it. | 22:41 |
| bac | thumper: you have anything to pass on? | 22:41 |
| thumper | ok, I guess we are done then | 22:41 |
| bac | right. have a good day. | 22:41 |
| thumper | not, not really | 22:41 |
| thumper | see you next week | 22:41 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!