I uploaded the patch for <sqlunit> task and you can see it[1]. Here is a detailed explanation on how I implemented it. I wanted to make as little changes to the sqlunit code base as possible, so my idea (which is of course subject to change) was to create a reporter that contains other reporters
1) The original SqlunitTask.java creates its single reporter using the this code:
reporter = ReporterFactory.getInstance(logformat, logfile);
Since I wanted to provide my own reporter implementation I isolated the above line in a new protected method in SqlunitTask.java:
protected IReporter createReporter() throws Exception {
return ReporterFactory.getInstance(logformat, logfile);
}
and the reporter that will be used in SqlunitTask.java is created by calling this method.
2) Next, I wrote a class IReporterList which implements IReporter and contains a List of IReporters and methods for its population. All methods that it inherits from IReporter are implemented by iterating over the List and calling the corresponding methods for each element (which is an IReporter).
3) Having that IReporterList, I created a class called XSqlunitTask that extends SqlunitTask and overrides createReporter() method from 1) to return an instance of IReporterList. It also contains some Ant stuff related with parsing the nested elements.
I also added some testcases for the task using the Ant testing mini-framework. If you find these testcases valuable you can check [2].
Also as I said earlier, I was looking for the fastest solution and not for the most elegant one and I feel we can improve it. For example, I thing here we can use a technique like Swing listeners. Actually, the sqlunit engine should fire events to its listeners like ECHO, or TEST_STARTED.
Looked through the code and it looks great! I will try to finish incorporating it into the main codebase by today and check it in tomorrow. Few things:
1) I am thinking along the lines of making another ReporterFactory.getInstance() method which takes a List object and returns a ReporterList (see 2). That would save us from having to subclass the SqlUnitTask.
2) I changed the name of IReporterList to ReporterList (since Ixxx is usually an interface in SQLUnit land), I realize you meant it to mean a List of IReporters.
I liked the EmptyReporter implementation and moved it to the reporters package and made it accessible by the logformat=none key.
I will also include your BuildFileTest implementation for XSqlUnit into the test/java directory so it can be reused and improved.
I had a question. I see that ReporterList is implementing IReporter with slightly different method signatures, eg setUpTest(Element) instead of setUpTest() for example. Since the class basically delegates to the underlying IReporters, I changed the signatures to use the current IReporter interface signatures instead. Not sure if I am missing something?
I agree using listeners would be a much better approach, but that is probably too big a change at this stage. I am starting a thread called "Thoughts on Refactoring" in which I will put this idea and also a few I have so we can discuss this.
Thanks again for the code.
-sujit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hello Sujit:
<quote>
1) I am thinking along the lines of making another ReporterFactory.getInstance() method which takes a List object and returns a ReporterList (see 2).
</quote>
Well, I think it should be declared to return IReporter, the real implementation of this IReporter should be ReporterList.
<quote>I see that ReporterList is implementing IReporter with slightly different method signatures, eg setUpTest(Element) instead of setUpTest() for example. Since the class basically delegates to the underlying IReporters, I changed the signatures to use the current IReporter interface signatures instead. Not sure if I am missing something? </quote>
Most probably I missed to provide some explanations. Let us make a little resume. Currently SQLUnit supports <classifiers> tag, on which our project relies heavily. By definition <classifiers> tag can accept arbitrary nested tags. We also provided a custom report handler to generate an XML report in a custom format, in which we needed the <classifier>'s nested tags as <severity> and <category>. For the time being sqlunit did not provide a way to pass these. I asked myself, if we have for example IReporter.runningTest() method why we need to pass it only the tag name of the test, the description and the index? Why don't we simply provide the whole *test* element as provided by the test file; thus we can access any information declared there and in particular the tag name, the description and the classifiers as well.
In this way of thinking consider IReporter.setUp() method. If we do some kind of elaborate setup which we want to report how can we do it currently. But if the chage the method to IReporter.setUp(Element elSetup) so that it can accept the <setup> tag it would make gathering information easier.
I know this change of interface method signatures means non-backward compatibity (and this was the only way to get my work done), so it is up to you to decide whether you like it and if you like I will send the patches. But perhaps before this we first discuss it in the Refactoring thread.
Regards
Ivan
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
The change in method signatures should not cause too much of a problem, we will need to fix the other implementations to work with them also in that case. The only way to do this currently is to make calls on the elSetup.getAttribute("id"), so that will mean an increase in the size of the code. Not such a big deal, since the code for the iIReporter implementations is just 2 classes, but will make future implementations able to access the Setup Element. If it adds value to the current implementaiton, we should do this in both the current and the refactored code.
Perhaps it would be cleaner if we passed in a Setup bean instead, with accessors to all its properties...
-sujit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I already changed the other reporters' implementation. The changes which I made in IReporter are the folllowing:
public interface IReporter {
void settingUpConnection(Element elConnection);
void setUp(Element elSetup);
void runningTest(Element elTest, int testIndex);
void finishedTest(Element elTest, long elapsed, boolean success);
void skippedTest(Element elTest, int testIndex, String reason);
void tearDown(Element elTeardown);
}
You can see, that with this change (while not being perfect) I have access to the node that "fired particular event" - for example test run, or test skipped, or tear down. Note that all the Elements passed to this methods are the exact elements as taken from the test file. Here is a sample call to one of this method, (which I make in SQLUnit.processDoc() method):
Element elTest = (Element) elTaskList.get(i);
if (!swappableTags.contains(elTest.getName())) {
continue;
}
testIndex++;
// if we are told to skip, we should and print a message
if (shouldSkip(elTest)) {
String reason =
SymbolTable.getValue(SymbolTable.SKIP_REASON);
I see your point of making these changes and I am ok with doing these. However, the reporters.zip file does not contain the changed IReporter implementations (TextReporter and CanooWebTestReporter). If you send them over, I can patch the code for these.
PS - also take a look at your gmail inbox for an email from me. Let me know what you think.
Thanks
Sujit
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Sujit,
I uploaded the patch for <sqlunit> task and you can see it[1]. Here is a detailed explanation on how I implemented it. I wanted to make as little changes to the sqlunit code base as possible, so my idea (which is of course subject to change) was to create a reporter that contains other reporters
1) The original SqlunitTask.java creates its single reporter using the this code:
reporter = ReporterFactory.getInstance(logformat, logfile);
Since I wanted to provide my own reporter implementation I isolated the above line in a new protected method in SqlunitTask.java:
protected IReporter createReporter() throws Exception {
return ReporterFactory.getInstance(logformat, logfile);
}
and the reporter that will be used in SqlunitTask.java is created by calling this method.
2) Next, I wrote a class IReporterList which implements IReporter and contains a List of IReporters and methods for its population. All methods that it inherits from IReporter are implemented by iterating over the List and calling the corresponding methods for each element (which is an IReporter).
3) Having that IReporterList, I created a class called XSqlunitTask that extends SqlunitTask and overrides createReporter() method from 1) to return an instance of IReporterList. It also contains some Ant stuff related with parsing the nested elements.
I also added some testcases for the task using the Ant testing mini-framework. If you find these testcases valuable you can check [2].
Also as I said earlier, I was looking for the fastest solution and not for the most elegant one and I feel we can improve it. For example, I thing here we can use a technique like Swing listeners. Actually, the sqlunit engine should fire events to its listeners like ECHO, or TEST_STARTED.
Regards
Ivan
[1]https://sourceforge.net/tracker/index.php?func=detail&aid=1242578&group_id=77832&atid=551359
[2]http://ant.apache.org/manual/tutorial-writing-tasks.html#TestingTasks
Hi Ivan,
Looked through the code and it looks great! I will try to finish incorporating it into the main codebase by today and check it in tomorrow. Few things:
1) I am thinking along the lines of making another ReporterFactory.getInstance() method which takes a List object and returns a ReporterList (see 2). That would save us from having to subclass the SqlUnitTask.
2) I changed the name of IReporterList to ReporterList (since Ixxx is usually an interface in SQLUnit land), I realize you meant it to mean a List of IReporters.
I liked the EmptyReporter implementation and moved it to the reporters package and made it accessible by the logformat=none key.
I will also include your BuildFileTest implementation for XSqlUnit into the test/java directory so it can be reused and improved.
I had a question. I see that ReporterList is implementing IReporter with slightly different method signatures, eg setUpTest(Element) instead of setUpTest() for example. Since the class basically delegates to the underlying IReporters, I changed the signatures to use the current IReporter interface signatures instead. Not sure if I am missing something?
I agree using listeners would be a much better approach, but that is probably too big a change at this stage. I am starting a thread called "Thoughts on Refactoring" in which I will put this idea and also a few I have so we can discuss this.
Thanks again for the code.
-sujit
Hello Sujit:
<quote>
1) I am thinking along the lines of making another ReporterFactory.getInstance() method which takes a List object and returns a ReporterList (see 2).
</quote>
Well, I think it should be declared to return IReporter, the real implementation of this IReporter should be ReporterList.
<quote>I see that ReporterList is implementing IReporter with slightly different method signatures, eg setUpTest(Element) instead of setUpTest() for example. Since the class basically delegates to the underlying IReporters, I changed the signatures to use the current IReporter interface signatures instead. Not sure if I am missing something? </quote>
Most probably I missed to provide some explanations. Let us make a little resume. Currently SQLUnit supports <classifiers> tag, on which our project relies heavily. By definition <classifiers> tag can accept arbitrary nested tags. We also provided a custom report handler to generate an XML report in a custom format, in which we needed the <classifier>'s nested tags as <severity> and <category>. For the time being sqlunit did not provide a way to pass these. I asked myself, if we have for example IReporter.runningTest() method why we need to pass it only the tag name of the test, the description and the index? Why don't we simply provide the whole *test* element as provided by the test file; thus we can access any information declared there and in particular the tag name, the description and the classifiers as well.
In this way of thinking consider IReporter.setUp() method. If we do some kind of elaborate setup which we want to report how can we do it currently. But if the chage the method to IReporter.setUp(Element elSetup) so that it can accept the <setup> tag it would make gathering information easier.
I know this change of interface method signatures means non-backward compatibity (and this was the only way to get my work done), so it is up to you to decide whether you like it and if you like I will send the patches. But perhaps before this we first discuss it in the Refactoring thread.
Regards
Ivan
Hi Ivan,
The change in method signatures should not cause too much of a problem, we will need to fix the other implementations to work with them also in that case. The only way to do this currently is to make calls on the elSetup.getAttribute("id"), so that will mean an increase in the size of the code. Not such a big deal, since the code for the iIReporter implementations is just 2 classes, but will make future implementations able to access the Setup Element. If it adds value to the current implementaiton, we should do this in both the current and the refactored code.
Perhaps it would be cleaner if we passed in a Setup bean instead, with accessors to all its properties...
-sujit
Sujit,
I already changed the other reporters' implementation. The changes which I made in IReporter are the folllowing:
public interface IReporter {
void settingUpConnection(Element elConnection);
void setUp(Element elSetup);
void runningTest(Element elTest, int testIndex);
void finishedTest(Element elTest, long elapsed, boolean success);
void skippedTest(Element elTest, int testIndex, String reason);
void tearDown(Element elTeardown);
}
You can see, that with this change (while not being perfect) I have access to the node that "fired particular event" - for example test run, or test skipped, or tear down. Note that all the Elements passed to this methods are the exact elements as taken from the test file. Here is a sample call to one of this method, (which I make in SQLUnit.processDoc() method):
Element elTest = (Element) elTaskList.get(i);
if (!swappableTags.contains(elTest.getName())) {
continue;
}
testIndex++;
// if we are told to skip, we should and print a message
if (shouldSkip(elTest)) {
String reason =
SymbolTable.getValue(SymbolTable.SKIP_REASON);
--->See me reporter.skippedTest(elTest, testIndex, reason == null ? "" : reason);
reporter.finishedTest(elTest, 0L, true);
continue;
}
It is also possible to pass beans instead of Element objects, but how can we transform them from Elements to beans?
Regards
Ivan
Hi Ivan,
I see your point of making these changes and I am ok with doing these. However, the reporters.zip file does not contain the changed IReporter implementations (TextReporter and CanooWebTestReporter). If you send them over, I can patch the code for these.
PS - also take a look at your gmail inbox for an email from me. Let me know what you think.
Thanks
Sujit