Qpid
  1. Qpid
  2. QPID-3513

Avoid use of shell script clean-dir during test cycle

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.13
    • Component/s: Java Tests
    • Labels:
      None

      Description

      Current the test harness provided by QpidBrokerTestCase utilises a UNIX shell script clean-dir to clean the broker's work
      directory. This approach is less than ideal: spawning a separate process is relativity slow and this delays the test cycle, it presents
      a barrier to running the test suite on Windows, and the approach is very sensitive to spaces within directory names (something that is preventing our use of Jenkins' matrix feature).

      This change will remove the use of the shellscript and replace with the existing FileUtils.delete utility method.

        Issue Links

          Activity

          Hide
          Keith Wall added a comment -

          Patch to resolve issue

          Show
          Keith Wall added a comment - Patch to resolve issue
          Hide
          Robbie Gemmell added a comment - - edited

          Looks good to me, have a couple of tiny notes relating to the cpp.testprofile file:

          Spurious licence header change:

          -# Unless required by applicable law or agreed to in writing,
          +# unless required by applicable law or agreed to in writing,
          

          The @PORT and data elements of the new data-dir path should be swapped to make it use the same default value for the path as it did before. Could probably use $

          {build}

          /work in there instead also:

          -broker.command=${broker.executable} -p @PORT --data-dir ${build.data}/@PORT -t --auth no --no-module-dir ${broker.modules} ${broker.args}
          +broker.command=${broker.executable} -p @PORT --data-dir ${project.root}/build/work/@PORT/data -t --auth no --no-module-dir ${broker.modules} ${broker.args}
          
          Show
          Robbie Gemmell added a comment - - edited Looks good to me, have a couple of tiny notes relating to the cpp.testprofile file: Spurious licence header change: -# Unless required by applicable law or agreed to in writing, +# unless required by applicable law or agreed to in writing, The @PORT and data elements of the new data-dir path should be swapped to make it use the same default value for the path as it did before. Could probably use $ {build} /work in there instead also: -broker.command=${broker.executable} -p @PORT --data-dir ${build.data}/@PORT -t --auth no --no-module-dir ${broker.modules} ${broker.args} +broker.command=${broker.executable} -p @PORT --data-dir ${project.root}/build/work/@PORT/data -t --auth no --no-module-dir ${broker.modules} ${broker.args}
          Hide
          Keith Wall added a comment -

          Hi Robbie,

          As discussed.. I resolved the licence typo. The cpp broker was using an area beneath build/scratch/data rather than the build/work. It seemed better to standardise on build/work so both Cpp and Java Brokers utilise the same directory and QpidBrokerTestCase can be agnostic.

          I've committed the change.

          Show
          Keith Wall added a comment - Hi Robbie, As discussed.. I resolved the licence typo. The cpp broker was using an area beneath build/scratch/data rather than the build/work. It seemed better to standardise on build/work so both Cpp and Java Brokers utilise the same directory and QpidBrokerTestCase can be agnostic. I've committed the change.
          Hide
          Robbie Gemmell added a comment -

          Changes look good to me

          Show
          Robbie Gemmell added a comment - Changes look good to me

            People

            • Assignee:
              Robbie Gemmell
              Reporter:
              Keith Wall
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development