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
|