[
https://issues.apache.org/jira/browse/OOZIE-1114?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13510207#comment-13510207
]
Robert Kanter commented on OOZIE-1114:
--------------------------------------
For the most part, I'm going to go with setup()+teardown() (setSystemProperty()
isn't always needed, most of the time, we just need to have the Services
running). The advantage is that it reduces the amount of duplicate code in
most cases, keeps the test methods cleaner, and makes it easier to write
additional tests without worrying about the Services (in most cases).
The only reason a test method would have to stop and start Services is if
setSystemProperty() needs to be used (or if its a test related to
starting/stopping Services). Even in this case, I think not using try-finally
is cleaner so you don't have the entire method's code in a block; so we can fix
the bug by reassigning the {{new Services()}} back to the class variable
({{services1}} in my pseudocode example). With that fix, tearDown() should
properly destroy Services.
I'm only going to do the try+finally solution when there's some reason to (e.g.
its already like that, there's only one test method, etc).
I'm glad that you're willing to help out. I've already started fixing some of
these and its not super difficult, so I don't think I need any help with the
coding. But, if you could review my patch when I post it, that would be really
helpful.
> Some tests don't use the Services singleton properly
> ----------------------------------------------------
>
> Key: OOZIE-1114
> URL: https://issues.apache.org/jira/browse/OOZIE-1114
> Project: Oozie
> Issue Type: Bug
> Affects Versions: trunk
> Reporter: Robert Kanter
> Assignee: Robert Kanter
> Fix For: trunk
>
>
> Some of the test classes keep a reference to the Services singleton as a
> class variable and then also re-create the Services in some of its test
> methods. This can cause it to not properly shut down all of the actual
> services, which can is causing (at least some of) the flakey test failures
> because some of those still running services interfere with some of the
> tests.
> The typical pattern where this issue is happening looks like this:
> {code}
> public class TestWhatever {
> private Services services1;
> setup() {
> services1 = new Services();
> services1.init();
> }
> testSomething() {
> Services.get().destroy(); // destroys services1's services
> Services services2 = new Services(); // services1 no longer
> points to the internal singleton
> services2.init();
> }
> tearDown() {
> services1.destroy(); // does not destroy the services started
> by services2, but does set the internal singleton to null so we've now lost
> the only remaining reference to those services
> }
> {code}
> You can see a concrete example of this by looking at
> TestStatusTransitService.testCoordStatusTransitServiceSuspendedWithError
> In general, we should make sure to properly destroy the Services at the end
> of each test
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
