| 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 |