On our project the business is kind of complicated and sometimes we build a 3 pager sql code. It would be nice to have a tag for those special occasions that simply call a method from a Java class. This Java class builds a huge SQL based on the arguments of the xml file and other information taken from the database.
Yes, it looks good, go for it. It would be a useful tag to have in SQLUnit. One suggestion, though, you may want to add type attributes for the constructorParam and methodParam tags, so you know what kind of Object to pass in to invoke().
-sujit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
handler.process(Element) just returns an Object, in your case it would be a large dynamic SQL string, right, so you could probably just make it return a string.
-sujit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Just something that occured to me on my bus ride home yesterday. You may want to call the tag <dynamicSql> and have a DynamicSqlHandler.java do the handling, which would essentially invoke the method and get back a String, then do whatever SqlHandler does with the string.
Actually there is scope for refactoring right there, DynamicSqlHandler should extend SqlHandler, and so should CallHandler. I actually started on the Mock testing stuff because I did not have a good way to exercise the CallHandler with PostgreSQL, since it returns stored functions.
Just FYI, I committed some code yesterday for the customized reporting patch contributed by Paul and Rob. So you may want to update your local directory. Let me know if you are going to be done soon, if not, I may just go ahead and release with the changes that are there at the moment, and ship the new tag in a future release.
Thanks
Sujit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
No problem, hope you had a good vacation. OK, I will release with the changes either tonight or tomorrow.
And yes, I think the methodInvoker tag should remain that way. Although I am a little confused, if the java method returns true or false, what are you going to check? The fact that it succeeded or that it did not throw an exception? Anyway, I guess I will wait until you check in the tag.
Let me know if you would be interested in writing the dynamicSql tag. It should be very similar to the methodInvoker tag, except it will invoke a named method in a named class to retrieve a String, which will then execute against the database.
-sujit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I try to login to source forge and It does not work.
was something changed? The administrator confirmed to me that the port is open in TCP and UDP
This is my log:
I tried with what you gave me, here is the log:
cvs -d :pserver:mlaureti@cvs.sqlunit.sf.net:/cvsroot./sqlunit login
Logging in to :pserver:mlaureti@cvs.sqlunit.sf.net:2401:/cvsroot./sqlunit
cvs [login aborted]: Error reading from server cvs.sqlunit.sf.net: -1: Unknown error
***** CVS exited normally with code 1 *****
And also tried with no dot at the end of cvsroot and got the same:
cvs -d :pserver:mlaureti@cvs.sqlunit.sf.net:/cvsroot/sqlunit login
Logging in to :pserver:mlaureti@cvs.sqlunit.sf.net:2401:/cvsroot/sqlunit
cvs [login aborted]: Error reading from server cvs.sqlunit.sf.net: -1: Unknown error
***** CVS exited normally with code 1 *****
Thanks
Mario L.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Yeah, the <dynamicSql> tag would be a great addition to SQLUnit. SQLUnit advertises that it is for testing stored procedures. But it can also test SQL statements. Unfortunately, if you use it in that capacity, there is no coupling between the SQL statement under test declared in test suite, and the actual program that uses that SQL statement. If I am a careless programmer, I might adversely change the SQL statement in my program, and the previously written SQLUnit unit test would never show that I broke it unless I remember "ok, I changed a SQL statement in my program, now I need to replace all instances of it in the SQLUnit tests to see if it still works."
That problem doesn't happen if you do all your SQL in stored procedures or if SQLUnit gains the ability to extract SQL statements from the program under test such as it would with the <dynamicSql> tag. You can feel more confident in your unit tests with this tight coupling between SQL that the program uses and the SQL under test.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Mario's finished writing the dynamicSql handler, it will be in the cvs repository shortly and will be released in the next release sometime late next week or early the week after. I will update this thread when the release happens or you can monitor the release so it automatically emails you.
It would be nice if you could run some test cases for the dynamicSql handler and let us know how it went.
Thanks
Sujit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I am running a little behind on putting your code into CVS. I discovered that you had refactored out stuff from SqlHandler to do the DynamicSqlHandler, which makes sense, but it means that I should also do some refactoring of my own to clean up my part of the code to match yours.
I guess thats the bad news. The good news is that the mock testing framework is now much improved and serves as a good basis for regression testing after refactoring, thus making it easy to do the refactoring.
Also if you could give me some documentation for these tags for the user's guide that would be cool too. Take a look at the user guide for other tags to see what they look like. I know you are pressed for time, so if you are not able to do this, I can just reverse-engineer something from the code.
Could you also send me the dtd entries for these new tags, looks like you may have missed them. Although I guess I can reverse engineer them from the code too.
Thanks
Sujit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I got Jalopy to rewrite your code (not fully, it failed with array index out of bounds exception in one deeply bracket nested call) in one case, but mostly it seems to work fine. Anyway, there is an ant reformat task to invoke Jalopy (you must have it installed separately, it will not be part of the distribution because of jar file incompatibilities, and in any case, nobody would probably want to use it. The code conventions XML file is included in the doc/ directory if anybody wants to use it).
I have some questions. Can we change the following attributes to make it more concise?
className -> class
methodName -> method
isMethodStatic -> static
Also, I was looking at the constructorArgs and methodArgs, and it may be nice to have them defined as a set of args, like so:
<constructorArgs>
..<arg name="arg1" type="java.lang.Boolean" value="true" />
...
</constructorArgs>
The type could be an optional attribute which is not used for other occurences of the arg element. Also the value moves from being in a text field to being an attribute, although I am looking in both places in the ArgHandler. This may make the DTD a little too loose however and may lead to NPEs which will need to get caught in the handler. Same for <methodArgs>. This would allow us to use the same logic to pull things out of the arg tag that already exists. I have created a new handler for the arg element too.
If you are ok with these changes, it will become necessary to change your test cases. Let me know if its too inconvenient, and we will rethink this. I think it makes sense to do this in light of long term usage, because it helps us to reuse code and to keep our test XMLs concise.
I have checked in the code for the DynamicSqlHandler and the MethodInvokerHandler, and also folded in the changes for SqlHandler and TestHandler that you provided. I have renamed MethodInvoker.java to MethodInvocationUtils.java and have changed the code in it somewhat to make it more concise. I did not check in the MethodInvokerException class, instead I folded in the functionality to one of IErrorCodes for the SQLUnitException. The last is because I really created the IErrorCodes and the MessageFormat stuff to avoid having to do more than one exception class.
The classes compile, but I have not written any test cases for it yet. I plan to use some of your testing code to do this, and once the tests pass, we will release this. I will let you know when that happens, maybe sometime late next week.
-sujit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Received email from Mario, he is ok with the changes, so this needs to go in, will do this tonight.
Another issue was whether methodinvoker should report any exception in the method. Transcript follows:
---
> In the methodinvoker tag, I see that all you are doing is to run the
> method in the process() method. In all cases, you return an empty
> DatabaseResult object. Is the intent to only check that the method ran
> without throwing exceptions. If so, we may want to trap the exception,
> and if one is encountered, resetAsException() the DatabaseResult object.
--
and Mario confirmed that this was the intent, so I will make this change too.
Just fyi, if you are following this thread.
-sujit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hello,
On our project the business is kind of complicated and sometimes we build a 3 pager sql code. It would be nice to have a tag for those special occasions that simply call a method from a Java class. This Java class builds a huge SQL based on the arguments of the xml file and other information taken from the database.
The following is an example:
<methodInvoker className="ca.gc.psc.psr.business.poster.PosterManager" methodName="createQuestionnaire">
<constructorParam>Job Familly 1</constructorParam>
<methodParam>1</methodParam>
<methodParam>550</methodParam>
<methodParam>1100</methodParam>
<methodParam>2000</methodParam>
<methodParam>5000</methodParam>
</methodInvoker>
Does that fit into the SQLUnit philosophy?
If it does, can you tell me if you like the tag names or suggest alternatives.
I will be happy to contribute and create that new tag.
Thanks
Mario Laureti
Hi Mario,
Yes, it looks good, go for it. It would be a useful tag to have in SQLUnit. One suggestion, though, you may want to add type attributes for the constructorParam and methodParam tags, so you know what kind of Object to pass in to invoke().
-sujit
Hello again sujit,
I am almost done with the tag...
I have one little question:
handler.process(sql/all) returns a DatabaseResult.
In this case it is a method call. Do you prefer that I create a MethodResult or should we just use the DatabaseResult class?
Thanks
Mario L.
Hi Mario,
handler.process(Element) just returns an Object, in your case it would be a large dynamic SQL string, right, so you could probably just make it return a string.
-sujit
Hi Mario,
Just something that occured to me on my bus ride home yesterday. You may want to call the tag <dynamicSql> and have a DynamicSqlHandler.java do the handling, which would essentially invoke the method and get back a String, then do whatever SqlHandler does with the string.
Actually there is scope for refactoring right there, DynamicSqlHandler should extend SqlHandler, and so should CallHandler. I actually started on the Mock testing stuff because I did not have a good way to exercise the CallHandler with PostgreSQL, since it returns stored functions.
Just FYI, I committed some code yesterday for the customized reporting patch contributed by Paul and Rob. So you may want to update your local directory. Let me know if you are going to be done soon, if not, I may just go ahead and release with the changes that are there at the moment, and ship the new tag in a future release.
Thanks
Sujit
Hello Sujit,
I took a couple days off to go on family camping...
I should have let you know. Sorry.
Go ahead and release with the changes that are there.
Also about the tag, the java method we are calling does not return a SQL String. It actually performs the sql and returns true or false.
Should we have two different tags then?
Thanks.
Mario Laureti
Hi Mario,
No problem, hope you had a good vacation. OK, I will release with the changes either tonight or tomorrow.
And yes, I think the methodInvoker tag should remain that way. Although I am a little confused, if the java method returns true or false, what are you going to check? The fact that it succeeded or that it did not throw an exception? Anyway, I guess I will wait until you check in the tag.
Let me know if you would be interested in writing the dynamicSql tag. It should be very similar to the methodInvoker tag, except it will invoke a named method in a named class to retrieve a String, which will then execute against the database.
-sujit
Hello Sujit,
I wrote the message too fast.
I will check for an exception.
Yes I will do the dynamicSql too.
Mario L.
Hello Sujit,
I try to login to source forge and It does not work.
was something changed? The administrator confirmed to me that the port is open in TCP and UDP
This is my log:
I tried with what you gave me, here is the log:
cvs -d :pserver:mlaureti@cvs.sqlunit.sf.net:/cvsroot./sqlunit login
Logging in to :pserver:mlaureti@cvs.sqlunit.sf.net:2401:/cvsroot./sqlunit
cvs [login aborted]: Error reading from server cvs.sqlunit.sf.net: -1: Unknown error
***** CVS exited normally with code 1 *****
And also tried with no dot at the end of cvsroot and got the same:
cvs -d :pserver:mlaureti@cvs.sqlunit.sf.net:/cvsroot/sqlunit login
Logging in to :pserver:mlaureti@cvs.sqlunit.sf.net:2401:/cvsroot/sqlunit
cvs [login aborted]: Error reading from server cvs.sqlunit.sf.net: -1: Unknown error
***** CVS exited normally with code 1 *****
Thanks
Mario L.
Hi Mario,
Here is the link to the FAQ, you should be using :ext: instead of :pserver: since you have read-write access and you should also set CVS_RSH=ssh.
https://sourceforge.net/docman/display_doc.php?docid=21828&group_id=77832
There are some docs from sourceforge as well about the subject, not sure what the link is, but thats where I got my info from.
-sujit
Hi Mario,
Were you able to get r/w access to cvs? Please let me know.
Thanks
Sujit
I guess not...
cvs add standalonerunner (in directory C:\_DEV\sqlunit\src\net\sourceforge\)
? standalonerunner/SqlunitRunner.java
cvs [server aborted]: "add" requires write access to the repository
Hi all,
Yeah, the <dynamicSql> tag would be a great addition to SQLUnit. SQLUnit advertises that it is for testing stored procedures. But it can also test SQL statements. Unfortunately, if you use it in that capacity, there is no coupling between the SQL statement under test declared in test suite, and the actual program that uses that SQL statement. If I am a careless programmer, I might adversely change the SQL statement in my program, and the previously written SQLUnit unit test would never show that I broke it unless I remember "ok, I changed a SQL statement in my program, now I need to replace all instances of it in the SQLUnit tests to see if it still works."
That problem doesn't happen if you do all your SQL in stored procedures or if SQLUnit gains the ability to extract SQL statements from the program under test such as it would with the <dynamicSql> tag. You can feel more confident in your unit tests with this tight coupling between SQL that the program uses and the SQL under test.
Hi Tim,
Good point about careless programmers :-).
Mario's finished writing the dynamicSql handler, it will be in the cvs repository shortly and will be released in the next release sometime late next week or early the week after. I will update this thread when the release happens or you can monitor the release so it automatically emails you.
It would be nice if you could run some test cases for the dynamicSql handler and let us know how it went.
Thanks
Sujit
Hi Mario,
I am running a little behind on putting your code into CVS. I discovered that you had refactored out stuff from SqlHandler to do the DynamicSqlHandler, which makes sense, but it means that I should also do some refactoring of my own to clean up my part of the code to match yours.
I guess thats the bad news. The good news is that the mock testing framework is now much improved and serves as a good basis for regression testing after refactoring, thus making it easy to do the refactoring.
Also if you could give me some documentation for these tags for the user's guide that would be cool too. Take a look at the user guide for other tags to see what they look like. I know you are pressed for time, so if you are not able to do this, I can just reverse-engineer something from the code.
Could you also send me the dtd entries for these new tags, looks like you may have missed them. Although I guess I can reverse engineer them from the code too.
Thanks
Sujit
Hi Mario,
Thanks for the DTD defs.
I got Jalopy to rewrite your code (not fully, it failed with array index out of bounds exception in one deeply bracket nested call) in one case, but mostly it seems to work fine. Anyway, there is an ant reformat task to invoke Jalopy (you must have it installed separately, it will not be part of the distribution because of jar file incompatibilities, and in any case, nobody would probably want to use it. The code conventions XML file is included in the doc/ directory if anybody wants to use it).
I have some questions. Can we change the following attributes to make it more concise?
className -> class
methodName -> method
isMethodStatic -> static
Also, I was looking at the constructorArgs and methodArgs, and it may be nice to have them defined as a set of args, like so:
<constructorArgs>
..<arg name="arg1" type="java.lang.Boolean" value="true" />
...
</constructorArgs>
The type could be an optional attribute which is not used for other occurences of the arg element. Also the value moves from being in a text field to being an attribute, although I am looking in both places in the ArgHandler. This may make the DTD a little too loose however and may lead to NPEs which will need to get caught in the handler. Same for <methodArgs>. This would allow us to use the same logic to pull things out of the arg tag that already exists. I have created a new handler for the arg element too.
If you are ok with these changes, it will become necessary to change your test cases. Let me know if its too inconvenient, and we will rethink this. I think it makes sense to do this in light of long term usage, because it helps us to reuse code and to keep our test XMLs concise.
I have checked in the code for the DynamicSqlHandler and the MethodInvokerHandler, and also folded in the changes for SqlHandler and TestHandler that you provided. I have renamed MethodInvoker.java to MethodInvocationUtils.java and have changed the code in it somewhat to make it more concise. I did not check in the MethodInvokerException class, instead I folded in the functionality to one of IErrorCodes for the SQLUnitException. The last is because I really created the IErrorCodes and the MessageFormat stuff to avoid having to do more than one exception class.
The classes compile, but I have not written any test cases for it yet. I plan to use some of your testing code to do this, and once the tests pass, we will release this. I will let you know when that happens, maybe sometime late next week.
-sujit
Received email from Mario, he is ok with the changes, so this needs to go in, will do this tonight.
Another issue was whether methodinvoker should report any exception in the method. Transcript follows:
---
> In the methodinvoker tag, I see that all you are doing is to run the
> method in the process() method. In all cases, you return an empty
> DatabaseResult object. Is the intent to only check that the method ran
> without throwing exceptions. If so, we may want to trap the exception,
> and if one is encountered, resetAsException() the DatabaseResult object.
--
and Mario confirmed that this was the intent, so I will make this change too.
Just fyi, if you are following this thread.
-sujit
BTW, this is out in version 3.8 now.
-sujit