IRC Chat : 2009-01-31 - OpenMRS

00:23:33 *** bwolfe has joined #openmrs
00:23:33 *** ChanServ sets mode: +o bwolfe
00:43:01 *** nribeka has joined #openmrs
02:07:26 *** nribeka1 has joined #openmrs
02:25:06 *** nribeka has quit IRC
04:42:59 *** nribeka has joined #openmrs
04:57:29 *** bwolfe has quit IRC
05:00:32 *** Echidna has quit IRC
05:03:23 *** nribeka1 has quit IRC
05:04:49 *** nribeka1 has joined #openmrs
05:22:31 *** nribeka has quit IRC
07:51:04 *** Echidna has joined #openmrs
07:51:11 *** Echidna has quit IRC
07:51:32 *** Echidna has joined #openmrs
07:57:31 *** [mharrison] has quit IRC
08:06:26 *** nribeka has joined #openmrs
08:24:15 *** nribeka1 has quit IRC
14:37:23 *** [mharrison] has joined #openmrs
15:14:07 *** bwolfe has joined #openmrs
15:14:07 *** ChanServ sets mode: +o bwolfe
15:51:08 *** atomicturtle has joined #openmrs
15:51:44 *** atomicturtle has left #openmrs
19:39:23 *** Keelhaul has joined #openmrs
19:39:23 *** ChanServ sets mode: +v Keelhaul
19:41:58 <Keelhaul> bwolfe: the ticket comment says i should ask you about @should
20:33:47 <Keelhaul> and it seems like i cant find anything on it on my own
20:33:51 <Keelhaul> maybe i suck at googling
20:33:56 <Keelhaul> but it seems to ignore the @
20:35:33 <bwolfe> Keelhaul: yeah, not sure what to do about that. its highly annoying :-p
20:35:41 <bwolfe> http://openmrs.org/wiki/Unit_Testing_with_@should
20:35:42 <OpenMRSBot> <http://ln-s.net/2UyD> (at openmrs.org)
20:35:47 <bwolfe> I think its on the testing page too
20:35:48 <bwolfe> !testing
20:35:48 <OpenMRSBot> bwolfe: Error: "testing" is not a valid command.
20:35:50 <bwolfe> !unittesting
20:35:51 <OpenMRSBot> bwolfe: Error: "unittesting" is not a valid command.
20:35:54 <bwolfe> !junit
20:35:54 <OpenMRSBot> bwolfe: "junit" --- OpenMRS uses unit testing. See http://openmrs.org/wiki/Unit_Testing and http://openmrs.org/wiki/Module_Unit_Testing
20:36:22 *** atomicturtle1 has joined #openmrs
20:37:02 <Keelhaul> http://rafb.net/p/ytJMG880.html
20:37:10 <Keelhaul> bwolfe: is that what he meant by testing transient tags?
20:37:29 <Keelhaul> unfortunately, i still cant run most of my unit tests
20:37:36 <Keelhaul> the modules still have that file not found error
20:39:06 <bwolfe> Keelhaul: yeah, thats what he meant
20:39:07 <Keelhaul> and LocationTest gives me this: http://rafb.net/p/I0N6GL34.html
20:39:20 <Keelhaul> bwolfe: isnt that the magic of hibernate
20:39:22 <bwolfe> do you think we should have that kind of "adding transient tags" functionality?
20:39:31 <Keelhaul> i dont think anything is saved explicitly
20:40:02 <Keelhaul> concept tags seem to work the same way
20:40:10 <Keelhaul> what's the alternative
20:41:43 <bwolfe> Keelhaul: the alternative is to not have any cascading to tags and that the programmer has to determine the location_tag_id and use that on the location
20:42:25 <Keelhaul> oh
20:43:00 <Keelhaul> oh i see now
20:43:17 <Keelhaul> tags arent something locations should be able to add on the fly?
20:44:02 <Keelhaul> well, where can it be prevented
20:44:08 <Keelhaul> i can only think of hibernate mapping files
20:48:37 <Keelhaul> bwolfe: looking through the classes the other day, i saw some code i dont remember ever writing
20:48:43 <Keelhaul> did you add to the patch?
20:52:53 <bwolfe> Keelhaul: I think I did fix a few things with the testing and dao
20:53:07 <bwolfe> Keelhaul: correct, it can be prevented at the mapping level
20:53:19 <Keelhaul> bwolfe: well, i cant think of any @should annotations on the fly
20:53:26 <Keelhaul> that make sense and arent' too trivial
20:53:32 <bwolfe> Keelhaul: trivial is fine
20:53:45 <bwolfe> tests can (and usually should) be only a few lines
20:53:57 <bwolfe> @should not return empty location list
20:54:11 <bwolfe> @should not save location without name
20:54:23 <Keelhaul> well
20:54:24 <bwolfe> @should not save transient tags
20:54:27 <bwolfe> etc
20:54:30 <bwolfe> :-)
20:54:42 <Keelhaul> the moment i think of a case that cause an error, i rather prevent it in the method itself
20:54:43 <Keelhaul> heh
20:54:46 *** atomicturtle1 has left #openmrs
20:54:53 <Keelhaul> +s
21:08:23 <bwolfe> Keelhaul: and you should then also test that case
21:08:39 <bwolfe> because code gets mis-merged, removed without thinking, replaced, etc
21:08:45 <Keelhaul> yea
21:08:47 <Keelhaul> true
21:08:50 <bwolfe> I've had tests save me several times
21:08:56 <Keelhaul> bwolfe: i think i'll upload the new patch w/o testing
21:09:00 <bwolfe> and there have been others when I wish I had them :-/
21:09:01 <Keelhaul> because i cant get the class to run anymore
21:09:12 <bwolfe> Keelhaul: still getting that same error ?
21:09:17 <Keelhaul> yea
21:09:21 <Keelhaul> http://rafb.net/p/I0N6GL34.html
21:09:57 <Keelhaul> bwolfe: is there a junit test for throwing exceptions?
21:12:43 *** atomicturtle has joined #openmrs
21:13:02 *** atomicturtle has left #openmrs
21:14:53 <bwolfe> Keelhaul: there is an annotation for it
21:15:12 <bwolfe> Keelhaul: or maybe you put it in the @Test(throws=MyException.class)
21:15:34 <Keelhaul> bwolfe: any way i can express it with @should beforehand?
21:16:13 <bwolfe> Keelhaul: just with text, like you would say it
21:16:25 <bwolfe> @should throw myexception if no name given
21:16:30 <bwolfe> or similar
21:18:13 <Keelhaul> bwolfe: where do the @should annotations go in services
21:18:18 <Keelhaul> service or impl
21:18:51 <bwolfe> the service interface
21:19:41 <Keelhaul> ok
21:19:52 <Keelhaul> i guess if the method is private, theres no point to put @should in it
21:20:00 <Keelhaul> no way to call it from the outside
21:20:02 <Keelhaul> even though it's nto trivial
21:20:53 <Keelhaul> hmm
21:21:00 <Keelhaul> the rest of the service seems too trivial
21:26:53 <Keelhaul> bwolfe: <set name="tags" table="location_tag_map" lazy="false" cascade="none">
21:27:06 <Keelhaul> that's what already in the hbm xml
21:27:11 <bwolfe> Keelhaul: you can make the method protected and test it that way
21:27:40 <bwolfe> Keelhaul: hmm, then why is it cascading the save ? are you calling save on the tags explicitly ?
21:27:47 <Keelhaul> no
21:27:55 <Keelhaul> who said it is cascading
21:28:08 <Keelhaul> my unit test saves the tag beforehand, yes
21:28:18 <Keelhaul> never tested transient
21:29:05 <Keelhaul> bwolfe: what i could do is add another check to addTag() that prevents adding of tags with no id
21:29:25 <bwolfe> Keelhaul: also a good option
21:29:38 <Keelhaul> the question is
21:29:46 <Keelhaul> should it silently ignore the user's request
21:29:52 <Keelhaul> or throw an exception when trying to save
21:32:34 <bwolfe> Keelhaul: exception probably
21:32:51 <bwolfe> Keelhaul: or you could try to match to a current tag by name
21:32:57 <bwolfe> if you can't match, then throw exception
21:33:06 <bwolfe> if you do match, set that location_tag_id
21:33:21 <bwolfe> (if you do this, you'll probably want to remove the check at the addTag() level)
21:38:03 <Keelhaul> bwolfe: why not set tag = exisstingTag;
21:38:14 <Keelhaul> i dont have a check in addTag() yet
21:40:09 <Keelhaul> if i just set the id, it will give locations the power to overwrite tags
21:40:12 <Keelhaul> should that be allowed?
21:40:45 <bwolfe> Keelhaul: don't just set the id, replace the existing non-id object with the object-with-id that you fetched from the database that has the same tag name
21:41:14 <Keelhaul> bwolfe: yes, that's what i said =P
21:41:26 <Keelhaul> and this is what you said: [22:33:01] <@bwolfe> if you do match, set that location_tag_id
21:43:32 <bwolfe> Keelhaul: ok, you win :-)
21:44:33 <Keelhaul> um
21:44:35 <Keelhaul> bwolfe: http://www.rafb.net/paste/
21:44:40 <Keelhaul> can you open that url correctly
21:44:46 <Keelhaul> or do you get an object not found
21:47:54 <Keelhaul> bwolfe: http://pastebin.com/m46a7562f
22:11:20 <Keelhaul> bwolfe: ok if you have no objections, i'll update the patch on the ticket =P
22:17:28 <bwolfe> Keelhaul: the only comment I have is to make the error message something like "Tag 'tagname' must be saved to the database before saving this location" so that its more specific
22:17:39 <Keelhaul> too late =(
22:18:09 <bwolfe> Keelhaul: did you use my patch, or go off your previous changes ?
22:18:15 <bwolfe> Keelhaul: in my patch, tests were running :-/
22:18:26 <Keelhaul> no, i think it's not the class itself
22:18:30 <Keelhaul> dunno what it could be
22:18:39 <Keelhaul> i used your patch
22:18:45 <Keelhaul> maybe you could apply it and try
22:19:10 <bwolfe> Keelhaul: hmm, interesting
22:21:32 <Keelhaul> !ticket 169
22:21:32 <OpenMRSBot> Keelhaul: Ticket #169: http://dev.openmrs.org/ticket/169
23:24:15 <OpenMRSBot> Recent updates in the world of openmrs: OpenMRS Changesets: Changeset [6763]: openmrs_logic: run each task on its own timer - use … <http://dev.openmrs.org/changeset/6763>
23:54:31 *** nribeka1 has joined #openmrs