=== salgado is now known as salgado-afk === mrevell is now known as mrevell-lunch === mrevell-lunch is now known as mrevell [14:00] #startmeeting [14:00] Meeting started at 09:00. The chair is barry. [14:00] Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] [14:00] hello everyone and welcome to this week's ameu reviewer's meeting. who's here today? [14:00] me [14:00] me [14:00] ma [14:00] ma [14:00] me [14:00] :-) [14:00] where is my e [14:00] me [14:00] moi [14:00] me [14:01] noodles775, you need to step one country to the right [14:01] mars: ah, wrong one... :/ [14:01] bac: ping [14:01] argh [14:01] me [14:01] bigjools, cprov, danilo_ ping [14:02] intellectronica: ping [14:02] me [14:02] rockstar: ping [14:02] salgado: ping [14:02] me! [14:02] today's a light day, so let's start off with the fun stuff [14:02] me [14:02] me [14:03] [TOPIC] agenda [14:03] New Topic: agenda [14:03] * Roll call [14:03] * Peanut gallery (anything not on the agenda) [14:03] * Mentoring update [14:03] * Action items [14:03] [TOPIC] * Peanut gallery (anything not on the agenda) [14:03] New Topic: * Peanut gallery (anything not on the agenda) [14:03] me === danilo_ is now known as danilos [14:03] does anybody have any reviewy type stuff they want to bring up that's not on the agenda? [14:03] me [14:04] sinzui: the floor is yours [14:04] bac discovered that there were a few places in our templates where we were making insane links. [14:04] Bad [14:04] [14:04] becomes [14:04] text [14:04] Good [14:04] [14:04] becomes [14:04] text [14:04] When reviewing tal, please verify anything that uses `structure` works in the containing elements. [14:05] that's wacky [14:05] sinzui, would html-tidy catch that? [14:05] mars yes [14:06] after rendering, yes, but not before rendering (in our templates) [14:06] yep, of course [14:06] mars: any HTML validator can see that is not permitted [14:06] not that we have any good or automated way to run said validator... [14:06] i have a branch that attempt to ferret them all out. even our 'featured projects' listing on the front page had the problem. [14:07] good catch sinzui and bac [14:08] bac: by visual inspection or through some tool? [14:08] well, when sinzui says "bac discovered" he means "bac repeated the problem flagrantly" such that sinzui discovered it. [14:08] what if we made the doctest runner do HTML validation on page open? [14:08] :-) [14:08] barry: just grep and examing them all [14:09] barry 'content="structure .*link' catches most of them [14:09] mars: interesting idea, but i'm affraid it would slow things down alot [14:09] of course we can write tal on multiple lines [14:10] [blue sky] we could have a nightly buildbot for what mars suggests [14:10] barry, we could bind it to 'make check' [14:10] mars: too slow, as is updating find_main_content [14:10] barry, since we'll have to do so for windmill tests anyway [14:10] which are even slower [14:11] Our link checker could take on the extra work. or... [14:11] mars: yeah [14:11] sinzui, yes, that's another good hook point [14:11] >>> view = create_initialized_view(person, '+index', principle=person) [14:11] >>> is_valid_html(view.render()) [14:11] True [14:11] sinzui, but the link-checker isn't under developer control [14:11] is it? [14:12] gary_poster: +1 -- a periodic scan should be enough to trap infrequent offenders [14:12] at least I could do ./test.py --validate-markup or something [14:13] sinzui: i think i like that approach best. cons: it's up to the dev to add the check, pros: you can decide to check only when you think it's necessary [14:14] does someone want to take this on as an action item? [14:14] i think the buldbot story is better [14:14] here [14:14] cons: it slows down our tests [14:14] doesn't slow down the test suite [14:14] it's not a big problem [14:15] i mean this has been broken for long and didn't cause any problems [14:15] invalid markup is ugly, but not a show-stoppers [14:15] thanks to the way browsers are implemented [14:15] we don't need to solve it here, but i'd like someone to own it and offer some options [14:15] flacoste, does our build process have provisions or the concept of warnings? [14:15] it sounds like a foundations issue [14:15] but it would be a very low priority to me [14:16] so no point putting it down as an actino [14:16] flacoste: okay [14:16] thanks. any other topics not on the agenda? [14:18] [TOPIC] mentoring update [14:18] New Topic: mentoring update [14:18] any mentor or mentat updates today? [14:18] me === ursula_ is now known as ghost === ghost is now known as Ursinha [14:19] rockstar: you don't have a mentoring update, right? [14:19] No, I'm just screwed by DST again. [14:19] [TOPIC] action items [14:19] New Topic: action items [14:20] * gary to add `getStore()` as an alias for `_get_store()` [14:20] gary_poster: ^^ [14:21] barry: i still suck at mine, but do keep it open, i plan to suck less [14:21] see, i'm pro-active :-) [14:21] flacoste: that's a start! :) [14:21] * gary will check to see if there's a bug open for adding a hook to `bzr send`, and submit one if there isn't [14:21] gary_poster: ^^ [14:22] * bigjools to take crack at helper functions for backpatching schemas to avoid circular imports [14:22] * barry to add `field_id` to coding guideline [14:22] * barry to update guidelines to never call `_foo()` methods from outside a class [14:22] * barry to add `pretty()` functions to reviewers docs [14:22] i actually do NOT suck today! i did all three of these :) [14:23] goddamn it and the time changes [14:23] * bigjools sucks at helper functions AND timekeeping [14:24] no worries [14:24] anyway, that's all i have today. anything else going on? [14:24] 5 [14:25] I have something [14:25] rockstar: go ahead [14:25] It looks like sinzui already quoted to code above... [14:25] I was reviewing a branch of sinzui's where he was using a doctest to test a view's API. [14:26] Now, I understand that our team has an unusual fetish for doctests, but I think that if we're doing something where we can use unittests, we should use them. [14:27] hmm [14:27] * mars sees the testing debate from the ML creeping in... [14:27] that's a controversial topic [14:27] rockstar: what are the arguments? [14:27] i might make the counter argument :) [14:28] yeah [14:28] Shall we wall read the 2008 June, July debate. [14:28] doctests are good for telling a user story, but if we're just testing the methods of a view, why not use what unittests for what they were made for? [14:28] I believe thumper wanted a paintball battle to decide the matter [14:28] lol [14:28] sinzui: M-x paintball [14:28] rockstar: we have usually refrained from enforcing a policy there [14:28] let developers choose which technology they prefer here [14:29] flacoste: right. i think that still works as a policy [14:29] flacoste, and I think that leads to a mess. [14:29] i think that unless we lose coverage because of that, we should allow teams to choose what format they prefer [14:29] for individuals to choose is already a bit extreme [14:29] If I change a view, I can't easily discern where the hell the test is that I need to edit. [14:30] Is it it doc? Is it in browser/tests? [14:30] I'm agnostic on the issue. Since all the registry view tests are in doctest, I wan to add to them instead of storing two kinds of tests in two areas [14:30] rockstar: conversely without a doctest if i'm trying to learn how a view works, it's much more difficult to discern that from its docstrings and unittest [14:30] barry, why can't you look at the unittest? You'll be doing essentially the same thing. [14:31] rockstar: doctests do not normally explain WHY the view does soemthing [14:31] rockstar: unittests lack the narrative that sets the context of the api [14:31] right [14:31] rockstar finding the tests is different than the format [14:31] unittests are superior ways of testing, I'm pretty tired of having a doctest halt when one of its tests fails [14:31] the "ties it all together" bits [14:31] it should be browser/tests [14:31] whatever the format [14:32] unless it's an API description [14:32] and even then [14:32] i'm starting the Launchpad-tree-apocalypse today [14:32] /o\ [14:32] * sinzui hand flacoste the fuel and a torch [14:32] I think mixing and matching tests is going to bite us one of these days. [14:32] that will makes things clearer for finding stuff and tests [14:32] for answers [14:32] and registry will they their apocalyspe next week [14:33] gary_poster, zope has a lot of experience in that area [14:33] * sinzui is bringing marshmallows. [14:33] anything to share here? [14:33] Anytime you say "In this folder, you might find unittests or doctests" it makes me think that one or the other is being misused. [14:34] flacoste: sorry, having connectivity issues and buildbot issues simultaneously [14:34] rockstar: personally, i'd like to see doctests in a 'docs' directory [14:34] sanitizing our testing infrastructure is definitely on the list of monsters to pitchfork [14:35] barry, personally, I'd like to see the doctests use testbrowser to tell a user story, or unittests to test a unit. [14:35] barry: well, if it's docs, it belongs in a docs directory [14:36] but many doctest are not docs [14:36] rockstar: what about documentation for internal components and subsystems? [14:36] rockstar: do you never use the doctests to learn about functionality? i often do, and i think that if we don't have doctests, we won't have documentation at all [14:36] flacoste: yes, that's a problem :) [14:36] docs is about API description [14:36] well, it's not [14:36] flacoste, isn't that the benefit of doctests? They are docs AND they are tests? [14:36] rockstar: model/components need to be doctests because their are the develop documentation. [14:36] well [14:36] i don't think a view is an API [14:36] interesting to document [14:36] most of them aren't [14:36] a few might be [14:36] but they are the exception [14:37] flacoste, a view has an API. [14:37] well, not really [14:37] the only API is __call__ [14:37] which renders the view [14:37] that's it [14:37] flacoste, and the properties [14:37] no [14:37] intellectronica, I look at unittests all the time to learn about functionality. [14:37] well, maybe [14:37] rockstar: i'm thinking about the multistep view i just refactored. i think documentation for it is essential so that others can understand how to reuse it. [14:38] i would argue that the properties are internal implementation details [14:38] My thoughts on this: (1) doctest for unittest purpose is emacs vs. vi IMO/IME. I think that using doctest for unittest purposes works well. Other people disagree. I tend to find that people think about these things differently. I can acknowledge the points people make (like bigjools, IRT doctests failing at the beginning) but I prefer doctests. [14:38] (2) I would much prefer it if our tests were closer to the pertinent code [14:38] i agree with both points [14:38] that appears to be (part of) the topic I lost while I was dealing with various issues [14:38] gary_poster, +1 on (2) [14:38] and my apocalyspe branch will improve 2 [14:39] flacoste, rockstar, maybe reorganize the tree, and wait and see? [14:39] * flacoste can't spell apocalypse [14:39] you can run more experiments, say with the view tests, after [14:39] flacoste: maybe that should be apocalisp? [14:39] lol [14:39] :-) [14:39] a smaller sandbox will make experimenting on a per-team basis easier [14:39] mars, I don't think that's the answer. [14:39] mars: I think teams have already made their decision [14:40] I see doctests primarily as documentation that carry examples, it's easy to abuse them to be full-blown tests [14:40] mars: yes, and it will make things much more discoverable and comprehensible [14:40] The per-team thing just makes things messier. That's why it's good to have cross-team-reviews [14:40] I think that teams that will not release their code are using unittests. [14:40] sinzui, :( === gary_poster_ is now known as gary_poster [14:41] rockstar: well, not really per-team. what i meant, at least, is that the division should be thematic, not personal [14:41] i think it's just too controversial to mandate at this point. i agree with gary that it feels like mandating vi or emacs [14:41] intellectronica, depending on the "theme" I agree with you. [14:41] perhaps emacs users could use doctests... [14:41] :-) [14:42] i personally think that both doctests and unittests have their place, both have positives... and negatives [14:42] barry, I don't see why. I think they serve two different purposes. It's like mandating motor oil v. beer. [14:42] rockstar, yuck [14:42] rockstar: right, but...other people disagree with you :-) [14:42] that's the point [14:42] rockstar: is that: unittests are greasy and icky but good for your motor, while doctests are fun, festive and easy to consume? [14:42] I think the crucial point that rockstar is making is that to test a class of problems, we should use one kind of test so that we know where and how to test it. [14:43] barry: lol [14:43] sinzui, yes! Thank you! [14:43] My concern is that if I switch to unittest, I then have lots of tests in doctest [14:43] insert old in the above sentece somewhere [14:43] do we agree that doctests are not tests, they're documentation? [14:44] no [14:44] bigjools, no [14:44] not as we use them atm. [14:44] no [14:44] no [14:44] doctests are tests [14:44] because I remember a mail thread from 2 years ago [14:44] it's not either or. doctests are documentation /first/ but they are tests too [14:44] I think doctests are great at telling a story about a user experience, and asserting that the story actually works with code. [14:44] that said they are not tests [14:44] barry: i would even argue that some doctests are not even doc first and that's fine too [14:44] and that writing words like "should" are not acceptable [14:44] s/user experience/developer experience/ [14:45] doctests CAN be tests. unit tests CAN'T be documentation [14:45] * noodles775 suddenly realizes why the email discussion the other day stopped so quickly... [14:45] barry: right, testable documentation [14:45] intellectronica: i agree with that statement (flacoste's too) [14:45] barry, why would there need to be a story about developer experience. "The developer can instantiate this class, and then you have these functions..." [14:45] The problem comes with - to pick an example at random - things like doc/externalbugtracker-comment-pushing.txt [14:46] In which we create a load of mock objects in the doctest in order to be able to run the tests. [14:46] That seems unit-testy. [14:46] bigjools: s/should/can|expected|required/ [14:46] rockstar: ever tried to understand how a class is supposed to be used by pydoctor api reference? it sucks. unittests are the same [14:46] sinzui: "will" [14:46] gmb: actually, i think that's a good example to the contrary. as someone who doesn't know that system as good as you, i really benefit from having documentation whenever i need to work on it [14:46] rockstar: otoh, once you understand how a class is to be used, api reference is great [14:47] bigjools: +1 [14:47] doctests are statements of intent [14:47] I would like to withdraw my request to use unittest to test units. It seems that it's not getting anywhere. [14:47] The meeting is getting close the the end. I just read unit-testies [14:47] intellectronica: Hmm, okay. Then I'd argue that the mock objects should at least be factored out into ftests/somethingorother for cleanliness. [14:47] sinzui: lol [14:47] sinzui: on that note... [14:47] fwiw, footnotes can help with this [14:48] rockstar: gracious of you. as a reviewer, you still have the option of asking that something be unit tested if you think it makes more sense [14:48] i propose we close the meeting for today and schedule a mud wrasslin' contest to settle this once and for all [14:48] at all hands [14:48] and fwiw, this example has been pointed to in the past by others as a reasonable unit test/doc test combo: http://svn.zope.org/zc.queue/trunk/src/zc/queue/queue.txt?view=auto [14:48] intellectronica, as a reviewer, I don't think that makes a lot of sense if there isn't really a standard. :/ [14:48] (I wrote it, so this the importance of "by others") [14:49] I will propose footnotes at another mtg :-) [14:49] okay, thanks everyone for a spirited debate! [14:50] apologies for going over. i'm sure we'll continue this at another time [14:50] #endmeeting [14:50] Meeting finished at 09:50. [14:50] thanks barry [14:50] barry: https://launchpad.canonical.com/TestsStyleGuide#Style%20to%20Avoid [14:50] * mars packs the pitchfork away for another day [14:50] "A very important consideration is that documentation tests are really documentation that happens to be testable." [14:50] bigjools: couldn't agree more! [14:51] so why did everyone disagree with me? :) [14:51] * barry -> #launchpad-code [14:51] bigjools: Because you're you :) [14:51] figures === salgado is now known as salgado-lunch === salgado-lunch is now known as salgado === salgado is now known as salgado-afk