You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Knut Anders Hatlen (JIRA)" <ji...@apache.org> on 2012/04/25 14:46:51 UTC

[jira] [Created] (DERBY-5726) Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses

Knut Anders Hatlen created DERBY-5726:
-----------------------------------------

             Summary: Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses
                 Key: DERBY-5726
                 URL: https://issues.apache.org/jira/browse/DERBY-5726
             Project: Derby
          Issue Type: Improvement
          Components: Test
    Affects Versions: 10.9.0.0
            Reporter: Knut Anders Hatlen
            Assignee: Knut Anders Hatlen
            Priority: Minor


Many of the classes that extend BaseJDBCTestCase and override the tearDown() method, forget to call super.tearDown(), and thereby prevent resources from being freed after completion. We should add a mechanism that enforces the correct behaviour.

If we were starting from scratch, we might have made BaseJDBCTestCase.tearDown() final and added a new overridable method that was called from BaseJDBCTestCase.tearDown() before it freed the statements and connections. Then there would be no way to prevent BaseJDBCTestCase.tearDown() from running in the subclasses. That would however require us to change all existing overrides of BaseJDBCTestCase.tearDown() (current count: 131), which would be a chunk of work.

I'd rather suggest that we add an override of runBare() in BaseJDBCTestCase that asserts that the connection has been cleared out when a test case has completed successfully. Something like this:

    public void runBare() throws Throwable {
        super.runBare();
        // It's quite common to forget to call super.tearDown() when
        // overriding tearDown() in sub-classes.
        assertNull(
            "Connection should be null by now. " +
            "Missing call to super.tearDown()?", conn);
    }

Then it would still be possible to forget to call super.tearDown(), but it would be discovered when trying to run the test.

Adding the above method to BaseJDBCTestCase and running InternationalConnectTest gave this result:

.....F.F....F
Time: 5,748
There were 3 failures:
1) testDriverManagerConnect(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
	at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
	at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
	at junit.extensions.TestSetup.run(TestSetup.java:25)
	at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
2) testBoundaries(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
(...)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DERBY-5726) Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses

Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DERBY-5726?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13266463#comment-13266463 ] 

Knut Anders Hatlen commented on DERBY-5726:
-------------------------------------------

Hi Kathey,

I think this cleanup is important because memory leaks in the test framework may prevent us from discovering real memory leaks in the product, and also because if we keep letting new leaks into the tests, the amount of memory required for running the tests will just keep growing.

For the record, with all the latest patches, suites.All now passes with 64MB heap in my environment if I disable the following test cases:

TriggerTest
  - testBlobInTriggerTable

Derby2017LayerATest
  - cs_FailedStreamInsertCharBufferBoundaries
  - cs_StreamInsertCharBufferBoundary

StatementJdbc30Test
  - xtestMaxOpenStatementsWithQueryTimeout

ClobReclamationTest
  - testBlobLinkedListReclamationOnRollback

When running the tests standalone, TriggerTest requires 165MB heap to pass, and the other three tests pass with 85MB.
                
> Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses
> --------------------------------------------------------------------------------------------
>
>                 Key: DERBY-5726
>                 URL: https://issues.apache.org/jira/browse/DERBY-5726
>             Project: Derby
>          Issue Type: Improvement
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>         Attachments: d5726-1a.diff
>
>
> Many of the classes that extend BaseJDBCTestCase and override the tearDown() method, forget to call super.tearDown(), and thereby prevent resources from being freed after completion. We should add a mechanism that enforces the correct behaviour.
> If we were starting from scratch, we might have made BaseJDBCTestCase.tearDown() final and added a new overridable method that was called from BaseJDBCTestCase.tearDown() before it freed the statements and connections. Then there would be no way to prevent BaseJDBCTestCase.tearDown() from running in the subclasses. That would however require us to change all existing overrides of BaseJDBCTestCase.tearDown() (current count: 131), which would be a chunk of work.
> I'd rather suggest that we add an override of runBare() in BaseJDBCTestCase that asserts that the connection has been cleared out when a test case has completed successfully. Something like this:
>     public void runBare() throws Throwable {
>         super.runBare();
>         // It's quite common to forget to call super.tearDown() when
>         // overriding tearDown() in sub-classes.
>         assertNull(
>             "Connection should be null by now. " +
>             "Missing call to super.tearDown()?", conn);
>     }
> Then it would still be possible to forget to call super.tearDown(), but it would be discovered when trying to run the test.
> Adding the above method to BaseJDBCTestCase and running InternationalConnectTest gave this result:
> .....F.F....F
> Time: 5,748
> There were 3 failures:
> 1) testDriverManagerConnect(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> 	at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
> 	at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
> 	at junit.extensions.TestSetup.run(TestSetup.java:25)
> 	at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
> 2) testBoundaries(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> (...)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DERBY-5726) Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses

Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DERBY-5726?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13274537#comment-13274537 ] 

Knut Anders Hatlen commented on DERBY-5726:
-------------------------------------------

If someone wants to run the tests with limited amount of memory, it might make more sense to have a way to disable known memory-hungry tests, as forking the tests would still require much memory in the sub-process. I'm not sure we'd want to introduce a hard limit on 64MB for running suites.All, though, as that was a number I picked rather arbitrarily to smoke out the leaks.
                
> Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses
> --------------------------------------------------------------------------------------------
>
>                 Key: DERBY-5726
>                 URL: https://issues.apache.org/jira/browse/DERBY-5726
>             Project: Derby
>          Issue Type: Improvement
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>             Fix For: 10.9.0.0
>
>         Attachments: d5726-1a.diff
>
>
> Many of the classes that extend BaseJDBCTestCase and override the tearDown() method, forget to call super.tearDown(), and thereby prevent resources from being freed after completion. We should add a mechanism that enforces the correct behaviour.
> If we were starting from scratch, we might have made BaseJDBCTestCase.tearDown() final and added a new overridable method that was called from BaseJDBCTestCase.tearDown() before it freed the statements and connections. Then there would be no way to prevent BaseJDBCTestCase.tearDown() from running in the subclasses. That would however require us to change all existing overrides of BaseJDBCTestCase.tearDown() (current count: 131), which would be a chunk of work.
> I'd rather suggest that we add an override of runBare() in BaseJDBCTestCase that asserts that the connection has been cleared out when a test case has completed successfully. Something like this:
>     public void runBare() throws Throwable {
>         super.runBare();
>         // It's quite common to forget to call super.tearDown() when
>         // overriding tearDown() in sub-classes.
>         assertNull(
>             "Connection should be null by now. " +
>             "Missing call to super.tearDown()?", conn);
>     }
> Then it would still be possible to forget to call super.tearDown(), but it would be discovered when trying to run the test.
> Adding the above method to BaseJDBCTestCase and running InternationalConnectTest gave this result:
> .....F.F....F
> Time: 5,748
> There were 3 failures:
> 1) testDriverManagerConnect(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> 	at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
> 	at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
> 	at junit.extensions.TestSetup.run(TestSetup.java:25)
> 	at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
> 2) testBoundaries(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> (...)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (DERBY-5726) Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses

Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-5726?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Knut Anders Hatlen updated DERBY-5726:
--------------------------------------

    Issue & fix info: Patch Available
    
> Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses
> --------------------------------------------------------------------------------------------
>
>                 Key: DERBY-5726
>                 URL: https://issues.apache.org/jira/browse/DERBY-5726
>             Project: Derby
>          Issue Type: Improvement
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>         Attachments: d5726-1a.diff
>
>
> Many of the classes that extend BaseJDBCTestCase and override the tearDown() method, forget to call super.tearDown(), and thereby prevent resources from being freed after completion. We should add a mechanism that enforces the correct behaviour.
> If we were starting from scratch, we might have made BaseJDBCTestCase.tearDown() final and added a new overridable method that was called from BaseJDBCTestCase.tearDown() before it freed the statements and connections. Then there would be no way to prevent BaseJDBCTestCase.tearDown() from running in the subclasses. That would however require us to change all existing overrides of BaseJDBCTestCase.tearDown() (current count: 131), which would be a chunk of work.
> I'd rather suggest that we add an override of runBare() in BaseJDBCTestCase that asserts that the connection has been cleared out when a test case has completed successfully. Something like this:
>     public void runBare() throws Throwable {
>         super.runBare();
>         // It's quite common to forget to call super.tearDown() when
>         // overriding tearDown() in sub-classes.
>         assertNull(
>             "Connection should be null by now. " +
>             "Missing call to super.tearDown()?", conn);
>     }
> Then it would still be possible to forget to call super.tearDown(), but it would be discovered when trying to run the test.
> Adding the above method to BaseJDBCTestCase and running InternationalConnectTest gave this result:
> .....F.F....F
> Time: 5,748
> There were 3 failures:
> 1) testDriverManagerConnect(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> 	at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
> 	at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
> 	at junit.extensions.TestSetup.run(TestSetup.java:25)
> 	at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
> 2) testBoundaries(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> (...)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DERBY-5726) Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses

