#musicbrainz-devel

/

      • ijabz
        Two vinyl release, I would just label as 1-20
      • 2009-12-04 33849, 2009

      • aCiD2`
        All my vinyl releases are 1-n for each disc in the set
      • 2009-12-04 33859, 2009

      • aCiD2`
        so my albums are literally tracks 1/2,over 5 discs
      • 2009-12-04 33824, 2009

      • ijabz
        I mean there are alot of people who dislike the (disc1) being added to ttile, isnt there a plugin in Picard to remove it
      • 2009-12-04 33829, 2009

      • aCiD2`
        yea
      • 2009-12-04 33800, 2009

      • ijabz
        I think the vote is split and it would be useful if the db better supported both methods
      • 2009-12-04 33820, 2009

      • aCiD2`
        decisions like that don't work
      • 2009-12-04 33834, 2009

      • nikki
        I don't like it in the title, but I don't number stuff from 1-20, I would have discnumber=1, tracknumber=1, etc to discnumber=2 tracknumber=10 or whatever :P
      • 2009-12-04 33836, 2009

      • aCiD2`
        the only way you could do this would be with some views on the database
      • 2009-12-04 33807, 2009

      • aCiD2`
        anyway, I gotta sleep
      • 2009-12-04 33811, 2009

      • nikki
        sleep well
      • 2009-12-04 33813, 2009

      • aCiD2`
        boiler repair man at 8am tomorrow, at last
      • 2009-12-04 33818, 2009

      • aCiD2` could do with some damn hot water
      • 2009-12-04 33810, 2009

      • ijabz
        nikki, that is a good point, anyway I also need to go to sleep now, so I'll mull it over some more
      • 2009-12-04 33840, 2009

      • ijabz
        acid, our water is too hot, Iturned downed the bolier thermostat but its made no difference !
      • 2009-12-04 33853, 2009

      • nikki
        our water is just right :D
      • 2009-12-04 33839, 2009

      • ijabz
        Sounds like a modern day version of Goldilocks and the three bears
      • 2009-12-04 33848, 2009

      • navap
        haha
      • 2009-12-04 33807, 2009

      • nikki
        one of the dinner ladies in infants school used to call me goldilocks :(
      • 2009-12-04 33816, 2009

      • ijabz
        ciao
      • 2009-12-04 33844, 2009

      • brianfreud_ joined the channel
      • 2009-12-04 33807, 2009

      • NXisGOD joined the channel
      • 2009-12-04 33820, 2009

      • ruaok joined the channel
      • 2009-12-04 33829, 2009

      • ruaok finally appears in california
      • 2009-12-04 33803, 2009

      • brianfreud joined the channel
      • 2009-12-04 33809, 2009

      • NXisGOD joined the channel
      • 2009-12-04 33802, 2009

      • brianfreud_ joined the channel
      • 2009-12-04 33815, 2009

      • brianfreud_ joined the channel
      • 2009-12-04 33816, 2009

      • NXisGOD joined the channel
      • 2009-12-04 33854, 2009

      • djce joined the channel
      • 2009-12-04 33800, 2009

      • brianfreud_ joined the channel
      • 2009-12-04 33846, 2009

      • pronik``` joined the channel
      • 2009-12-04 33831, 2009

      • brianfreud_ joined the channel
      • 2009-12-04 33831, 2009

      • warp
        aCiD2 \o/
      • 2009-12-04 33817, 2009

      • warp
        aCiD2: i don't really understand the layout of stuff under t/, any hints?
      • 2009-12-04 33827, 2009

      • aCiD2
        yea, I wanted to help you with that :)
      • 2009-12-04 33846, 2009

      • aCiD2
        the idea is now, we have an almost 1:1 mapping between controller actions and test files
      • 2009-12-04 33857, 2009

      • aCiD2
        because controller_artist.t was getting waay too busy for me to be able to dive in
      • 2009-12-04 33815, 2009

      • aCiD2
        So in your case, if you've got the create artist edit type, this is tested in t/actions/artist/artist-edit-create.t (or something like that)
      • 2009-12-04 33852, 2009

      • warp
        where does the 'actions' bit come from?
      • 2009-12-04 33856, 2009

      • aCiD2
        the name?
      • 2009-12-04 33818, 2009

      • aCiD2
        sub routines in a controller that are accessible to users are called "actions" in catalyst
      • 2009-12-04 33838, 2009

      • warp
        well, with php I'm used to having the unittest tree match 1:1 with the tree of files to be tested
      • 2009-12-04 33831, 2009

      • warp
        so <root>/src/foo/bar/baz.php is tested by e.g. <root>/tests/foo/bar/bazTest.php
      • 2009-12-04 33834, 2009

      • aCiD2
        right
      • 2009-12-04 33809, 2009

      • aCiD2
        I just found that there's a lot more testing to do for each action, that having it all together was getting messy
      • 2009-12-04 33811, 2009

      • warp
        (and in python I ussually have tests for src/foo/bar/baz.py in src/foo/bar/tests/test_baz.py :)
      • 2009-12-04 33848, 2009

      • aCiD2
        We still run the tests all together though, that's what t/controllers/artist.t does
      • 2009-12-04 33811, 2009

      • warp
        sure, but I don't understand the logic of the current t/ tree :)
      • 2009-12-04 33840, 2009

      • aCiD2
        I wanted it more hierarchical, but luks didn't want that
      • 2009-12-04 33850, 2009

      • aCiD2
        because he wanted to be able to do t/*artist*.t to run all artist related tests
      • 2009-12-04 33856, 2009

      • warp
        right
      • 2009-12-04 33819, 2009

      • warp
        so t/controllers/artist.t will test lib/MusicBrainz/Server/Controller/Artist.pm ?
      • 2009-12-04 33831, 2009

      • aCiD2
        Yep, by running everything in t/actions/artist
      • 2009-12-04 33853, 2009

      • warp
        why is one of them plural? :)
      • 2009-12-04 33804, 2009

      • aCiD2
        cause I'm weird probably :P
      • 2009-12-04 33809, 2009

      • warp
        hihi
      • 2009-12-04 33825, 2009

      • aCiD2
        I guess it's cause t/actions/artist should contain many files, as it corresponds to many actions
      • 2009-12-04 33835, 2009

      • aCiD2
        but t/controllers/artist points to a single file?
      • 2009-12-04 33840, 2009

      • aCiD2
        i dunno, i have no real reason :P
      • 2009-12-04 33852, 2009

      • warp
        no, I mean, Server/Controller/Artist.pm maps to controllerS/artist.t
      • 2009-12-04 33858, 2009

      • aCiD2
        ah right
      • 2009-12-04 33808, 2009

      • aCiD2
        yes, that's inconsistent, I should rename that
      • 2009-12-04 33814, 2009

      • warp
        (nevermind skipping the MusicBrainz/Server bit :)
      • 2009-12-04 33820, 2009

      • aCiD2
        heh
      • 2009-12-04 33825, 2009

      • aCiD2
        well that's common perl practice
      • 2009-12-04 33853, 2009

      • aCiD2
        well, perl practice for tests is to name the 01_compiles.t, 02_basic.t, 03_throws_exceptions.t and stuff, which really doesn't work for something this big
      • 2009-12-04 33800, 2009

      • warp nods.
      • 2009-12-04 33815, 2009

      • warp
        so now you've got a frankenstein of twe different approaches :)
      • 2009-12-04 33859, 2009

      • aCiD2
        pretty much :(
      • 2009-12-04 33803, 2009

      • warp
        i don't particularly care about which layout is used, I just would like a fairly predictable mapping of source files to test files.
      • 2009-12-04 33836, 2009

      • aCiD2
        if you have a formal way we could structure stuff, I'm happy to move stuff around
      • 2009-12-04 33812, 2009

      • warp
        ideally, you could write a simple regex to transform any path+filename to .pm file to the exact .t file which tests that file.
      • 2009-12-04 33812, 2009

      • aCiD2
        yea, I'd be ok with just having MusicBrainz::Server::Data::Artist -> t/data/artist.t
      • 2009-12-04 33808, 2009

      • warp
        cool
      • 2009-12-04 33850, 2009

      • aCiD2
        ok, i'll be back in about 20 minutes, I have *got* to buy a heater
      • 2009-12-04 33857, 2009

      • aCiD2
        coding when you can see your own breath is no fun
      • 2009-12-04 33859, 2009

      • warp
        haha, sounds like a plan
      • 2009-12-04 33806, 2009

      • warp nods.
      • 2009-12-04 33854, 2009

      • nikki
        I thought they were fixing the boiler today?
      • 2009-12-04 33813, 2009

      • aCiD2
        so did I, so did I
      • 2009-12-04 33825, 2009

      • nikki
        I see...
      • 2009-12-04 33843, 2009

      • aCiD2
        tut tut warp, your age role breaks test.mb!
      • 2009-12-04 33850, 2009

      • aCiD2
        apparently N_Delta_Ymd only came out in a recent Date::Calc
      • 2009-12-04 33804, 2009

      • warp
        oh :(, sorry!
      • 2009-12-04 33812, 2009

      • aCiD2
        no worries, I wouldn't have known either :)
      • 2009-12-04 33828, 2009

      • aCiD2
        "running t/m013" there you go, we could have that naming convention ;)
      • 2009-12-04 33838, 2009

      • warp
        awesome
      • 2009-12-04 33853, 2009

      • aCiD2
        oh, alongside the convention I mentioned, I wouldn't mind having t/tickets/1947.t
      • 2009-12-04 33857, 2009

      • aCiD2
        or something
      • 2009-12-04 33814, 2009

      • warp
        we could have a predictable hashing algorithm from sourcefilepath to t/ testfilename.
      • 2009-12-04 33820, 2009

      • aCiD2
        haha
      • 2009-12-04 33836, 2009

      • warp
        ooh, t/tickets/1947.t sounds nice.
      • 2009-12-04 33812, 2009

      • warp
        btw, it takes about an hour to run the full test suite here... :S
      • 2009-12-04 33817, 2009

      • aCiD2
        wow
      • 2009-12-04 33828, 2009

      • aCiD2
        the biggest bottleneck atm is InsertTestdata.sql
      • 2009-12-04 33830, 2009

      • warp
        celeron 2ghz
      • 2009-12-04 33843, 2009

      • aCiD2
        that's why controllers/artist.t is faster than say, controllers/label.t
      • 2009-12-04 33845, 2009

      • warp would like some kind of desktop diff viewer
      • 2009-12-04 33843, 2009

      • warp
        git diff master HEAD | less just isn't as usuable when you're used to the review board or trac interfaces.
      • 2009-12-04 33821, 2009

      • aCiD2
        hrm, git diff is meant to use a pager anyway
      • 2009-12-04 33849, 2009

      • warp
        oh, yeah, it does. i'm just used to piping everything into less :)
      • 2009-12-04 33813, 2009

      • aCiD2
        :)
      • 2009-12-04 33815, 2009

      • aCiD2
        old habits!
      • 2009-12-04 33802, 2009

      • warp
        even cat foo | less is not uncommon.
      • 2009-12-04 33804, 2009

      • warp
        >_<
      • 2009-12-04 33812, 2009

      • aCiD2
        alright, bbs
      • 2009-12-04 33856, 2009

      • brianfreud_ joined the channel
      • 2009-12-04 33812, 2009

      • aCiD2
        oh goody, he came while I was out
      • 2009-12-04 33815, 2009

      • aCiD2
        no surprises there
      • 2009-12-04 33826, 2009

      • warp
        ah, "git diff master HEAD | kompare -" works.
      • 2009-12-04 33821, 2009

      • warp
        well, sort of anyway.
      • 2009-12-04 33840, 2009

      • warp
        apparantly mercurial is the only SCM which makes this easy with any diff too, with their 'extdiff' option.
      • 2009-12-04 33850, 2009

      • warp
        s/diff too/diff tool/
      • 2009-12-04 33844, 2009

      • aCiD2
        git can be configured to use merge tools
      • 2009-12-04 33853, 2009

      • warp
        yes, but i'm not merging.
      • 2009-12-04 33853, 2009

      • aCiD2
        even so, I think it uses the merge tool
      • 2009-12-04 33853, 2009

      • warp does not understand.
      • 2009-12-04 33826, 2009

      • nikki_ joined the channel
      • 2009-12-04 33848, 2009

      • aCiD2
        I mean it uses the same tool to view the diffs
      • 2009-12-04 33856, 2009

      • aCiD2
        I could be wrong, I just use git diff and that's normally enough :)
      • 2009-12-04 33842, 2009

      • warp
      • 2009-12-04 33849, 2009

      • warp
        which adds a 'git difftool' command.
      • 2009-12-04 33807, 2009

      • warp
        which seems to have been included in gits later than the one i'm using :)
      • 2009-12-04 33846, 2009

      • aCiD2
        ah :)
      • 2009-12-04 33817, 2009

      • warp
        ugh, difftool invokes the diff tool once for each changed file, not particularly useful.
      • 2009-12-04 33829, 2009

      • warp stops mucking with it.
      • 2009-12-04 33806, 2009

      • aCiD2
        woohoo, I love resolving issues by just upgrading packages from cpan
      • 2009-12-04 33833, 2009

      • warp
        :)
      • 2009-12-04 33859, 2009

      • warp tries to fix whatever is wrong with t/edit_artist_edit.t
      • 2009-12-04 33815, 2009

      • aCiD2
        it should be a case of just removing anything that tests "$edit->artist" with just loading "$artist" and using that
      • 2009-12-04 33824, 2009

      • aCiD2
        like in t/edit_label/edit.t
      • 2009-12-04 33845, 2009

      • warp nods.
      • 2009-12-04 33828, 2009

      • warp
        oh, seems all I needed to add to that file is already there.
      • 2009-12-04 33843, 2009

      • warp
        there's some TODO stuff which i'll leave untouched for now.
      • 2009-12-04 33803, 2009

      • aCiD2
        interestingly, the todo stuff should pass
      • 2009-12-04 33818, 2009

      • nikki_
        warp: useless uses of cat are fun!
      • 2009-12-04 33820, 2009

      • warp
        oh, no, i've confused myself.
      • 2009-12-04 33824, 2009

      • aCiD2
        hehe
      • 2009-12-04 33854, 2009

      • warp
        the edit itself is tested, but I need to test the display_data of the edit, because that's what I added.
      • 2009-12-04 33814, 2009

      • aCiD2
        Right, at the moment I'm not doing that in the edit_ files though
      • 2009-12-04 33826, 2009

      • aCiD2
        I'm doing that in t/actions/foo/edit-foo.t
      • 2009-12-04 33842, 2009

      • aCiD2
        i was unsure whether we really need to test it in the edit tests, what do you think?
      • 2009-12-04 33828, 2009

      • warp
        actions confuses me, because there is no lib/MusicBrainz/Server/Actions/Edit.pm
      • 2009-12-04 33846, 2009

      • aCiD2
        I guess t/controller/artist/index.t would be a better place then?
      • 2009-12-04 33802, 2009

      • aCiD2
        Because there is a MusicBrainz::Server::Controller::Artist::index (well, show, but you get the idea)
      • 2009-12-04 33849, 2009

      • aCiD2
        But the action tests don't necessarily focus on testing a single action. create artist, for example, also hits the /edit/show action to try viewing the edit it created
      • 2009-12-04 33845, 2009

      • warp
        well, I've changed build_display_data() in Edit/Artist/Edit.pm. to test that, edit_artist_edit.t seemed like a logical place. but ofcourse build_display_data is only part of the machinery which provides $edit->display_data, and I don't have the overview which other bits are involved
      • 2009-12-04 33823, 2009

      • aCiD2
        I wouldn't ever test build_display_data directly, because that's never meant to be called
      • 2009-12-04 33832, 2009

      • aCiD2
        it's for Data::Edit->load_all