Fix issue 597 -- entity manager is properly removed even if close operation fails + unit test. #820
Conversation
…ration fails + unit test.
|
Thanks for the this! @dhanji, can you glance at it? You're most familiar with persist. @frido37 - - could you sign https://developers.google.com/open-source/cla/individual if you haven't already? (I'll try to set up automatic pull request checks for the CLA sometime today / this week.) |
|
@sameb -- I already signed the CLA (three weeks ago) |
|
Please sign CLA at https://developers.google.com/open-source/cla/individual |
|
Sorry about that auto-comment, it doesn't seem to have found your signature. Looks like the script works based off the email in your commit (see https://github.com/google/guice/pull/820.patch), which is different than the email you signed the CLA with (an @gmail.com). I'll figure out what to do. Sorry about the noise! |
| when(factory.createEntityManager()).thenReturn(entityManager); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
can you confirm this test is actually running? i didn't think we used junit4. might need to convert to junit3.
|
After confirmation that the test actually runs (or converting to junit3 if it isn't), this LGTM. I'd love @dhanji to pipe in, but no need to wait for him if he's not around. |
|
In the log of travis ci is see the following: Running com.google.inject.persist.jpa.JpaPersistServiceTest |
|
Surprising! For consistency with the rest of the package, would you be able to convert to JUnit3 style tests anyway? Looks like everything in the extension and the rest of Guice uses JUnit3-style tests. I'll merge in after that, thanks! |
|
Test is now converted to Junit3 - the change is in the second commit of this pull request. |
| @@ -0,0 +1,49 @@ | |||
| package com.google.inject.persist.jpa; | |||
There was a problem hiding this comment.
sorry i missed this before: please add the apache copyright header to this file. you can copy it from the top of the other ones, just fix up the year.
Fix issue 597 -- entity manager is properly removed even if close operation fails + unit test.
|
Thanks again for submitting this! |
|
Thanks for accepting! |
No description provided.