Posted by "Kathey Marsden (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DERBY-5726?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13265015#comment-13265015 ] 

Kathey Marsden commented on DERBY-5726:
---------------------------------------

This sounds like a good idea to ensure teardown gets called and things get cleaned up.  I remember sometime back doing a test cleanup cleanup effort, but can't exactly remember my motivation. It might have been making sure things run in the default heap or maybe it had something to do with permgen space.  Why is the cleanup important?



                
> Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses
> --------------------------------------------------------------------------------------------
>
>                 Key: DERBY-5726
>                 URL: https://issues.apache.org/jira/browse/DERBY-5726
>             Project: Derby
>          Issue Type: Improvement
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>         Attachments: d5726-1a.diff
>
>
> Many of the classes that extend BaseJDBCTestCase and override the tearDown() method, forget to call super.tearDown(), and thereby prevent resources from being freed after completion. We should add a mechanism that enforces the correct behaviour.
> If we were starting from scratch, we might have made BaseJDBCTestCase.tearDown() final and added a new overridable method that was called from BaseJDBCTestCase.tearDown() before it freed the statements and connections. Then there would be no way to prevent BaseJDBCTestCase.tearDown() from running in the subclasses. That would however require us to change all existing overrides of BaseJDBCTestCase.tearDown() (current count: 131), which would be a chunk of work.
> I'd rather suggest that we add an override of runBare() in BaseJDBCTestCase that asserts that the connection has been cleared out when a test case has completed successfully. Something like this:
>     public void runBare() throws Throwable {
>         super.runBare();
>         // It's quite common to forget to call super.tearDown() when
>         // overriding tearDown() in sub-classes.
>         assertNull(
>             "Connection should be null by now. " +
>             "Missing call to super.tearDown()?", conn);
>     }
> Then it would still be possible to forget to call super.tearDown(), but it would be discovered when trying to run the test.
> Adding the above method to BaseJDBCTestCase and running InternationalConnectTest gave this result:
> .....F.F....F
> Time: 5,748
> There were 3 failures:
> 1) testDriverManagerConnect(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> 	at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
> 	at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
> 	at junit.extensions.TestSetup.run(TestSetup.java:25)
> 	at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
> 2) testBoundaries(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> (...)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (DERBY-5726) Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses

Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-5726?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Knut Anders Hatlen updated DERBY-5726:
--------------------------------------

    Attachment: d5726-1a.diff

Attaching a patch that adds the assert to BaseJDBCTestCase.runBare(), makes runBare() final to prevent sub-classes from being able to bypass the check, and add an overridable method (runBareOverridable) that is called from the final runBare() method.

I successfully ran suites.All and perf.basic._Suite (which is the suite that contains the only tests that override BaseJDBCTestCase.runBare()).
                
> Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses
> --------------------------------------------------------------------------------------------
>
>                 Key: DERBY-5726
>                 URL: https://issues.apache.org/jira/browse/DERBY-5726
>             Project: Derby
>          Issue Type: Improvement
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>         Attachments: d5726-1a.diff
>
>
> Many of the classes that extend BaseJDBCTestCase and override the tearDown() method, forget to call super.tearDown(), and thereby prevent resources from being freed after completion. We should add a mechanism that enforces the correct behaviour.
> If we were starting from scratch, we might have made BaseJDBCTestCase.tearDown() final and added a new overridable method that was called from BaseJDBCTestCase.tearDown() before it freed the statements and connections. Then there would be no way to prevent BaseJDBCTestCase.tearDown() from running in the subclasses. That would however require us to change all existing overrides of BaseJDBCTestCase.tearDown() (current count: 131), which would be a chunk of work.
> I'd rather suggest that we add an override of runBare() in BaseJDBCTestCase that asserts that the connection has been cleared out when a test case has completed successfully. Something like this:
>     public void runBare() throws Throwable {
>         super.runBare();
>         // It's quite common to forget to call super.tearDown() when
>         // overriding tearDown() in sub-classes.
>         assertNull(
>             "Connection should be null by now. " +
>             "Missing call to super.tearDown()?", conn);
>     }
> Then it would still be possible to forget to call super.tearDown(), but it would be discovered when trying to run the test.
> Adding the above method to BaseJDBCTestCase and running InternationalConnectTest gave this result:
> .....F.F....F
> Time: 5,748
> There were 3 failures:
> 1) testDriverManagerConnect(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> 	at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
> 	at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
> 	at junit.extensions.TestSetup.run(TestSetup.java:25)
> 	at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
> 2) testBoundaries(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> (...)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DERBY-5726) Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses

