Re: svn commit: r1481814

[Oleg Kalnichevski <ol...@apache.org>]

On Mon, 2013-05-13 at 15:34 +0200, Stefano Bagnara wrote:
> 2013/5/13  <ol...@apache.org>:
> > Author: olegk
> > Date: Mon May 13 12:13:09 2013
> > New Revision: 1481814
> >
> > URL: http://svn.apache.org/r1481814
> > Log:
> > Refactored sample message based test suites; fixed frivolous usage of file based resources
> 
> Not sure why you call it "frivolous" and think it should be "fixed":
> can you elaborate?
> 
> That pattern is used in many of james products and is done that way in
> order to support full suite and single test execution (where for
> single test I mean a specific test for a single "file" resource) in
> Maven and Eclipse (via simple GUI right click) and also supported test
> execution for the dom module both using the resources from the
> filesystem or using the resources from the dependency jar
> (core-tests.jar includes file that are used as tests by dom module
> too).
> 
> I didn't check your refactor so I can't tell if it broke something or
> not: I hope to find the time soon to check if it works fine. In the
> mean time I wanted to explain why it was written that way and to ask
> you to explain the rationale behind the "frivolous" and if there are
> technical reasons to refactor it (e.g: did you find issues with
> running the tests in a specific environment or using specific tools?).
> 
> Stefano

Prior to refactoring the test suites in question made no attempts to
release file resources allocated by the tests. File backed resource
streams were never closed. To me this is quite frivolous, even for test
code. All other aspects of the test cases remained unchanged. I simply
factored out bits of bootstrapping code that was common to all sample
message based test cases.

Oleg    

Re: svn commit: r1481814

[Stefano Bagnara <ap...@bago.org>]

2013/5/13 Oleg Kalnichevski <ol...@apache.org>:
>> Make sense. I admit I've never cared about resource management in
>> tests (the JVM free them shutting down).
>
> Test cases can be run as a part of a larger test suite inside the same
> JVM and impact other tests. Sloppy resource management is simply a bad
> idea no matter the context.

Sure. I wrote "Make sense" ;-)

>> IIRC the testsuite declaration as a nested class and the code
>> duplication was needed in order to be able to run single tests in
>> Eclipse (and maybe in maven, but I don't remember the exact scope of
>> this statement). So I guess that extracting the common code could have
>> broken this feature (unfortunately I don't have eclipse on this
>> computer right now).
>> [...]

> Still works for me. I can still run the test for individual messages in
> Eclipse after refactoring.

Great! Maybe junit 4 is better, then, and we don't need that hacks anymore!!

Thank you,
Stefano

Re: svn commit: r1481814

[Oleg Kalnichevski <ol...@apache.org>]

On Mon, 2013-05-13 at 20:37 +0200, Stefano Bagnara wrote:
> 2013/5/13 Oleg Kalnichevski <ol...@apache.org>:
> > On Mon, 2013-05-13 at 15:34 +0200, Stefano Bagnara wrote:
> > Prior to refactoring the test suites in question made no attempts to
> > release file resources allocated by the tests. File backed resource
> > streams were never closed. To me this is quite frivolous, even for test
> > code.
> 
> Make sense. I admit I've never cared about resource management in
> tests (the JVM free them shutting down).
> 

Test cases can be run as a part of a larger test suite inside the same
JVM and impact other tests. Sloppy resource management is simply a bad
idea no matter the context.

> > All other aspects of the test cases remained unchanged. I simply
> > factored out bits of bootstrapping code that was common to all sample
> > message based test cases.
> 
> IIRC the testsuite declaration as a nested class and the code
> duplication was needed in order to be able to run single tests in
> Eclipse (and maybe in maven, but I don't remember the exact scope of
> this statement). So I guess that extracting the common code could have
> broken this feature (unfortunately I don't have eclipse on this
> computer right now).
> 
> I always found very useful to be able to test a single msg file from
> the eclipse testrunner (and other junit testrunners) while I'm
> debugging/fixing an issue in the parser or similar things, but I'm not
> working anymore on mime4j so feel free to do what you prefer. 
> 
> At most I will fix it again if I will have to work again on mime4j.
> 
> Stefano

Still works for me. I can still run the test for individual messages in
Eclipse after refactoring. 

Oleg 

Re: svn commit: r1481814

[Stefano Bagnara <ap...@bago.org>]

2013/5/13 Oleg Kalnichevski <ol...@apache.org>:
> On Mon, 2013-05-13 at 15:34 +0200, Stefano Bagnara wrote:
> Prior to refactoring the test suites in question made no attempts to
> release file resources allocated by the tests. File backed resource
> streams were never closed. To me this is quite frivolous, even for test
> code.

Make sense. I admit I've never cared about resource management in
tests (the JVM free them shutting down).

> All other aspects of the test cases remained unchanged. I simply
> factored out bits of bootstrapping code that was common to all sample
> message based test cases.

IIRC the testsuite declaration as a nested class and the code
duplication was needed in order to be able to run single tests in
Eclipse (and maybe in maven, but I don't remember the exact scope of
this statement). So I guess that extracting the common code could have
broken this feature (unfortunately I don't have eclipse on this
computer right now).

I always found very useful to be able to test a single msg file from
the eclipse testrunner (and other junit testrunners) while I'm
debugging/fixing an issue in the parser or similar things, but I'm not
working anymore on mime4j so feel free to do what you prefer.

At most I will fix it again if I will have to work again on mime4j.

Stefano