User1: bac, just you and me today
User2: oic
User2: ok
User1: call as soon as you show up
User1: no rush ;-)
User1: back in a sec
User1: Whenever I see "Force a build" on buildbot for some strange reason I always think "Bust a move"
User1: bac, going to go get some lunch.  If you have something even partially working with the --load-test thing, I'd love to try it, or pair with you or something.  It will greatly help with triaging the bugs that we get more and more of, I hope.
User2: User1 my --load-list (basic) is working now and i just completed my first test run
User1: yay bac.  I would love to use it for help with diagnosis, whenever you have a branch I can mess with.
User2: ok, i'll push something soon
User1: cool thanks
User2: User1 pushed to lp:~bac/launchpad/ordered-load-list
User2: User1 i was able to trim the test failures produced by the branch.txt ordering bug down to the 110 on the DatabaseFunctionalLayer and the failure is shown
User1: ordered-load-list: awesome, thanks bac.  writing doc for tomorrow's meeting then will return to running diagnostics for the bugs.
User1: branch.txt: that's one of the bugs we have on the board?
User1: No need to watch, but if you are interested, the doc is https://docs.google.com/a/canonical.com/document/d/1_BtI6VDHbHL5oFp8gPX8b1X9sKfK1uj1pnjFbF1a9n4/edit
User2: User1 that's the vocabularly bug i fixed by sorting the branch names.
User2: this branch, pre-fix, shows the bug cropping up with a small number of tests
User2: by using the new --load-list
User1: bac, oh, interesting.  So, it is an ordering issue of some sort, and it's reproducable now.  excellent!
User1: I suppose it does indicate a test isolation thing to investigate.  If you want to, I'd be fine with it, but I'd prefer for it to be after we dig into some of the other bugs
User2: ok.  it has just grabbed my curiousity
User2: http://pastebin.ubuntu.com/960718/
User2: if these five tests are run, with branch.txt last, the failure occurs.  eliminating any one of them causes the failure to not be seen.
User1: wow
User1: bac, if you want to be stumped, I'd prefer that you be stumped about, say, bug 992184 :-)
User3: Bug #992184: lib/lp/services/database/doc/textsearching.txt fails intermittently/rarely on parallel tests <paralleltest> <Launchpad itself:Triaged> < https://launchpad.net/bugs/992184 >
User2: ok
User1: as the most stumpy of the stumps I've seen lately
User4: that's beautiful
User1: lol
User2: User1 when you have time could you review https://code.launchpad.net/~bac/launchpad/ordered-load-list/+merge/104287 ?
User1: bac, will do it now.  My call is in 7.
User2: User1 it'll only take you 6
User1: :-) k
User1: bac, how odd that the old code used a list for the tests.  A set, which it created along the way, would have been much more efficient for the "in" questions, and the order appears irrelevant to me.
User2: User1 line 16 shows the old deleted code did use a set
User1: bac, sorted will create a list
User1: line 36 of the diff would have been faster with a set
User2: User1 ah, right.  both the use of the set and the sorted destroy the original ordering
User1: right
User2: User1 and, of course, the test suites get populated not by the order of the tests list but the order they are seen in iterate_tests(suite)
User2: so, basically, we can say original order was not important.  :)
User1: :-) right
User1: my point was simply that you were concerned about efficiency of the new approach, and the old approach was not as efficient as it could have been, even given trivial optimizations/consideration.
User1: oh, flacoste does not appear to be around.  maybe he is at that pre-UDS meeting.  In any case, no rush there apparently. :-)
User1: bac, I question raising an exception (lines 55-56) but I'm considering.  If I am trying to rerun tests on a new branch/revision, should it really throw up because a test is no longer around?  If it really should, it would be nicer to throw up with *all* the missing tests.  Let me see what the old code did...
User1: yeah the old code used it as a filter
User1: it did not puke
User1: I'm inclined to do the same
User1: though could be argued otherwise, maybe
User1: bac, if you are worried about efficiency, I think you could have a more efficient initial data structure.
User1: rather than layers -> testnames -> suites
User1: you could simply have testnames -> (layer, suite)
User1: then you would no longer need find_layer
User1: you could simply do a single dict lookup
User1: that would probably be way way faster
User1: also afaict we discard ordered_layers and do nothing with it
User1: removing it would be a good idea
User1: bac ^^
User2: looking
User2: yep, ordered_layers is a leftover from a previous attempt
User2: i like the improvements you sugggest, gary_poster
User1: cool
User2: i didn't do a time comparison but it isn't noticably slow, as is.
User2: with your change it may be faster than the original...
User1: I think it would probably still be simpler/less code in the way I suggest, at least
User1: what do you think about the exception, bac?  treat the list as an ordered filter and silently ignore missing tests, or abort if things are not as we expect?
User1: If someone cares, they will presumably be looking at the tests that are run
User1: I dunno
User1: could be argued either way
User1: though I still incline to no exception
User2: i think maintaining the previous behavior is the tie breaker
User1: cool
User1: bac, I added one other note in the MP when I was summarazing our discussion:
User1: - We should include a comment as to why the code is maintaining order, so that future code navigators will have a clue as to intent.
User2: ok
User2: User1 so, here are the timings for the three versions: http://paste.ubuntu.com/960961/
User2: with your changes, it is minimally faster and has few LOC
User2: </deadhorse>
User1: bac, :-) cool
User1: do you think it reads more nicely?
User2: yes, and the reader won't get creeped out by that awful looping find_layer
User1: :-) cool
User1: interesting that the initial version was so ,uch slower
User1: m
User1: bac, btw, please don't lose your ideas from this morning to include an option to only run tests in the layer of a given target test.
User1: I thought that would be interesting to experiment with, and possibly a real time saver
User2: User1 no, i haven't forgotten.  but, i discovered it is quite easy to discover using --load-list / --list-tests in conjuction so that you can manually trim the input list
User2: but making that automatic would be swell
User1: bac, oh, what is that trick?  I don't understand yet
User2: bin/test --load-list in.txt --list-test > out.txt
User1: but what does that buy you?
User2: out.txt will then show the layer breaks
User1: oh!
User1: cool
User1: remember that trick for Friday sharing time please bac :-)
User2: yeah, so my itch got less itchy when i found that
User1: yeah
User2: +1
User2: thanks for the reminder...
User2: it's hell being clever and forgetful.  you don't get to show off.
User2: User1 your review was just a comment, not an approve.  i've pushed the changes if you'd like to vote.
User1: bac, cool, going
User1: bac, all approved.  thank you!
User2: thx
launchpad-yellow
Ubuntu IRC