Derby
  1. Derby
  2. DERBY-2054

Rewrite 'derbynet/SuicideOfStreaming' to a JUnit test

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.3.1.4
    • Fix Version/s: 10.3.1.4
    • Component/s: Test
    • Labels:
      None

      Description

      The test 'derbynet/SuicideOfStreaming' should be rewritten to a JUnit test more in line with our newly created test system.
      It is one of the last tests still being run from the deprecated 'tests/junitTests/' directory, through a wrapper class.

      1. derby-2054-preview.diff
        15 kB
        Kristian Waagan
      2. derby-2054-1b.diff
        0.7 kB
        Kristian Waagan
      3. derby-2054-1a.stat
        1 kB
        Kristian Waagan
      4. derby-2054-1a.diff
        22 kB
        Kristian Waagan

        Issue Links

          Activity

          Kristian Waagan created issue -
          Kristian Waagan made changes -
          Field Original Value New Value
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Kristian Waagan added a comment -

          'derby-2054-preview.diff' is the first attempt at rewriting SuicideOfStreaming.
          It is kind of special, since it uses a property inside a sanity manager block in the Derby code to generate an exception on the server. There is also the matter of only returning a suite with tests if Derby is built in sane mode (see the suite-method).

          Please have a look at the suggested test, and read the comments in the code. The diffs of the two old files implementing this test are included for reference.

          I will post another patch shortly, which will add a _Suite in the derbynet directory and fix a few other small things.

          Show
          Kristian Waagan added a comment - 'derby-2054-preview.diff' is the first attempt at rewriting SuicideOfStreaming. It is kind of special, since it uses a property inside a sanity manager block in the Derby code to generate an exception on the server. There is also the matter of only returning a suite with tests if Derby is built in sane mode (see the suite-method). Please have a look at the suggested test, and read the comments in the code. The diffs of the two old files implementing this test are included for reference. I will post another patch shortly, which will add a _Suite in the derbynet directory and fix a few other small things.
          Kristian Waagan made changes -
          Attachment derby-2054-preview.diff [ 12344591 ]
          Hide
          Tomohito Nakayama added a comment -

          Hello.

          I remember OwnServerTests_app.properties which was needed only for OwnServerTests.java.
          I think it also should be removed in your patch.

          http://svn.apache.org/repos/asf/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/junitTests/derbyNet/OwnServerTests_app.properties

          Show
          Tomohito Nakayama added a comment - Hello. I remember OwnServerTests_app.properties which was needed only for OwnServerTests.java. I think it also should be removed in your patch. http://svn.apache.org/repos/asf/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/junitTests/derbyNet/OwnServerTests_app.properties
          Hide
          Kristian Waagan added a comment -

          Thanks Tomohito. My patch removes several files.

          'derby-2054-1a.diff' rewrites the SuicideOfStreaming tests to a JUnit test and: Deletes files no longer needed, adds SqlException and ByteArrayCombinerStreamTest to derbynet/_Suite, update suite files for the old harness, rewrite DDMWriter so that it requires the debug property to be true (not only set).

          Patch ready for review/commit.

          Show
          Kristian Waagan added a comment - Thanks Tomohito. My patch removes several files. 'derby-2054-1a.diff' rewrites the SuicideOfStreaming tests to a JUnit test and: Deletes files no longer needed, adds SqlException and ByteArrayCombinerStreamTest to derbynet/_Suite, update suite files for the old harness, rewrite DDMWriter so that it requires the debug property to be true (not only set). Patch ready for review/commit.
          Kristian Waagan made changes -
          Attachment derby-2054-1a.stat [ 12344753 ]
          Attachment derby-2054-1a.diff [ 12344752 ]
          Kristian Waagan made changes -
          Derby Info [Patch Available]
          Hide
          Tomohito Nakayama added a comment -

          I'm not familiar with newly created test system at all.
          Then this comment may be missed ...

          System property of derby.debug.suicideOfLayerBStreaming is effective on code of NetworkServer.
          Then System property must be set for process of NetworkServer.

          Reading the code setUp(),
          I found both of code to call JDBC interface to execute sql which would be at NetworkClient and
          code to set System property of derby.debug.suicideOfLayerBStreaming in a same method.

          In newly created test system, does NetworkServer and NetworkClient run on a same process ?
          I remember that they are different in old test system ...

          Show
          Tomohito Nakayama added a comment - I'm not familiar with newly created test system at all. Then this comment may be missed ... System property of derby.debug.suicideOfLayerBStreaming is effective on code of NetworkServer. Then System property must be set for process of NetworkServer. Reading the code setUp(), I found both of code to call JDBC interface to execute sql which would be at NetworkClient and code to set System property of derby.debug.suicideOfLayerBStreaming in a same method. In newly created test system, does NetworkServer and NetworkClient run on a same process ? I remember that they are different in old test system ...
          Hide
          Kristian Waagan added a comment -

          You are correct, currently the NetworkServer and the client code run in the same process (see NetworkServerTestSetup).

          When/if we extend the test system to allow for running the NetworkServer in a separate process (possibly on a remote host), the test must be rewritten/modified. I wrote a comment regarding this in the class JavaDoc.

          Show
          Kristian Waagan added a comment - You are correct, currently the NetworkServer and the client code run in the same process (see NetworkServerTestSetup). When/if we extend the test system to allow for running the NetworkServer in a separate process (possibly on a remote host), the test must be rewritten/modified. I wrote a comment regarding this in the class JavaDoc.
          Hide
          Daniel John Debrunner added a comment -

          An alternative is that the test could be skipped if running in a separate process/machine.
          Would need to see how much extra value having a particular test running with a remote server would be if there were many other tests running remote.

          Show
          Daniel John Debrunner added a comment - An alternative is that the test could be skipped if running in a separate process/machine. Would need to see how much extra value having a particular test running with a remote server would be if there were many other tests running remote.
          Hide
          Tomohito Nakayama added a comment -

          I think this may be very very basic question ...

          I executed derbyall suite with this patch.
          But could not found the name of this test in derbyall_pass.txt (nor derbyall_fail.txt nor derbyall_skip.txt) ...

          Is newly created test system executed in derbyall suite ...?

          Show
          Tomohito Nakayama added a comment - I think this may be very very basic question ... I executed derbyall suite with this patch. But could not found the name of this test in derbyall_pass.txt (nor derbyall_fail.txt nor derbyall_skip.txt) ... Is newly created test system executed in derbyall suite ...?
          Hide
          Tomohito Nakayama added a comment -

          According to http://wiki.apache.org/db-derby/DerbyJUnitTesting,
          I executed org.apache.derbyTesting.functionTests.suites.All and
          confirmed that SuicideOfStreamingTest was passed.

          Show
          Tomohito Nakayama added a comment - According to http://wiki.apache.org/db-derby/DerbyJUnitTesting , I executed org.apache.derbyTesting.functionTests.suites.All and confirmed that SuicideOfStreamingTest was passed.
          Hide
          Kristian Waagan added a comment -

          Committed 'derby-2054-1a.diff' to trunk with revision 474803.

          Show
          Kristian Waagan added a comment - Committed 'derby-2054-1a.diff' to trunk with revision 474803.
          Kristian Waagan made changes -
          Fix Version/s 10.3.0.0 [ 12310800 ]
          Status In Progress [ 3 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Derby Info [Patch Available]
          Hide
          Daniel John Debrunner added a comment -

          SuicideOfStreamingTest.tearDown() does not call super.tearDown() which may be causing other later tests to fail due to the extra time needed to start the network server. However adding super.tearDown() causes authentication errors in later tests and in the tearDown method.

          Show
          Daniel John Debrunner added a comment - SuicideOfStreamingTest.tearDown() does not call super.tearDown() which may be causing other later tests to fail due to the extra time needed to start the network server. However adding super.tearDown() causes authentication errors in later tests and in the tearDown method.
          Hide
          Kristian Waagan added a comment -

          Dan, could you please elaborate on the problems you see with the test?

          As far as I see, the only thing that would happen if super.tearDown is called, is that a rollback is issued and the the connection is closed.
          I do not understand how not doing this would affect other tests with regard of "the extra time needed to start the network server".

          I think that due to the other problems I had with converting the test, I forgot to close the connection manually. This would have to be done inside a try-block, where any exceptions are ignored. This is because setting the debug property causes havoc in the communication between the client and the server. My intention was that the network server I use is to be used only for this test, since simply unsetting the debug property is not enough for it to recover.

          Also, what authentication errors are you seeing?

          Thanks,

          Show
          Kristian Waagan added a comment - Dan, could you please elaborate on the problems you see with the test? As far as I see, the only thing that would happen if super.tearDown is called, is that a rollback is issued and the the connection is closed. I do not understand how not doing this would affect other tests with regard of "the extra time needed to start the network server". I think that due to the other problems I had with converting the test, I forgot to close the connection manually. This would have to be done inside a try-block, where any exceptions are ignored. This is because setting the debug property causes havoc in the communication between the client and the server. My intention was that the network server I use is to be used only for this test, since simply unsetting the debug property is not enough for it to recover. Also, what authentication errors are you seeing? Thanks,
          Hide
          Daniel John Debrunner added a comment -

          Having spent some time tracking down the network server timeouts (see DERBY-1966) I think those are probably unrelated.
          Can you try caling super.tearDown() and see if you get the authentication failures?

          Show
          Daniel John Debrunner added a comment - Having spent some time tracking down the network server timeouts (see DERBY-1966 ) I think those are probably unrelated. Can you try caling super.tearDown() and see if you get the authentication failures?
          Kristian Waagan made changes -
          Attachment derby-2054-1b.diff [ 12348727 ]
          Hide
          Kristian Waagan added a comment -

          Committed 'derby-2054-1b.diff' with revision 495213, which calls super.tearDown().
          I can't see any authentication failures when running suites.All (Java 1.5 and Java SE 6) anymore.
          Will close the issue soon if no problems arise in the nightly tests.

          Show
          Kristian Waagan added a comment - Committed 'derby-2054-1b.diff' with revision 495213, which calls super.tearDown(). I can't see any authentication failures when running suites.All (Java 1.5 and Java SE 6) anymore. Will close the issue soon if no problems arise in the nightly tests.
          Kristian Waagan made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Dag H. Wanvik made changes -
          Issue Type Test [ 6 ] Improvement [ 4 ]
          Knut Anders Hatlen made changes -
          Link This issue is duplicated by DERBY-1347 [ DERBY-1347 ]
          Kristian Waagan made changes -
          Link This issue is part of DERBY-1888 [ DERBY-1888 ]
          Gavin made changes -
          Workflow jira [ 12388816 ] Default workflow, editable Closed status [ 12801487 ]

            People

            • Assignee:
              Kristian Waagan
              Reporter:
              Kristian Waagan
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development