Derby
  1. Derby
  2. DERBY-5791

Replication tests should use BaseTestCase.execJavaCmd() to run local commands

    Details

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

      Description

      The replication tests invoke Runtime.exec() directly in order to spawn processes. The sub-processes that run on the same host as the main test process, should instead be started with the helper method BaseTestCase.execJavaCmd(). Having all the tests use the helper method would make it easier if we for example want to pass specific flags to all sub-processes created in a test run.

      Note that the replication tests also have code for starting processes on a remote host via ssh. BaseTestCase.execJavaCmd() cannot do that, so only local processes can be started with the helper method. When the replication tests run as part of suites.All, all the spawned processes run locally.

      1. d5791-1a.diff
        39 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Knut Anders Hatlen added a comment -

          Attaching a patch that makes ReplicationRun use
          BaseTestCase.execJavaCmd() to run local commands. The patch is built
          on top of the patch for DERBY-5783, so it can't be committed until
          that patch has been checked in.

          The patch rearranges the code so that Runtime.exec() is only called
          two places in the code (one for remote processes, and one for local
          processes), and replaces the call that creates the local processes
          with a call to BaseTestCase.execJavaCmd().

          The replication tests passed with the patch. Running the full
          regression test suite now.

          More details:

          • junit/BaseTestCase.java
          • Overloaded execJavaCmd() with a variant that allows specifying the
            working directory.
          • functionTests/tests/replicationTests/Utils.java
          • Added helper methods toStringArray() and splice() to replace some
            repeated operations in ReplicationRun.
          • Removed unused method NIOcopy.
          • functionTests/tests/replicationTests/ReplicationRun.java
          • Moved body of runUserCommand() method into runUserCommandLocally()
            to make it clearer that it was never used for remote processes.
          • Renamed runUserCommandInThread() runUserCommandInThreadLocally() to
            make it clearer that it was only for local processes, and to make
            method naming consistent with that of the methods that start remote
            processes.
          • Made runUserCommandInThreadLocally() and
            runUserCommandInThreadRemotely() wrap runUserCommandLocally() and
            runUserCommandRemotely(), respectively, to reduce the number of
            places that call exec(), and to reduce the total amount of code.
          • Made startServer() call runUserCommandInThreadLocally() and
            runUserCommandInThreadRemotely() instead of calling exec() directly,
            and made stopServer() call runUserCommandLocally() and
            runUserCommandRemotely() for the same reason.
          • Changed signatures of runUserCommand[InThread]Locally() to take the
            command as an array instead of a flat string, so that it could
            easily be passed on to BaseTestCase.execJavaCmd().
          • Made runUserCommandLocally() call BaseTestCase.execJavaCmd().
          Show
          Knut Anders Hatlen added a comment - Attaching a patch that makes ReplicationRun use BaseTestCase.execJavaCmd() to run local commands. The patch is built on top of the patch for DERBY-5783 , so it can't be committed until that patch has been checked in. The patch rearranges the code so that Runtime.exec() is only called two places in the code (one for remote processes, and one for local processes), and replaces the call that creates the local processes with a call to BaseTestCase.execJavaCmd(). The replication tests passed with the patch. Running the full regression test suite now. More details: junit/BaseTestCase.java Overloaded execJavaCmd() with a variant that allows specifying the working directory. functionTests/tests/replicationTests/Utils.java Added helper methods toStringArray() and splice() to replace some repeated operations in ReplicationRun. Removed unused method NIOcopy. functionTests/tests/replicationTests/ReplicationRun.java Moved body of runUserCommand() method into runUserCommandLocally() to make it clearer that it was never used for remote processes. Renamed runUserCommandInThread() runUserCommandInThreadLocally() to make it clearer that it was only for local processes, and to make method naming consistent with that of the methods that start remote processes. Made runUserCommandInThreadLocally() and runUserCommandInThreadRemotely() wrap runUserCommandLocally() and runUserCommandRemotely(), respectively, to reduce the number of places that call exec(), and to reduce the total amount of code. Made startServer() call runUserCommandInThreadLocally() and runUserCommandInThreadRemotely() instead of calling exec() directly, and made stopServer() call runUserCommandLocally() and runUserCommandRemotely() for the same reason. Changed signatures of runUserCommand [InThread] Locally() to take the command as an array instead of a flat string, so that it could easily be passed on to BaseTestCase.execJavaCmd(). Made runUserCommandLocally() call BaseTestCase.execJavaCmd().
          Hide
          Dag H. Wanvik added a comment -

          Good cleanup, approach is good. I did not vet all the details, but from 10000 feet, +1.
          If nothing else, diffstat gave me: 235 insertions, 437 deletions, so Derby gets slimmer

          Show
          Dag H. Wanvik added a comment - Good cleanup, approach is good. I did not vet all the details, but from 10000 feet, +1. If nothing else, diffstat gave me: 235 insertions , 437 deletions, so Derby gets slimmer
          Hide
          Knut Anders Hatlen added a comment -

          Thanks, Dag. Committed r1344191.

          Show
          Knut Anders Hatlen added a comment - Thanks, Dag. Committed r1344191.

            People

            • Assignee:
              Knut Anders Hatlen
              Reporter:
              Knut Anders Hatlen
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development