Posted by "Rick Hillegas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DERBY-5726?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13261603#comment-13261603 ] 

Rick Hillegas commented on DERBY-5726:
--------------------------------------

Hi Knut,

That sounds like a good idea. Maybe you can apply your first suggestion to the runBare() solution, making runBare() final and having it call an overridable method. Shouldn't be too messy. I only see overrides of runBare() in a handful of classes.

Thanks,
-Rick
                
> Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses
> --------------------------------------------------------------------------------------------
>
>                 Key: DERBY-5726
>                 URL: https://issues.apache.org/jira/browse/DERBY-5726
>             Project: Derby
>          Issue Type: Improvement
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>
> Many of the classes that extend BaseJDBCTestCase and override the tearDown() method, forget to call super.tearDown(), and thereby prevent resources from being freed after completion. We should add a mechanism that enforces the correct behaviour.
> If we were starting from scratch, we might have made BaseJDBCTestCase.tearDown() final and added a new overridable method that was called from BaseJDBCTestCase.tearDown() before it freed the statements and connections. Then there would be no way to prevent BaseJDBCTestCase.tearDown() from running in the subclasses. That would however require us to change all existing overrides of BaseJDBCTestCase.tearDown() (current count: 131), which would be a chunk of work.
> I'd rather suggest that we add an override of runBare() in BaseJDBCTestCase that asserts that the connection has been cleared out when a test case has completed successfully. Something like this:
>     public void runBare() throws Throwable {
>         super.runBare();
>         // It's quite common to forget to call super.tearDown() when
>         // overriding tearDown() in sub-classes.
>         assertNull(
>             "Connection should be null by now. " +
>             "Missing call to super.tearDown()?", conn);
>     }
> Then it would still be possible to forget to call super.tearDown(), but it would be discovered when trying to run the test.
> Adding the above method to BaseJDBCTestCase and running InternationalConnectTest gave this result:
> .....F.F....F
> Time: 5,748
> There were 3 failures:
> 1) testDriverManagerConnect(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> 	at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
> 	at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
> 	at junit.extensions.TestSetup.run(TestSetup.java:25)
> 	at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
> 2) testBoundaries(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> (...)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DERBY-5726) Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses

