You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cactus-dev@jakarta.apache.org by Kazuhito SUGURI <su...@lab.ntt.co.jp> on 2004/03/15 13:22:23 UTC
[PATCH] Re: Cactus/test name is null
Hi,
In article <40...@rogers.com>,
Fri, 05 Mar 2004 17:50:19 -0500,
"J. B. Rainsberger" <jb...@rogers.com> wrote:
jbrains> It points out a potential defect in Cactus, though. When the test name
jbrains> is null, JUnit usually pukes; but in this case, it looks like a test
jbrains> failure. Vince... maybe it's worth changing Cactus so that it blows up
jbrains> the same way that JUnit does when the test name is null.
[snip: my first proposal]
In article <0d...@vma>,
Fri, 12 Mar 2004 09:43:28 +0100,
"Vincent Massol" <vm...@pivolis.com> wrote:
vmassol> What would be excellent is if you could also provide some test cases
vmassol> proving that it works. You can either reuse an existing test case class
vmassol> if it makes sense or create a new one. That will enrich our test suite
vmassol> and not lower our test coverage! :-)
vmassol>
vmassol> I've had a brief look at the patch. It looks good. Maybe, as a second
vmassol> step, it would be good to factorize both your check and the ones that
vmassol> already exist into a single place. ATM, there are lots of checks in the
vmassol> ClientTestCaseCaller.callGeneric*Method(...)
vmassol>
vmassol> It would be excellent if all the checks could be gathered in one place.
Patchs, new java files and test cases are appending.
These may have too many changes for one time.
Key points are:
1. added test case name checks at AbstractCactusTestCase#runBare()
[test case: TestNoNameTestCase ]
2. collected some test case check logics into TestCaseImplementChecker
[test case: TestTestCaseImplementChecker]
3. refactored ClientTestCaseCaller#callGeneric*Method
to use TestCaseImplementChecker
[modified existing test case: AbstractTestAbstractTestCase,
some expected messages were changed]
4. introduced TestCaseImplementError as a subclass of AssertionFailedError
---- appending files ----
New java files to be located under framework/src/share/org/apache/cactus/util:
TestCaseImplementChecker.java
TestCaseImplementError.java
New test case to be located under framework/src/test/org/apache/cactus:
TestNoNameTestCase.java
New test case to be located under framework/src/test/org/apache/cactus/util:
TestTestCaseImplementChecker.java
patch.AbstractCactusTestCase.1.7:
a patch against AbstractTestCase.java Rev.1.7
patch.ClientTestCaseCaller.1.3:
a patch against ClientTestCaseCaller.java Rev.1.3
patch.AbstractTestAbstractTestCase.1.7:
a patch against test/share/org/apache/cactus/AbstractTestAbstractTestCase.java Rev.1.7
patch.TestAll.1.19
a patch against test/share/org/apache/cactus/TestAll.java Rev.1.19
patch.util.TestAll.1.4
a patch against test/share/org/apache/cactus/util/TestAll.java Rev.1.4
Regards,
----
Kazuhito SUGURI
mailto:suguri.kazuhito@lab.ntt.co.jp
RE: [PATCH] Re: Cactus/test name is null
Posted by Vincent Massol <vm...@pivolis.com>.
Wow! Thanks Kazuhito!
Great patch. I've just applied it in CVS.
Some very small comments:
- the date in the license at the top of file needs to contain the time
span during which the file has existed. Thus for a new file, it's 2004
(and not 2001-2004). For files that were done last year and that are
modified this year for example, it must be 2002-2004, etc.
- I haven't looked at the tests yet but I have the feeling they can be
improved. Not sure though, it's just a feeling ATM. I'll have a closer
look in the coming days.
- I think we can continue further your refactoring. There are still some
checks in ClientTestCaseCaller that verify the validity of Cactus test
cases. For example:
if (!getCurrentTestName().startsWith(TEST_METHOD_PREFIX))
{
throw new RuntimeException("bad name ["
+ getCurrentTestName()
+ "]. It should start with ["
+ TEST_METHOD_PREFIX + "].");
}
This could probably be also refactored in your TestCaseImplementChecker
I think.
Kazuhito this is really great work. You've been providing several high
quality patches. Thanks a lot for all your help :-)
Feel free to scratch any other itch you may have!
Thanks
-Vincent
> -----Original Message-----
> From: Kazuhito SUGURI [mailto:suguri.kazuhito@lab.ntt.co.jp]
> Sent: 15 March 2004 13:22
> To: cactus-dev@jakarta.apache.org
> Subject: [PATCH] Re: Cactus/test name is null
>
> Hi,
>
> In article <40...@rogers.com>,
> Fri, 05 Mar 2004 17:50:19 -0500,
> "J. B. Rainsberger" <jb...@rogers.com> wrote:
> jbrains> It points out a potential defect in Cactus, though. When the
test
> name
> jbrains> is null, JUnit usually pukes; but in this case, it looks like
a
> test
> jbrains> failure. Vince... maybe it's worth changing Cactus so that it
> blows up
> jbrains> the same way that JUnit does when the test name is null.
>
> [snip: my first proposal]
>
> In article <0d...@vma>,
> Fri, 12 Mar 2004 09:43:28 +0100,
> "Vincent Massol" <vm...@pivolis.com> wrote:
> vmassol> What would be excellent is if you could also provide some
test
> cases
> vmassol> proving that it works. You can either reuse an existing test
case
> class
> vmassol> if it makes sense or create a new one. That will enrich our
test
> suite
> vmassol> and not lower our test coverage! :-)
> vmassol>
> vmassol> I've had a brief look at the patch. It looks good. Maybe, as
a
> second
> vmassol> step, it would be good to factorize both your check and the
ones
> that
> vmassol> already exist into a single place. ATM, there are lots of
checks
> in the
> vmassol> ClientTestCaseCaller.callGeneric*Method(...)
> vmassol>
> vmassol> It would be excellent if all the checks could be gathered in
one
> place.
> Patchs, new java files and test cases are appending.
>
> These may have too many changes for one time.
> Key points are:
> 1. added test case name checks at AbstractCactusTestCase#runBare()
> [test case: TestNoNameTestCase ]
> 2. collected some test case check logics into TestCaseImplementChecker
> [test case: TestTestCaseImplementChecker]
> 3. refactored ClientTestCaseCaller#callGeneric*Method
> to use TestCaseImplementChecker
> [modified existing test case: AbstractTestAbstractTestCase,
> some expected messages were changed]
> 4. introduced TestCaseImplementError as a subclass of
AssertionFailedError
>
>
> ---- appending files ----
> New java files to be located under
> framework/src/share/org/apache/cactus/util:
> TestCaseImplementChecker.java
> TestCaseImplementError.java
>
> New test case to be located under
framework/src/test/org/apache/cactus:
> TestNoNameTestCase.java
>
> New test case to be located under
> framework/src/test/org/apache/cactus/util:
> TestTestCaseImplementChecker.java
>
> patch.AbstractCactusTestCase.1.7:
> a patch against AbstractTestCase.java Rev.1.7
>
> patch.ClientTestCaseCaller.1.3:
> a patch against ClientTestCaseCaller.java Rev.1.3
>
> patch.AbstractTestAbstractTestCase.1.7:
> a patch against
> test/share/org/apache/cactus/AbstractTestAbstractTestCase.java Rev.1.7
>
> patch.TestAll.1.19
> a patch against test/share/org/apache/cactus/TestAll.java
Rev.1.19
>
> patch.util.TestAll.1.4
> a patch against test/share/org/apache/cactus/util/TestAll.java
> Rev.1.4
>
>
> Regards,
> ----
> Kazuhito SUGURI
> mailto:suguri.kazuhito@lab.ntt.co.jp
---------------------------------------------------------------------
To unsubscribe, e-mail: cactus-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: cactus-dev-help@jakarta.apache.org