Derby
  1. Derby
  2. DERBY-5504

SecureServerTest, Driver40UnbootedTest and replication tests not prepared for space in java.home

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.9.1.0
    • Fix Version/s: 10.9.1.0
    • Component/s: Test
    • Labels:
      None

      Description

      SecureServerTest, Driver40UnbootedTest and the replication tests fork JVMs by calling Runtime.exec() methods that take the command as a String parameter, not a String[]. This causes problems like the ones seen in DERBY-5490 when there's a space in $

      {java.home}

      .

      The tests should be rewritten to use the helper method BaseTestCase.execJavaCmd(), which handles this case correctly.

      1. d5504-2a.diff
        16 kB
        Knut Anders Hatlen
      2. d5504-1a.diff
        8 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Knut Anders Hatlen added a comment -

          Here's a patch that fixes the issue for SecureServerTest and Driver40UnbootedTest. It removes the explicit calls to Runtime.exec() and instead uses BaseTestCase.execJavaCmd(). Multiple command line arguments passed as a space-separated string have been changed into string arrays to prevent the problems with spaces in path names.

          Show
          Knut Anders Hatlen added a comment - Here's a patch that fixes the issue for SecureServerTest and Driver40UnbootedTest. It removes the explicit calls to Runtime.exec() and instead uses BaseTestCase.execJavaCmd(). Multiple command line arguments passed as a space-separated string have been changed into string arrays to prevent the problems with spaces in path names.
          Hide
          Dag H. Wanvik added a comment -

          Patch looks good to me, +1. Do you plan to do likewise for the replication tests in a follow-up patch?

          Show
          Dag H. Wanvik added a comment - Patch looks good to me, +1. Do you plan to do likewise for the replication tests in a follow-up patch?
          Hide
          Knut Anders Hatlen added a comment -

          Thanks for looking at the patch, Dag.

          Committed revision 1203113.

          Yes, I intend to do something similar for the replication tests, but it looks as if more changes are needed for them, especially for the code that supports running tests against remote hosts. I don't think anyone has run the replication tests against remote hosts for a very long time, and given the instabilities we see when running the tests locally, I suspect there are even more problems. Maybe it's easier just to remove that code. There hasn't been much interest in improving the replication tests lately, and simplifying the tests by removing the code that supports remote testing might in fact make it easier to maintain the parts of the replication tests that we actually use in the daily testing.

          Show
          Knut Anders Hatlen added a comment - Thanks for looking at the patch, Dag. Committed revision 1203113. Yes, I intend to do something similar for the replication tests, but it looks as if more changes are needed for them, especially for the code that supports running tests against remote hosts. I don't think anyone has run the replication tests against remote hosts for a very long time, and given the instabilities we see when running the tests locally, I suspect there are even more problems. Maybe it's easier just to remove that code. There hasn't been much interest in improving the replication tests lately, and simplifying the tests by removing the code that supports remote testing might in fact make it easier to maintain the parts of the replication tests that we actually use in the daily testing.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a patch for the replication tests (d5504-2a.diff).

          I took the path of least resistance and only changed the exec() calls that are used when running local tests. Cleaning up the calls for distributed tests could be done later if someone starts running those tests. If someone thinks we should remove that code, that could be done in a separate JIRA issue.

          The replication test suite ran cleanly in my environment when the java executable was located in a directory with a space in its name.

          Show
          Knut Anders Hatlen added a comment - Attaching a patch for the replication tests (d5504-2a.diff). I took the path of least resistance and only changed the exec() calls that are used when running local tests. Cleaning up the calls for distributed tests could be done later if someone starts running those tests. If someone thinks we should remove that code, that could be done in a separate JIRA issue. The replication test suite ran cleanly in my environment when the java executable was located in a directory with a space in its name.
          Hide
          Knut Anders Hatlen added a comment -

          Committed revision 1203605.

          Resolving the issue.

          Show
          Knut Anders Hatlen added a comment - Committed revision 1203605. Resolving the issue.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development