Posted by "Dag H. Wanvik (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DERBY-5726?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13273576#comment-13273576 ] 

Dag H. Wanvik commented on DERBY-5726:
--------------------------------------

Would it make sense to run these memory hungry tests in forked VMs, so we can have them as part of the regression tests, but keep the main VM to 64MB requirement you have squeezed it down to?

                
> Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses
> --------------------------------------------------------------------------------------------
>
>                 Key: DERBY-5726
>                 URL: https://issues.apache.org/jira/browse/DERBY-5726
>             Project: Derby
>          Issue Type: Improvement
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>             Fix For: 10.9.0.0
>
>         Attachments: d5726-1a.diff
>
>
> Many of the classes that extend BaseJDBCTestCase and override the tearDown() method, forget to call super.tearDown(), and thereby prevent resources from being freed after completion. We should add a mechanism that enforces the correct behaviour.
> If we were starting from scratch, we might have made BaseJDBCTestCase.tearDown() final and added a new overridable method that was called from BaseJDBCTestCase.tearDown() before it freed the statements and connections. Then there would be no way to prevent BaseJDBCTestCase.tearDown() from running in the subclasses. That would however require us to change all existing overrides of BaseJDBCTestCase.tearDown() (current count: 131), which would be a chunk of work.
> I'd rather suggest that we add an override of runBare() in BaseJDBCTestCase that asserts that the connection has been cleared out when a test case has completed successfully. Something like this:
>     public void runBare() throws Throwable {
>         super.runBare();
>         // It's quite common to forget to call super.tearDown() when
>         // overriding tearDown() in sub-classes.
>         assertNull(
>             "Connection should be null by now. " +
>             "Missing call to super.tearDown()?", conn);
>     }
> Then it would still be possible to forget to call super.tearDown(), but it would be discovered when trying to run the test.
> Adding the above method to BaseJDBCTestCase and running InternationalConnectTest gave this result:
> .....F.F....F
> Time: 5,748
> There were 3 failures:
> 1) testDriverManagerConnect(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> 	at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
> 	at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
> 	at junit.extensions.TestSetup.run(TestSetup.java:25)
> 	at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
> 2) testBoundaries(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> (...)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DERBY-5726) Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses

Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DERBY-5726?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13261847#comment-13261847 ] 

Knut Anders Hatlen commented on DERBY-5726:
-------------------------------------------

There were no failures in suites.All when running with the above mentioned runBare() method in BaseJDBCTestCase, as well as the uncommitted patches attached to DERBY-5721, DERBY-5722, DERBY-5723 and DERBY-5725. So at least it looks like we've smoked out most of these problems now.

I'll see if I can modify the patch as Rick suggested. There is actually just one override of BaseJDBCTestCase.runBare(), and that's in JDBCPerfTestCase. The other runBare() overrides are in direct subclasses of BaseTestCase, which doesn't have any connections or statements to close.
                
> Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses
> --------------------------------------------------------------------------------------------
>
>                 Key: DERBY-5726
>                 URL: https://issues.apache.org/jira/browse/DERBY-5726
>             Project: Derby
>          Issue Type: Improvement
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>
> Many of the classes that extend BaseJDBCTestCase and override the tearDown() method, forget to call super.tearDown(), and thereby prevent resources from being freed after completion. We should add a mechanism that enforces the correct behaviour.
> If we were starting from scratch, we might have made BaseJDBCTestCase.tearDown() final and added a new overridable method that was called from BaseJDBCTestCase.tearDown() before it freed the statements and connections. Then there would be no way to prevent BaseJDBCTestCase.tearDown() from running in the subclasses. That would however require us to change all existing overrides of BaseJDBCTestCase.tearDown() (current count: 131), which would be a chunk of work.
> I'd rather suggest that we add an override of runBare() in BaseJDBCTestCase that asserts that the connection has been cleared out when a test case has completed successfully. Something like this:
>     public void runBare() throws Throwable {
>         super.runBare();
>         // It's quite common to forget to call super.tearDown() when
>         // overriding tearDown() in sub-classes.
>         assertNull(
>             "Connection should be null by now. " +
>             "Missing call to super.tearDown()?", conn);
>     }
> Then it would still be possible to forget to call super.tearDown(), but it would be discovered when trying to run the test.
> Adding the above method to BaseJDBCTestCase and running InternationalConnectTest gave this result:
> .....F.F....F
> Time: 5,748
> There were 3 failures:
> 1) testDriverManagerConnect(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> 	at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
> 	at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
> 	at junit.extensions.TestSetup.run(TestSetup.java:25)
> 	at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
> 2) testBoundaries(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> (...)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Resolved] (DERBY-5726) Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses

Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DERBY-5726?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Knut Anders Hatlen resolved DERBY-5726.
---------------------------------------

          Resolution: Fixed
       Fix Version/s: 10.9.0.0
    Issue & fix info:   (was: Patch Available)

Committed revision 1332967.
                
> Make it more difficult to forget calling super.tearDown() from BaseJDBCTestCase's subclasses
> --------------------------------------------------------------------------------------------
>
>                 Key: DERBY-5726
>                 URL: https://issues.apache.org/jira/browse/DERBY-5726
>             Project: Derby
>          Issue Type: Improvement
>          Components: Test
>    Affects Versions: 10.9.0.0
>            Reporter: Knut Anders Hatlen
>            Assignee: Knut Anders Hatlen
>            Priority: Minor
>             Fix For: 10.9.0.0
>
>         Attachments: d5726-1a.diff
>
>
> Many of the classes that extend BaseJDBCTestCase and override the tearDown() method, forget to call super.tearDown(), and thereby prevent resources from being freed after completion. We should add a mechanism that enforces the correct behaviour.
> If we were starting from scratch, we might have made BaseJDBCTestCase.tearDown() final and added a new overridable method that was called from BaseJDBCTestCase.tearDown() before it freed the statements and connections. Then there would be no way to prevent BaseJDBCTestCase.tearDown() from running in the subclasses. That would however require us to change all existing overrides of BaseJDBCTestCase.tearDown() (current count: 131), which would be a chunk of work.
> I'd rather suggest that we add an override of runBare() in BaseJDBCTestCase that asserts that the connection has been cleared out when a test case has completed successfully. Something like this:
>     public void runBare() throws Throwable {
>         super.runBare();
>         // It's quite common to forget to call super.tearDown() when
>         // overriding tearDown() in sub-classes.
>         assertNull(
>             "Connection should be null by now. " +
>             "Missing call to super.tearDown()?", conn);
>     }
> Then it would still be possible to forget to call super.tearDown(), but it would be discovered when trying to run the test.
> Adding the above method to BaseJDBCTestCase and running InternationalConnectTest gave this result:
> .....F.F....F
> Time: 5,748
> There were 3 failures:
> 1) testDriverManagerConnect(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> 	at junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
> 	at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
> 	at junit.extensions.TestSetup.run(TestSetup.java:25)
> 	at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
> 2) testBoundaries(org.apache.derbyTesting.functionTests.tests.jdbcapi.InternationalConnectTest)junit.framework.AssertionFailedError: Connection should be null by now. Missing call to super.tearDown()?
> 	at org.apache.derbyTesting.junit.BaseJDBCTestCase.runBare(BaseJDBCTestCase.java:431)
> (...)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira