Hi Marina,
Answers inline:
Marina Vatkina wrote:
>Tom, Team,
>
>I'm in the process of fixing this issue
>(https://glassfish.dev.java.net/issues/show_bug.cgi?id=1226),
>and have several related questions:
>
>1. In some tests the 'catch (...Exception e)' results in em.close() calls, while
>in other it doesn't. Should I add em.close() in at least those cases that I
>change? Or should I try to fix all of them?
>
>
In general, I think it is ok to add em.close() in most cases we catch an
exception. I, however, think you should do decide on a case by case
basis whether it is appropriate to the test you are modifying.
It is probably a good idea to eventually ensure all EntityManager's
close when we are done using them in the testing, but that may be a
bigger task then just fixing them where exceptions are thrown. Unless
you are seeing some errors as a result of these EMs not closing, I
suggest simply filing an Enhancement.
>2. There are cases, when the exception is completely ignored (i.e. no 'fail' or
>anything to that extent being done inside the catch block). Is it OK to ignore
>those exceptions?
>
>
I think this is another issue that you need to decide on a case by case
basis. I'd be happy to comment on any specific examples you have
questions about.
>3. This is a more generic question: let's come with the rules that we can
>document in the readme file, to make sure we don't need to do this exercise
>again :(.
>
>
I think a published set of rules for the tests is a great idea -
although maybe not necessarily simple to put together in a way that
keeps everyone in the community happy due to coding styles and such. In
general, however, I think this is one of several pieces of collateral
that we need to provide to ease the process of contributing to the
project. Do you know of any examples of such standards currently
available to the glassfish community. Ideally, we could use an already
published one as a basis for the one we eventually settle on.
-Tom
>thanks,
>-marina
>
>