Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.6.1.0
    • Fix Version/s: 10.6.1.0
    • Component/s: Test
    • Labels:
      None
    • Urgency:
      Normal
    • Issue & fix info:
      Patch Available

      Description

      The goal is to make the port used for suites.All configurable through a system property passed on to the JVM.

      1. DERBY-4217-fixRegex.patch
        0.9 kB
        Tiago R. Espinha
      2. DERBY-4217-basePort.patch
        11 kB
        Tiago R. Espinha
      3. DERBY-4217-basePort.patch
        12 kB
        Tiago R. Espinha
      4. DERBY-4217-basePort.patch
        11 kB
        Tiago R. Espinha
      5. DERBY-4217-basePort.patch
        13 kB
        Tiago R. Espinha
      6. DERBY-4217-dtap.patch
        3 kB
        Tiago R. Espinha
      7. DERBY-4217-dtap.patch
        43 kB
        Tiago R. Espinha
      8. DERBY-4217-dtp.patch
        39 kB
        Tiago R. Espinha
      9. DERBY-4217-dtp.patch
        41 kB
        Tiago R. Espinha
      10. ErrorLog_suitesAll_bound.tgz
        9.77 MB
        Tiago R. Espinha
      11. DERBY-4217-dtp.patch
        39 kB
        Tiago R. Espinha
      12. DERBY-4217-dtp.patch
        39 kB
        Tiago R. Espinha
      13. DERBY-4217-dtp.patch
        33 kB
        Tiago R. Espinha
      14. DERBY-4217-ij.patch
        18 kB
        Tiago R. Espinha
      15. DERBY-4217-ij.patch
        18 kB
        Tiago R. Espinha
      16. DERBY-4217-ij.patch
        17 kB
        Tiago R. Espinha
      17. DERBY-4217-ij.stat
        0.6 kB
        Tiago R. Espinha
      18. DERBY-4217-ij.patch
        17 kB
        Tiago R. Espinha
      19. DERBY-4217-ij.stat
        0.6 kB
        Tiago R. Espinha
      20. DERBY-4217-ij.patch
        17 kB
        Tiago R. Espinha
      21. DERBY-4217.stat
        0.8 kB
        Tiago R. Espinha
      22. DERBY-4217.patch
        33 kB
        Tiago R. Espinha
      23. ReproNetworkServerControl.java
        7 kB
        Tiago R. Espinha
      24. DERBY-4217.patch
        2 kB
        Tiago R. Espinha
      25. DERBY-4217.patch
        1 kB
        Tiago R. Espinha
      26. DERBY-4217.stat
        0.1 kB
        Tiago R. Espinha
      27. DERBY-4217.patch
        1 kB
        Tiago R. Espinha

        Issue Links

          Activity

          Hide
          Tiago R. Espinha added a comment -

          Attaching a possible patch for this. It's a simple approach but I think it works.

          I'll be running the suites.All overnight.

          Show
          Tiago R. Espinha added a comment - Attaching a possible patch for this. It's a simple approach but I think it works. I'll be running the suites.All overnight.
          Hide
          Kathey Marsden added a comment -

          We should probably call the property derby.tests.port instead of just port. Otherwise the patch looks ok on visual inspection. I didn't try it. You should also update http://wiki.apache.org/db-derby/DerbyJUnitTesting after this has been changed.

          Show
          Kathey Marsden added a comment - We should probably call the property derby.tests.port instead of just port. Otherwise the patch looks ok on visual inspection. I didn't try it. You should also update http://wiki.apache.org/db-derby/DerbyJUnitTesting after this has been changed.
          Hide
          Tiago R. Espinha added a comment -

          Kathey is right, for a moment there I forgot our standard for property names. Fixed that and optimised the code so that only one call is made to retrieve the property. Minor stuff.

          Show
          Tiago R. Espinha added a comment - Kathey is right, for a moment there I forgot our standard for property names. Fixed that and optimised the code so that only one call is made to retrieve the property. Minor stuff.
          Hide
          Tiago R. Espinha added a comment -

          First I have to leave a heads-up. Do NOT commit these patches into the trunk as they are still being tested.

          I have run into something weird that I could use an extra pair of eyes on.

          Basically, this Repro contains a test that passes if the property -Dderby.tests.port=1527 but fails with pretty much any other port.

          In essence, by running:
          java -Dderby.tests.port=1527 junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.derbynet.ReproNetworkServerControl

          the test passes. However, if you run it like:
          java -Dderby.tests.port=9980 junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.derbynet.ReproNetworkServerControl

          the test will fail with an assertion error.

          Please keep in mind that you have to apply this latest patch to get the described behavior.

          Show
          Tiago R. Espinha added a comment - First I have to leave a heads-up. Do NOT commit these patches into the trunk as they are still being tested. I have run into something weird that I could use an extra pair of eyes on. Basically, this Repro contains a test that passes if the property -Dderby.tests.port=1527 but fails with pretty much any other port. In essence, by running: java -Dderby.tests.port=1527 junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.derbynet.ReproNetworkServerControl the test passes. However, if you run it like: java -Dderby.tests.port=9980 junit.textui.TestRunner org.apache.derbyTesting.functionTests.tests.derbynet.ReproNetworkServerControl the test will fail with an assertion error. Please keep in mind that you have to apply this latest patch to get the described behavior.
          Hide
          Kathey Marsden added a comment -

          I think in testMaxThreads_0 the NetworkServerControl command needs to specify -p <portNumber>
          Currently it is just using the default port.

          Show
          Kathey Marsden added a comment - I think in testMaxThreads_0 the NetworkServerControl command needs to specify -p <portNumber> Currently it is just using the default port.
          Hide
          Tiago R. Espinha added a comment -

          Thank you Kathey, you were indeed right!

          Show
          Tiago R. Espinha added a comment - Thank you Kathey, you were indeed right!
          Hide
          Tiago R. Espinha added a comment -

          I have hit a speedbump with this issue. I have taken Kathey's advice and now those tests are no longer a problem. However there's a new issue.

          The issue in question has to do with the ScriptTests. Basically, some of the .sql files that are ran as part of a ScriptTestCase, have the port hardcoded right on the .sql file (e.g. connect 'jdbc:derby://localhost:1527/testij;create=true' USER 'dbadmin' PASSWORD 'dbadmin').

          I have been thinking about how to overcome this, but the solution I had thought of is far from ideal and I have had issues trying to implement it. My idea was to change the port references to a '<port>' placeholder, and then replace the <port> occurrences with the actual port. This however is complicated.

          If I was to replace the ports on the actual files, then the first test run would be okay, but the consequent ones would not have the '<port>' placeholder; instead, they'd have the port from the previous run. Not good.

          My idea was to make a temporary copy of the file, replace it on that file, and have the test run upon that sql file. This would also have to be done to the canon file (.out). When the test is done, the file is deleted.

          I started implementing this last solution, but I'm running into so many problems (with permissions to copy the files, etc) that I thought I'd gather some opinions here first. Maybe there's an easier way to go about this.

          Any thoughts?

          Show
          Tiago R. Espinha added a comment - I have hit a speedbump with this issue. I have taken Kathey's advice and now those tests are no longer a problem. However there's a new issue. The issue in question has to do with the ScriptTests. Basically, some of the .sql files that are ran as part of a ScriptTestCase, have the port hardcoded right on the .sql file (e.g. connect 'jdbc:derby://localhost:1527/testij;create=true' USER 'dbadmin' PASSWORD 'dbadmin'). I have been thinking about how to overcome this, but the solution I had thought of is far from ideal and I have had issues trying to implement it. My idea was to change the port references to a '<port>' placeholder, and then replace the <port> occurrences with the actual port. This however is complicated. If I was to replace the ports on the actual files, then the first test run would be okay, but the consequent ones would not have the '<port>' placeholder; instead, they'd have the port from the previous run. Not good. My idea was to make a temporary copy of the file, replace it on that file, and have the test run upon that sql file. This would also have to be done to the canon file (.out). When the test is done, the file is deleted. I started implementing this last solution, but I'm running into so many problems (with permissions to copy the files, etc) that I thought I'd gather some opinions here first. Maybe there's an easier way to go about this. Any thoughts?
          Hide
          Tiago R. Espinha added a comment -

          I forgot to mention, but I should. The test that is currently failing and that I'm looking at is the NetIjTest under org.apache.derbyTesting.functionTests.tests.derbynet which loads the testclientij.sql .

          Show
          Tiago R. Espinha added a comment - I forgot to mention, but I should. The test that is currently failing and that I'm looking at is the NetIjTest under org.apache.derbyTesting.functionTests.tests.derbynet which loads the testclientij.sql .
          Hide
          Kathey Marsden added a comment -

          It seemed to work for me with ij to specify:
          java -Dij.protocol=jdbc:derby://localhost:1528/ org.apache.derby.tools.ij

          and then in ij, just specify the database and connection attributes. If you do this, the calling program could control the port and you could change the script to not include it.

          Would that work?

          Show
          Kathey Marsden added a comment - It seemed to work for me with ij to specify: java -Dij.protocol=jdbc:derby://localhost:1528/ org.apache.derby.tools.ij and then in ij, just specify the database and connection attributes. If you do this, the calling program could control the port and you could change the script to not include it. Would that work?
          Hide
          Tiago R. Espinha added a comment -

          Just uploading the patch and stat for future reference. This patch is NOT to be commited to the source.

          This version contains some deep changes to the ScriptTestCase, the NetIjTest and its resource files, and some other auxiliary classes. The changes should however be non-intrusive for existent test cases.

          Show
          Tiago R. Espinha added a comment - Just uploading the patch and stat for future reference. This patch is NOT to be commited to the source. This version contains some deep changes to the ScriptTestCase, the NetIjTest and its resource files, and some other auxiliary classes. The changes should however be non-intrusive for existent test cases.
          Hide
          Tiago R. Espinha added a comment -

          I'm marking this issue as Patch Available.

          However, please note:

          • This issue will require a lot of tangling with the code, therefore Kathey suggested cumulative patches rather than a big fat patch.
          • As it is, this patch effectively allows the use of derby.tests.port on some of the tests. Still, it does not break anything for any of the tests if no system property for the port is specified. Kathey refered that this was the requirement for the patch to be commited. After running suites.All with this patch on, I can say that this requirement is met.
          • Please do not close the issue, as I will still be doing additional work to fix the remaining failing tests when a port is indeed specified.
          Show
          Tiago R. Espinha added a comment - I'm marking this issue as Patch Available. However, please note: This issue will require a lot of tangling with the code, therefore Kathey suggested cumulative patches rather than a big fat patch. As it is, this patch effectively allows the use of derby.tests.port on some of the tests. Still, it does not break anything for any of the tests if no system property for the port is specified. Kathey refered that this was the requirement for the patch to be commited. After running suites.All with this patch on, I can say that this requirement is met. Please do not close the issue, as I will still be doing additional work to fix the remaining failing tests when a port is indeed specified.
          Hide
          Kathey Marsden added a comment -

          Hi Tiago. Thank you for the patch. Just a few comments.

          For NetworkServerControlApi changes, I think it would be worthwile to have a TestConfiguration.newNetworkServerControl() method that uses the default host and port in case we want to fix up the tests to allow remote testing.

          In general, outside of TestConfiguration, I think a TestConfiguration.getPort() method would be better than referring to DEFUALT_PORT directly.

          I think it would be good if the runScript method that doesn't take the loadSystemProperties parameter just called the new one with loadSystemProperties set to false instead of having duplicate code. Then you can also remove the old getUtilMain method from Main.java

          Show
          Kathey Marsden added a comment - Hi Tiago. Thank you for the patch. Just a few comments. For NetworkServerControlApi changes, I think it would be worthwile to have a TestConfiguration.newNetworkServerControl() method that uses the default host and port in case we want to fix up the tests to allow remote testing. In general, outside of TestConfiguration, I think a TestConfiguration.getPort() method would be better than referring to DEFUALT_PORT directly. I think it would be good if the runScript method that doesn't take the loadSystemProperties parameter just called the new one with loadSystemProperties set to false instead of having duplicate code. Then you can also remove the old getUtilMain method from Main.java
          Hide
          Tiago R. Espinha added a comment -

          Hello Kathey.

          I'm not sure I get the point of having such a method on the TestConfiguration. I believe that having a way of overriding the derby.tests.port property might put in jeopardy the whole purpose of this issue. This issue was originally created as part of DERBY-4090, and if there is the possibility of overriding this property, then concurrent testing on the same machine might have unexpected results (i.e. two tests, one from JUnit and another one from Canon, trying to bind on the 1527 port).

          As to the second point, I agree that it would be more correct to have a getPort() on the TestConfiguration and actually such method does exist already. However, this method returns the property port that is stored within TestConfiguration. In some cases, this port is specified through methods such as existingServerSuite(Class testClass, boolean cleanDB, String hostName, int portNumber) or clientExistingServerSuite(...).

          I'm just not sure if it's wise to mess with the port property that already exists in TestConfiguration.

          By tapping into the DEFAULT_PORT property, I think I managed to make the change more transparent for the majority of the tests, although it still remains to be seen how we'd handle tests that specify their own port, as Dag pointed out on DERBY-4090.

          On that last point, I agree. It was my bad #

          Show
          Tiago R. Espinha added a comment - Hello Kathey. I'm not sure I get the point of having such a method on the TestConfiguration. I believe that having a way of overriding the derby.tests.port property might put in jeopardy the whole purpose of this issue. This issue was originally created as part of DERBY-4090 , and if there is the possibility of overriding this property, then concurrent testing on the same machine might have unexpected results (i.e. two tests, one from JUnit and another one from Canon, trying to bind on the 1527 port). As to the second point, I agree that it would be more correct to have a getPort() on the TestConfiguration and actually such method does exist already. However, this method returns the property port that is stored within TestConfiguration. In some cases, this port is specified through methods such as existingServerSuite(Class testClass, boolean cleanDB, String hostName, int portNumber) or clientExistingServerSuite(...). I'm just not sure if it's wise to mess with the port property that already exists in TestConfiguration. By tapping into the DEFAULT_PORT property, I think I managed to make the change more transparent for the majority of the tests, although it still remains to be seen how we'd handle tests that specify their own port, as Dag pointed out on DERBY-4090 . On that last point, I agree. It was my bad #
          Hide
          Kathey Marsden added a comment -

          hmmm, on looking at this more closely, I don't think that changing DEFAULT_PORT is the way to go as that should just be a constant representing what the default is if it is not overriden (1527).

          Perhaps port should just be initialized to either DEFAULT_PORT or the system property as appropriate in the no argument constructor. I don't quite get why it is being set to -1 and the hostName to null instead being set to the defaults.

          If you do it this way, the tests would call TestConffiguration.getCurrent() .getPort() or TestConfiguration.getCurrent().newNetworkServerControl() which would rely upon the port and hostName fields in the current TestConfiguration object. I really don't think they should be referencing the static value directly. I think maybe it should be private even.

          Show
          Kathey Marsden added a comment - hmmm, on looking at this more closely, I don't think that changing DEFAULT_PORT is the way to go as that should just be a constant representing what the default is if it is not overriden (1527). Perhaps port should just be initialized to either DEFAULT_PORT or the system property as appropriate in the no argument constructor. I don't quite get why it is being set to -1 and the hostName to null instead being set to the defaults. If you do it this way, the tests would call TestConffiguration.getCurrent() .getPort() or TestConfiguration.getCurrent().newNetworkServerControl() which would rely upon the port and hostName fields in the current TestConfiguration object. I really don't think they should be referencing the static value directly. I think maybe it should be private even.
          Hide
          Kristian Waagan added a comment -

          Linking DERBY-2419, as it concerns some of the same issues. I'm not sure what the current status of that issue is, but at least there are some comments there that may be of interest.

          Show
          Kristian Waagan added a comment - Linking DERBY-2419 , as it concerns some of the same issues. I'm not sure what the current status of that issue is, but at least there are some comments there that may be of interest.
          Hide
          Tiago R. Espinha added a comment -

          Ok, I'm lost here

          The encapsulation of DEFAULT_PORT seems to be issue of debate on DERBY-2419, and Kathey suggested the use of the port property of the TestConfiguration class instead of overriding the DEFAULT_PORT static property. However, we have a handful of constructors for this test and I am unsure of how to tackle this. Should the port be overridden even when the constructor specifies a port?

          With the static initialization I was using, the DEFAULT_PORT would always be overridden and the port was left to be set by tests that rely on the TestConfiguration constructors. However, with this method, only when the empty constructor is used will the port be changed. On all the other cases (including when tests invoke TestConfiguration.getCurrent() ) the port used will be the 1527 or whichever other is specified on the constructor.

          I have set the port property to be defined to the system property when using the empty constructor, and the test passes with and without the property specified. However, some tests do still use the 1527 which isn't really desired.

          This raises another question. When a test calls for the default configuration, what configuration should be given to the test? One with the 1527 port or should it contain the changed port? If we give it the 1527 port, then concurrent testing will be out of the picture in the long run...

          Show
          Tiago R. Espinha added a comment - Ok, I'm lost here The encapsulation of DEFAULT_PORT seems to be issue of debate on DERBY-2419 , and Kathey suggested the use of the port property of the TestConfiguration class instead of overriding the DEFAULT_PORT static property. However, we have a handful of constructors for this test and I am unsure of how to tackle this. Should the port be overridden even when the constructor specifies a port? With the static initialization I was using, the DEFAULT_PORT would always be overridden and the port was left to be set by tests that rely on the TestConfiguration constructors. However, with this method, only when the empty constructor is used will the port be changed. On all the other cases (including when tests invoke TestConfiguration.getCurrent() ) the port used will be the 1527 or whichever other is specified on the constructor. I have set the port property to be defined to the system property when using the empty constructor, and the test passes with and without the property specified. However, some tests do still use the 1527 which isn't really desired. This raises another question. When a test calls for the default configuration, what configuration should be given to the test? One with the 1527 port or should it contain the changed port? If we give it the 1527 port, then concurrent testing will be out of the picture in the long run...
          Hide
          Kathey Marsden added a comment -

          From a functional perspective, when tests are run, we want the value set by derby.tests.port to override the 1527 setting. Ultimately I think we will need three system properties derby.tests.port, derby.tests.alternativePort (currently used by NSinSameJVMTest) and derby.tests.replicationPort.

          The question becomes "Where do we want to process these system properties?", in TestConfiguration itself or in the code that calls the TestConfiguration constructor with the port argument? For example this is some code from ReplicationTestRun which ultimately calls the TestConfiguration constructor with the port. It looks like someone already tried to partially implement this system property business before, so things look a bit tangled up.

          String masterHostName = System.getProperty("test.serverHost", "localhost");
          int masterPortNo = Integer.parseInt(System.getProperty("test.serverPort", "1527"));

          suite.addTest(StandardTests.simpleTest(masterHostName, masterPortNo));
          System.out.println("*** Done suite.addTest(StandardTests.simpleTest())");

          Let me study this a bit and get back to you tomorrow with a recommended approach. Meanwhile if anyone else wants to jump in, suggestions are welcome.

          Show
          Kathey Marsden added a comment - From a functional perspective, when tests are run, we want the value set by derby.tests.port to override the 1527 setting. Ultimately I think we will need three system properties derby.tests.port, derby.tests.alternativePort (currently used by NSinSameJVMTest) and derby.tests.replicationPort. The question becomes "Where do we want to process these system properties?", in TestConfiguration itself or in the code that calls the TestConfiguration constructor with the port argument? For example this is some code from ReplicationTestRun which ultimately calls the TestConfiguration constructor with the port. It looks like someone already tried to partially implement this system property business before, so things look a bit tangled up. String masterHostName = System.getProperty("test.serverHost", "localhost"); int masterPortNo = Integer.parseInt(System.getProperty("test.serverPort", "1527")); suite.addTest(StandardTests.simpleTest(masterHostName, masterPortNo)); System.out.println("*** Done suite.addTest(StandardTests.simpleTest())"); Let me study this a bit and get back to you tomorrow with a recommended approach. Meanwhile if anyone else wants to jump in, suggestions are welcome.
          Hide
          Tiago R. Espinha added a comment -

          I have an idea. If I'm right, most of our tests extend from the BaseJDBCTestCase and then ultimately from BaseTestCase (pretty much all should extend from BaseTestCase?). So what if the BaseTestCase class (or the BaseJDBCTestCase) had a port property that would be initialized with either 1527 or the port sent through the property, if any? This would of course require going through all the tests and make sure that the port was then being used on the test decorators, but it might be the cleaniest alternative to using the DEFAULT_PORT property in TestConfiguration.

          Regarding the alternative port, I believe that currently that's calculated with Port+1. Unless the user defines 65535 for a port, then I see no setback in keeping this method.

          Show
          Tiago R. Espinha added a comment - I have an idea. If I'm right, most of our tests extend from the BaseJDBCTestCase and then ultimately from BaseTestCase (pretty much all should extend from BaseTestCase?). So what if the BaseTestCase class (or the BaseJDBCTestCase) had a port property that would be initialized with either 1527 or the port sent through the property, if any? This would of course require going through all the tests and make sure that the port was then being used on the test decorators, but it might be the cleaniest alternative to using the DEFAULT_PORT property in TestConfiguration. Regarding the alternative port, I believe that currently that's calculated with Port+1. Unless the user defines 65535 for a port, then I see no setback in keeping this method.
          Hide
          Kathey Marsden added a comment -

          Regarding keeping alternative port, being port +1, I guess that's ok. Folks would just have to be aware that they need to skip a port when running concurrent tests.

          I am not sure I like putting port in BaseTestCase, initializing it there and passing it around from all the tests, when the same work can be done in TestConfiguration. For non-replication tests and decorators, I think that they can all use the no-arg constructor and the port can be initialized there to either the default or the system property setting and the tests and decorators don't need to pass in port at all. If you agree, I think you can go ahead and make those changes.

          Replication I need to understand better. I unfortunately don't have any experience with this feature or the tests. ReplicationRun.java has these ports
          static int masterServerPort = 1527; // .. default..
          static String slaveServerHost = "localhost";
          static int slaveServerPort = 3527; // .. ..
          static String testClientHost = "localhost";
          static int slaveReplPort = 6666;

          I seem to recall another one that was four thousand something, that I can't seem to find right away. I need to study what all these ports are used for (and how replication works).

          My gut feeling is that ultimately we will be able to remove the TestConfiguration constructor that takes a port argument and process the replication specific ports setting in the replication tests. Alternately there could be fields for all the additional replication ports needed in TestConfiguration again processed in the no-arg constructor.

          I will look at the replication piece some more and would sure appreciate advice from someone familiar with replication and the tests. Meanwhile for the other tests, I think our goal should be that non of them use the TestConfiguration constructor that takes a port parameter, but rather use the no-arg constructor and initialize port there.

          Show
          Kathey Marsden added a comment - Regarding keeping alternative port, being port +1, I guess that's ok. Folks would just have to be aware that they need to skip a port when running concurrent tests. I am not sure I like putting port in BaseTestCase, initializing it there and passing it around from all the tests, when the same work can be done in TestConfiguration. For non-replication tests and decorators, I think that they can all use the no-arg constructor and the port can be initialized there to either the default or the system property setting and the tests and decorators don't need to pass in port at all. If you agree, I think you can go ahead and make those changes. Replication I need to understand better. I unfortunately don't have any experience with this feature or the tests. ReplicationRun.java has these ports static int masterServerPort = 1527; // .. default.. static String slaveServerHost = "localhost"; static int slaveServerPort = 3527; // .. .. static String testClientHost = "localhost"; static int slaveReplPort = 6666; I seem to recall another one that was four thousand something, that I can't seem to find right away. I need to study what all these ports are used for (and how replication works). My gut feeling is that ultimately we will be able to remove the TestConfiguration constructor that takes a port argument and process the replication specific ports setting in the replication tests. Alternately there could be fields for all the additional replication ports needed in TestConfiguration again processed in the no-arg constructor. I will look at the replication piece some more and would sure appreciate advice from someone familiar with replication and the tests. Meanwhile for the other tests, I think our goal should be that non of them use the TestConfiguration constructor that takes a port parameter, but rather use the no-arg constructor and initialize port there.
          Hide
          Ole Solberg added a comment -

          I had a look at the replication tests wrt port numbers:

          ReplicationRun.initEnvironment() sets
          masterServerPort = 1527;
          slaveServerPort = 4527;
          slaveReplPort = 8888;

          With the suggested pattern of system properties (derby.tests.port, derby.tests.alternativePort, derby.tests.replicationPort) I think we could add derby.tests.slavePort, and set masterServerPort, slaveServerPort, slaveReplPort to derby.tests.port, derby.tests.slavePort, derby.tests.replicationPort respectively.

          The masterPortNo set to "test.serverPort" in ReplicationTestRun.suite() should then be set to derby.tests.port.

          It would also be ok, as far as I can see, to use offsets from derby.tests.port for alternativePort, slavePort and replicationPort, if they are not explicitly set. Using +1, +2, +3, would then mean that we would need to skip (at least) three ports when running concurrent tests.

          Show
          Ole Solberg added a comment - I had a look at the replication tests wrt port numbers: ReplicationRun.initEnvironment() sets masterServerPort = 1527; slaveServerPort = 4527; slaveReplPort = 8888; With the suggested pattern of system properties (derby.tests.port, derby.tests.alternativePort, derby.tests.replicationPort) I think we could add derby.tests.slavePort, and set masterServerPort, slaveServerPort, slaveReplPort to derby.tests.port, derby.tests.slavePort, derby.tests.replicationPort respectively. The masterPortNo set to "test.serverPort" in ReplicationTestRun.suite() should then be set to derby.tests.port. It would also be ok, as far as I can see, to use offsets from derby.tests.port for alternativePort, slavePort and replicationPort, if they are not explicitly set. Using +1, +2, +3, would then mean that we would need to skip (at least) three ports when running concurrent tests.
          Hide
          Tiago R. Espinha added a comment - - edited

          So to wrap it up in a nutshell. We need four ports as far as testing goes:

          • derby.tests.port
          • derby.tests.alternativePort
          • derby.tests.replicationPort
          • derby.tests.slavePort

          And basically, if they aren't explicitly set, they can be defined with offsets based on a port. This port is 1527 unless otherwise overridden with the derby.tests.port property.

          My idea is to add four (static) properties to TestConfiguration and force the decorators to use these properties [which are set to their default values or to the system properties if they are defined]. The point of having new properties is that I need a coherent way of fetching at least the main port from within other tests (like the ij test). I could just fetch the system property from within the other tests, but that doesn't seem a good programming practice at all.

          Show
          Tiago R. Espinha added a comment - - edited So to wrap it up in a nutshell. We need four ports as far as testing goes: derby.tests.port derby.tests.alternativePort derby.tests.replicationPort derby.tests.slavePort And basically, if they aren't explicitly set, they can be defined with offsets based on a port. This port is 1527 unless otherwise overridden with the derby.tests.port property. My idea is to add four (static) properties to TestConfiguration and force the decorators to use these properties [which are set to their default values or to the system properties if they are defined] . The point of having new properties is that I need a coherent way of fetching at least the main port from within other tests (like the ij test). I could just fetch the system property from within the other tests, but that doesn't seem a good programming practice at all.
          Hide
          Kathey Marsden added a comment -

          Tiago said:
          >My idea is to add four (static) properties to TestConfiguration and force the >decorators to use these properties [which are set to their default values or to >the system properties if they are defined].

          I think that sounds fine and then get rid of the TestConfiguration constructor that takes a port. I think we should shoot for incremental patches though.
          1) The ij changes to pick up system properties DERBY-4223.
          2) The changes for derby.tests.port.
          3) The changes for derby.tests.alternativePort
          4) Replication and removing the TestConfiguration with port constructor.

          Show
          Kathey Marsden added a comment - Tiago said: >My idea is to add four (static) properties to TestConfiguration and force the >decorators to use these properties [which are set to their default values or to >the system properties if they are defined] . I think that sounds fine and then get rid of the TestConfiguration constructor that takes a port. I think we should shoot for incremental patches though. 1) The ij changes to pick up system properties DERBY-4223 . 2) The changes for derby.tests.port. 3) The changes for derby.tests.alternativePort 4) Replication and removing the TestConfiguration with port constructor.
          Hide
          Bryan Pendleton added a comment -

          The terminology "alternativePort" does not seem very descriptive, would it help to pick
          a more explicit name for that port? Is it only used for the test where we have two
          network servers inside the same JVM? If so, perhaps something like:

          derby.tests.NSInSameJVM.secondServerPort

          I know that's very verbose, but I just thought I'd throw it out as an idea.

          Show
          Bryan Pendleton added a comment - The terminology "alternativePort" does not seem very descriptive, would it help to pick a more explicit name for that port? Is it only used for the test where we have two network servers inside the same JVM? If so, perhaps something like: derby.tests.NSInSameJVM.secondServerPort I know that's very verbose, but I just thought I'd throw it out as an idea.
          Hide
          Tiago R. Espinha added a comment -

          Submitting a patch for the IJ part. This also changes the way the port property is initialized in TestConfiguration under the empty constructor. From now on, that is initialized as 1527. Haven't still ran suites.All but will do so tonight, to be sure it doesn't break anything.

          Show
          Tiago R. Espinha added a comment - Submitting a patch for the IJ part. This also changes the way the port property is initialized in TestConfiguration under the empty constructor. From now on, that is initialized as 1527. Haven't still ran suites.All but will do so tonight, to be sure it doesn't break anything.
          Hide
          Tiago R. Espinha added a comment -

          Attaching another patch to replace the previous one. The previous used the DEFAULT_PORT and it is better to bundle it up with the TestConfiguration.getCurrent().getPort(), where the port is initialized to either the DEFAULT_PORT or the system property in case it is used.

          Show
          Tiago R. Espinha added a comment - Attaching another patch to replace the previous one. The previous used the DEFAULT_PORT and it is better to bundle it up with the TestConfiguration.getCurrent().getPort(), where the port is initialized to either the DEFAULT_PORT or the system property in case it is used.
          Hide
          Tiago R. Espinha added a comment -

          There was a small typo on the patch (was derby.test.port, should be derby.tests.port). Fixing that and running tests again.

          Show
          Tiago R. Espinha added a comment - There was a small typo on the patch (was derby.test.port, should be derby.tests.port). Fixing that and running tests again.
          Hide
          Tiago R. Espinha added a comment -

          It seems like the current patch is failing, I'll be looking at submitting a fixed one today.

          Show
          Tiago R. Espinha added a comment - It seems like the current patch is failing, I'll be looking at submitting a fixed one today.
          Hide
          Kathey Marsden added a comment -

          Was your May 26 patch tested and worked ok? If so it might make sense to commit that one as a clean and independent patch of DERBY-4223 without the derby.tests.port handling.

          Show
          Kathey Marsden added a comment - Was your May 26 patch tested and worked ok? If so it might make sense to commit that one as a clean and independent patch of DERBY-4223 without the derby.tests.port handling.
          Hide
          Tiago R. Espinha added a comment -

          No Kathey, that's the one that I tested last night and it failed. This one does not address the handling of derby.tests.port yet.

          Show
          Tiago R. Espinha added a comment - No Kathey, that's the one that I tested last night and it failed. This one does not address the handling of derby.tests.port yet.
          Hide
          Tiago R. Espinha added a comment -

          Submitting new patch. Ran into some troubles with the client/server decorators.

          This patch is a tangent to the changes required for derby.tests.port, meaning that it taps into one of the decorator with the port change. I tried changing as little as possible within TestConfiguration since the most work will be done for derby.tests.port.

          Running suites.All again tonight.

          Show
          Tiago R. Espinha added a comment - Submitting new patch. Ran into some troubles with the client/server decorators. This patch is a tangent to the changes required for derby.tests.port, meaning that it taps into one of the decorator with the port change. I tried changing as little as possible within TestConfiguration since the most work will be done for derby.tests.port. Running suites.All again tonight.
          Hide
          Tiago R. Espinha added a comment -

          Attaching possibly the final patch for the ij bit.

          The previous patch had an issue. Since I was forcing the port to be TestConfiguration.getCurrent().getPort() in a constructor that is widely used in other tests, this would break them since they were expecting the port to be 1527 and not whatever property was specified.

          This patch creates an additional constructor specifically for ij that allows the port to be customized. Then the test specifies the port as getCurrent().getPort.

          The suites.All ran fine for this patch when the port property was specified, but it gave me the following error when I ran it without the port property:
          -------------8<--------------
          1) testAttributeAccumulatedConnectionCount(org.apache.derbyTesting.functionTests.tests.management.NetworkServerMBeanTest)java.security.PrivilegedActionException: javax.management.InstanceNotFoundException: org.apache.derby:type=NetworkServer,system=c013800d-0121-94a9-8310-ffff8baa048d
          at java.security.AccessController.doPrivileged(Native Method)
          at org.apache.derbyTesting.functionTests.tests.management.MBeanTest.getAttribute(MBeanTest.java:379)
          at org.apache.derbyTesting.functionTests.tests.management.NetworkServerMBeanTest.testAttributeAccumulatedConnectionCount(NetworkServerMBeanTest.java:93)
          (...)
          --------------8<----------------

          I'm thinking that it might be unrelated though, and as so I'm running the suites.All again without a port specified.

          Show
          Tiago R. Espinha added a comment - Attaching possibly the final patch for the ij bit. The previous patch had an issue. Since I was forcing the port to be TestConfiguration.getCurrent().getPort() in a constructor that is widely used in other tests, this would break them since they were expecting the port to be 1527 and not whatever property was specified. This patch creates an additional constructor specifically for ij that allows the port to be customized. Then the test specifies the port as getCurrent().getPort. The suites.All ran fine for this patch when the port property was specified, but it gave me the following error when I ran it without the port property: ------------- 8< -------------- 1) testAttributeAccumulatedConnectionCount(org.apache.derbyTesting.functionTests.tests.management.NetworkServerMBeanTest)java.security.PrivilegedActionException: javax.management.InstanceNotFoundException: org.apache.derby:type=NetworkServer,system=c013800d-0121-94a9-8310-ffff8baa048d at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.functionTests.tests.management.MBeanTest.getAttribute(MBeanTest.java:379) at org.apache.derbyTesting.functionTests.tests.management.NetworkServerMBeanTest.testAttributeAccumulatedConnectionCount(NetworkServerMBeanTest.java:93) (...) -------------- 8< ---------------- I'm thinking that it might be unrelated though, and as so I'm running the suites.All again without a port specified.
          Hide
          Tiago R. Espinha added a comment -

          Ok, suites.All and derbyall ran fine with and without the derby.tests.port property so the patch is ready for committal.

          I will be working on finishing the changes for derby.tests.port now.

          Show
          Tiago R. Espinha added a comment - Ok, suites.All and derbyall ran fine with and without the derby.tests.port property so the patch is ready for committal. I will be working on finishing the changes for derby.tests.port now.
          Hide
          Tiago R. Espinha added a comment -

          I am now stuck on the ServerPropertiesTest and this test will require some thought. On this test a derby.properties file is created where a port is specified; this must be handled with care, because for the test to make sense, that port must take precedence over the derby.tests.port property.

          This test also has a getAlternativePort() method and I am considering the possibility of getting it to use the TestConfiguration.getAlternativePort() instead.

          Other than this one test, I believe the remaining tests are running fine. My methodology has been running suites.All and as soon as I have failures, I stop the run and check the error files to see which tests are failing. Then I fix those tests and run it again. This is one of the tests that fails right on the start, so there may still be some tests failing towards the end of the suite.

          In the long run we should also debate whether we want a decorator in TestConfiguration that allows for the port to be customized. If we allow this, it may be dangerous for concurrent testing. On my current patch for ij we do have such decorator, but that's necessary evil for incremental patches; it could be removed when I issue the final derby.tests.port patch.

          Show
          Tiago R. Espinha added a comment - I am now stuck on the ServerPropertiesTest and this test will require some thought. On this test a derby.properties file is created where a port is specified; this must be handled with care, because for the test to make sense, that port must take precedence over the derby.tests.port property. This test also has a getAlternativePort() method and I am considering the possibility of getting it to use the TestConfiguration.getAlternativePort() instead. Other than this one test, I believe the remaining tests are running fine. My methodology has been running suites.All and as soon as I have failures, I stop the run and check the error files to see which tests are failing. Then I fix those tests and run it again. This is one of the tests that fails right on the start, so there may still be some tests failing towards the end of the suite. In the long run we should also debate whether we want a decorator in TestConfiguration that allows for the port to be customized. If we allow this, it may be dangerous for concurrent testing. On my current patch for ij we do have such decorator, but that's necessary evil for incremental patches; it could be removed when I issue the final derby.tests.port patch.
          Hide
          Tiago R. Espinha added a comment -

          Unchecking the patch available switch. The available patch has been committed to revision 781200, but this issue entails at least 3 more patches.

          Show
          Tiago R. Espinha added a comment - Unchecking the patch available switch. The available patch has been committed to revision 781200, but this issue entails at least 3 more patches.
          Hide
          Tiago R. Espinha added a comment -

          I'm submitting a preliminary patch for derby.tests.port.

          This patch should NOT be committed. I am pretty sure I have some issues on the TestConfiguration class because of conflicts with the previous ij patch. I will be running suites.All to iron out what's still amiss.

          This patch already covers the ServerPropertiesTest that I commented about earlier. Also, unlike what I said, this one test is keeping its own getAlternativePort() method as the behavior expected is different. This test expects the getAlternativePort() method to continuously supply ports that are valid alternatives to the main port. This was done by going with +1 increments to the default port until it finds a port that is unbound to any server. This behavior is kept but instead of using the default port as a base, it now uses the derby.tests.port property.

          The derby.tests.alternativePort property wouldn't be much good here as this test requires several (4 or 5) alternative ports and is just a very special case of using alternative ports.

          Show
          Tiago R. Espinha added a comment - I'm submitting a preliminary patch for derby.tests.port. This patch should NOT be committed. I am pretty sure I have some issues on the TestConfiguration class because of conflicts with the previous ij patch. I will be running suites.All to iron out what's still amiss. This patch already covers the ServerPropertiesTest that I commented about earlier. Also, unlike what I said, this one test is keeping its own getAlternativePort() method as the behavior expected is different. This test expects the getAlternativePort() method to continuously supply ports that are valid alternatives to the main port. This was done by going with +1 increments to the default port until it finds a port that is unbound to any server. This behavior is kept but instead of using the default port as a base, it now uses the derby.tests.port property. The derby.tests.alternativePort property wouldn't be much good here as this test requires several (4 or 5) alternative ports and is just a very special case of using alternative ports.
          Hide
          Tiago R. Espinha added a comment -

          As I suspected there were still some failing tests. I have fixed those and am running regressions now to figure out if there still are failing tests.

          It needs to be mentioned that due what I mentioned on my previous comment, whoever runs concurrent tests should account for a 5 port gap for the ports chosen for each run. This should be added to the wiki page of Derby Testing when this patch is finally aubmitted.

          Show
          Tiago R. Espinha added a comment - As I suspected there were still some failing tests. I have fixed those and am running regressions now to figure out if there still are failing tests. It needs to be mentioned that due what I mentioned on my previous comment, whoever runs concurrent tests should account for a 5 port gap for the ports chosen for each run. This should be added to the wiki page of Derby Testing when this patch is finally aubmitted.
          Hide
          Tiago R. Espinha added a comment -

          Attaching one possible final patch for derby.tests.port.

          My last suites.All run had an issue that I fixed for this patch and it should all be running properly now. I will be running suites.All again and when that's finished, I will run one last test:

          • I will create a small application that binds on the 1527 port, and with that running on the background I will run the suites.All suite with a port different from the default. If there is still some test somewhere that tries to use the default port, hopefully this will make it pop-up.
          Show
          Tiago R. Espinha added a comment - Attaching one possible final patch for derby.tests.port. My last suites.All run had an issue that I fixed for this patch and it should all be running properly now. I will be running suites.All again and when that's finished, I will run one last test: I will create a small application that binds on the 1527 port, and with that running on the background I will run the suites.All suite with a port different from the default. If there is still some test somewhere that tries to use the default port, hopefully this will make it pop-up.
          Hide
          Tiago R. Espinha added a comment -

          The previous patch had a small issue with two lines of code switched around.

          Show
          Tiago R. Espinha added a comment - The previous patch had a small issue with two lines of code switched around.
          Hide
          Tiago R. Espinha added a comment -

          The suites.All ran fine with a port set through derby.tests.port. I'm now running it again, this time with an application binding the 1527 port.

          If all goes well, the test should pass. If, on the other hand, the suites.All is still using the 1527 port hardcoded somewhere, it should fail when it tries to bind on this port.

          Show
          Tiago R. Espinha added a comment - The suites.All ran fine with a port set through derby.tests.port. I'm now running it again, this time with an application binding the 1527 port. If all goes well, the test should pass. If, on the other hand, the suites.All is still using the 1527 port hardcoded somewhere, it should fail when it tries to bind on this port.
          Hide
          Tiago R. Espinha added a comment -

          I'm posting the result of the latest test run with the 1527 port bound.

          There seems to be some issues with replication, although that is probably normal. I haven't had the chance to go over these results yet but there also seems to be an issue with a ping. I will be going over this later today.

          Show
          Tiago R. Espinha added a comment - I'm posting the result of the latest test run with the 1527 port bound. There seems to be some issues with replication, although that is probably normal. I haven't had the chance to go over these results yet but there also seems to be an issue with a ping. I will be going over this later today.
          Hide
          Tiago R. Espinha added a comment -

          Provided that the patch is good (I have been experiencing some oddity regarding end of line characters, but nothing serious it seems), this will be the last patch for derby.tests.port.

          The replication run still had the 1527 port hardcoded in it as the masterServerPort, but as Ole suggested, this port is now being set to derby.tests.port.

          The NetworkServerControlClientCommandTest also had some changes of mine that would break should the default port be bound. Nothing big, just the pings when no port is supplied to the NetworkServerControl. If no port is supplied, the default port (1527) is used, and if there's something listening and replying on the 1527 port (the app. I created sends out random replies on connection attempts since some tests will wait to hear something), the ping passes and I wasn't accounting for that in the Network...CommandTest.

          With this latest patch, the replication run and the Network...CommandTest run fine, but I have not yet ran the suites.All. I will once more run it with and without the 1527 port bound.

          Do note that the changes for the replication ports will come in a separate patch.

          Show
          Tiago R. Espinha added a comment - Provided that the patch is good (I have been experiencing some oddity regarding end of line characters, but nothing serious it seems), this will be the last patch for derby.tests.port. The replication run still had the 1527 port hardcoded in it as the masterServerPort, but as Ole suggested, this port is now being set to derby.tests.port. The NetworkServerControlClientCommandTest also had some changes of mine that would break should the default port be bound. Nothing big, just the pings when no port is supplied to the NetworkServerControl. If no port is supplied, the default port (1527) is used, and if there's something listening and replying on the 1527 port (the app. I created sends out random replies on connection attempts since some tests will wait to hear something), the ping passes and I wasn't accounting for that in the Network...CommandTest. With this latest patch, the replication run and the Network...CommandTest run fine, but I have not yet ran the suites.All. I will once more run it with and without the 1527 port bound. Do note that the changes for the replication ports will come in a separate patch.
          Hide
          Tiago R. Espinha added a comment -

          The suites.All ran without errors or failures with and without the 1527 port bound. I'm now running it without the derby.tests.port specified but things are looking good.

          The patch should be ready for commit.

          Show
          Tiago R. Espinha added a comment - The suites.All ran without errors or failures with and without the 1527 port bound. I'm now running it without the derby.tests.port specified but things are looking good. The patch should be ready for commit.
          Hide
          Tiago R. Espinha added a comment -

          I'm marking this issue with Patch Available until the latest patch gets committed.

          In the meanwhile I'll be working on the alternative port changes.

          Show
          Tiago R. Espinha added a comment - I'm marking this issue with Patch Available until the latest patch gets committed. In the meanwhile I'll be working on the alternative port changes.
          Hide
          Kathey Marsden added a comment -

          I looked a the patch DERBY-4217.dtp.patch and just have one comment:

          BaseTestCase - execJavaCmd
          I don't really like so much adding the portIsSpecified parameter to execJavaCmd and translating that into "-p <portNumber>" since it is supposed to be a generic java command execution. I think it would be better to just pass the -p and portnumber in the cmd[] array or perhaps have a separate execNetworkServerControl method.

          Show
          Kathey Marsden added a comment - I looked a the patch DERBY-4217 .dtp.patch and just have one comment: BaseTestCase - execJavaCmd I don't really like so much adding the portIsSpecified parameter to execJavaCmd and translating that into "-p <portNumber>" since it is supposed to be a generic java command execution. I think it would be better to just pass the -p and portnumber in the cmd[] array or perhaps have a separate execNetworkServerControl method.
          Hide
          Tiago R. Espinha added a comment -

          Hmm, I made it this way because of the order of the parameters since that was giving me some trouble (when passing the -p on the command), but now that I look at it, it actually does the same. I will remove that method and try to use the command parameter for this purpose, although I am fairly sure that I did run into some issues with this approach.

          Show
          Tiago R. Espinha added a comment - Hmm, I made it this way because of the order of the parameters since that was giving me some trouble (when passing the -p on the command), but now that I look at it, it actually does the same. I will remove that method and try to use the command parameter for this purpose, although I am fairly sure that I did run into some issues with this approach.
          Hide
          Tiago R. Espinha added a comment -

          As per Kathey's comment, I've removed the said method from the patch and will be running the tests again to catch the failures.

          Show
          Tiago R. Espinha added a comment - As per Kathey's comment, I've removed the said method from the patch and will be running the tests again to catch the failures.
          Hide
          Tiago R. Espinha added a comment -

          Regarding my comment on 09/Jun/09 10:45 AM, there has been another change here.

          At this point, the pings under NetworkServerControlClientCommandTest that don't specify a port through the -p parameter won't be ran if the current port is different from the DEFAULT_PORT in TestConfiguration. The portless pings go to the 1527 port by default so it makes no sense to ping this port if we are using a different one.

          It needs to be noted that the possibility of asserting a failed ping would be a good option but it isn't quite viable. The ping is a 'blind' operation, meaning that whichever program binds the port being pinged, the ping will pass. My suites.All run with a custom port and with an application binding on to 1527 would fail with this option.

          Show
          Tiago R. Espinha added a comment - Regarding my comment on 09/Jun/09 10:45 AM, there has been another change here. At this point, the pings under NetworkServerControlClientCommandTest that don't specify a port through the -p parameter won't be ran if the current port is different from the DEFAULT_PORT in TestConfiguration. The portless pings go to the 1527 port by default so it makes no sense to ping this port if we are using a different one. It needs to be noted that the possibility of asserting a failed ping would be a good option but it isn't quite viable. The ping is a 'blind' operation, meaning that whichever program binds the port being pinged, the ping will pass. My suites.All run with a custom port and with an application binding on to 1527 would fail with this option.
          Hide
          Tiago R. Espinha added a comment -

          I'm submitting the final patch for derby.tests.port.

          I did as Kathey suggested and am now passing the -p parameter on every call to execJavaCmd().

          Since there were some side-changes to TestConfiguration per DERBY-4262, I'm recompiling and doing one last run of suites.All just to be sure. Still, before this, I had run suites.All successfully with and without the 1527 port bound.

          Show
          Tiago R. Espinha added a comment - I'm submitting the final patch for derby.tests.port. I did as Kathey suggested and am now passing the -p parameter on every call to execJavaCmd(). Since there were some side-changes to TestConfiguration per DERBY-4262 , I'm recompiling and doing one last run of suites.All just to be sure. Still, before this, I had run suites.All successfully with and without the 1527 port bound.
          Hide
          Tiago R. Espinha added a comment -

          The last suites.All run ran with no problems. The patch is ready to commit if there are no objections.

          Show
          Tiago R. Espinha added a comment - The last suites.All run ran with no problems. The patch is ready to commit if there are no objections.
          Hide
          Tiago R. Espinha added a comment -

          Attaching a preliminary patch for the alternative port. The parameter is derby.tests.alternativePort.

          I've ran suites.All with success without specifying the port (which is the critical path that must be achieved). When a port isn't specified, the alternative port is the normal port +1 as it has been agreed upon here.

          My next test will be running suites.All with an alternative port specified using this property.

          This patch includes all the changes contained in the derby.tests.port patch. I will upload a new one, just with the changes for the alternative port when the other patch has been committed.

          Show
          Tiago R. Espinha added a comment - Attaching a preliminary patch for the alternative port. The parameter is derby.tests.alternativePort. I've ran suites.All with success without specifying the port (which is the critical path that must be achieved). When a port isn't specified, the alternative port is the normal port +1 as it has been agreed upon here. My next test will be running suites.All with an alternative port specified using this property. This patch includes all the changes contained in the derby.tests.port patch. I will upload a new one, just with the changes for the alternative port when the other patch has been committed.
          Hide
          Kathey Marsden added a comment -

          Committed patch DERBY-4217-dtp.patch with revision 785049.

          Show
          Kathey Marsden added a comment - Committed patch DERBY-4217 -dtp.patch with revision 785049.
          Hide
          Tiago R. Espinha added a comment -

          This patch passed suites.All without the alternative port being explicitly set (but with d.t.port set) and is ready to be reviewed.

          I will still be running the following tests:

          • Not specifying any port at all
          • Specifying the alternative port and binding 1528 (this one was previously the default alternative port)
          Show
          Tiago R. Espinha added a comment - This patch passed suites.All without the alternative port being explicitly set (but with d.t.port set) and is ready to be reviewed. I will still be running the following tests: Not specifying any port at all Specifying the alternative port and binding 1528 (this one was previously the default alternative port)
          Hide
          Kathey Marsden added a comment -

          I looked at DERBY-4217dtap patch.
          I think there is a typo on line 1022
          this.alternativePort = copy.port;
          should be
          this.alternativePort = copy.alternativePort;

          Regarding Bryan's comment on renaming the property. I think that would be fine or perhaps just derby.tests.secondServerPort in case another tests wants to start a second server. Either way is fine.

          But ...
          Tiago and I talked on IRC about any potential conflict between this port and the ports used in ServerPropertiesTest and he determined there would be none because they are run sequentially, but it is a bit confusing and he had a suggestion for a cleaner implementation which would encapsulate all the alternative port assignments in TestConfiguration. I will let him summarize that idea.

          Show
          Kathey Marsden added a comment - I looked at DERBY-4217 dtap patch. I think there is a typo on line 1022 this.alternativePort = copy.port; should be this.alternativePort = copy.alternativePort; Regarding Bryan's comment on renaming the property. I think that would be fine or perhaps just derby.tests.secondServerPort in case another tests wants to start a second server. Either way is fine. But ... Tiago and I talked on IRC about any potential conflict between this port and the ports used in ServerPropertiesTest and he determined there would be none because they are run sequentially, but it is a bit confusing and he had a suggestion for a cleaner implementation which would encapsulate all the alternative port assignments in TestConfiguration. I will let him summarize that idea.
          Hide
          Tiago R. Espinha added a comment -

          You are right Kathey, that's a typo on my end. However, since we will probably change things a bit regarding the alternative ports, that patch will stay on hold.

          So, the cleaner implementation would imply having a getNextAvailablePort() method in TestConfiguration that would replace our getAlternativePort().

          I think that ideally we'd even only allow for the base port to be set and then every other port would build on that by offsetting from the base port. Each time we'd invoke getNextAvailablePort(), we would get an usable port for our a server. Do note that the base port remains fixed throughout the suite run; it is just alternative ports (replication ports, jmx, actual alternative ports that some tests like ServerPropertiesTest require) that would build on upon the base port.

          We would have no real offset, the alternative ports would be assigned on the go with lastAvailablePort+1.

          Also, Kathey suggested we would create a constant under TestConfiguration named MAX_PORTS_USED, which would tell us the maximum number of alternative ports that can be assigned. This constant would then be used in getNextAvailablePort() in an assert - if the port being requested is over the limit of ports that can be used, it would mean that a new test that uses an alternative port had been added and so the developer would have to update the constant and the wiki page. (This is an attempt to make sure that the wiki page on concurrent test runs is always up-to-date, and that people running concurrent runs always know how many ports they should leave between runs)

          To finalize, with this idea, the concept of alternative port is lost. Or better said, it merges with the concept of JMX and replication ports since those would also be obtained through the getNextAvailablePort().

          It might be too restrictive to only allow the base port to be set, but I think we would be able to keep it simple this way. If we allow JMX, or replication (or both) ports to be set through properties, then this concept might not be the ideal; and honestly, from a developer perspective, I think it is much straightforward if we only have to set the base port and know that the concurrent run just has to be X ports away from the first run.

          Show
          Tiago R. Espinha added a comment - You are right Kathey, that's a typo on my end. However, since we will probably change things a bit regarding the alternative ports, that patch will stay on hold. So, the cleaner implementation would imply having a getNextAvailablePort() method in TestConfiguration that would replace our getAlternativePort(). I think that ideally we'd even only allow for the base port to be set and then every other port would build on that by offsetting from the base port. Each time we'd invoke getNextAvailablePort(), we would get an usable port for our a server. Do note that the base port remains fixed throughout the suite run; it is just alternative ports (replication ports, jmx, actual alternative ports that some tests like ServerPropertiesTest require) that would build on upon the base port. We would have no real offset, the alternative ports would be assigned on the go with lastAvailablePort+1. Also, Kathey suggested we would create a constant under TestConfiguration named MAX_PORTS_USED, which would tell us the maximum number of alternative ports that can be assigned. This constant would then be used in getNextAvailablePort() in an assert - if the port being requested is over the limit of ports that can be used, it would mean that a new test that uses an alternative port had been added and so the developer would have to update the constant and the wiki page. (This is an attempt to make sure that the wiki page on concurrent test runs is always up-to-date, and that people running concurrent runs always know how many ports they should leave between runs) To finalize, with this idea, the concept of alternative port is lost. Or better said, it merges with the concept of JMX and replication ports since those would also be obtained through the getNextAvailablePort(). It might be too restrictive to only allow the base port to be set, but I think we would be able to keep it simple this way. If we allow JMX, or replication (or both) ports to be set through properties, then this concept might not be the ideal; and honestly, from a developer perspective, I think it is much straightforward if we only have to set the base port and know that the concurrent run just has to be X ports away from the first run.
          Hide
          Tiago R. Espinha added a comment - - edited

          This is a preliminary patch for the idea mentioned before. There's one thing I don't like about it yet and that's the initialization of the lastAssignedPort. It's being done on all constructors and that doesn't seem very elegant. On the other hand, doing a static initialization doesn't work either as I have to initialize it to TestConfiguration.getCurrent().getPort(). (However, I might just as well fetch the derby.tests.port property in the initializer...)

          This test also contains a change suggested by Kathey - the change of the property name from derby.tests.port to derby.tests.basePort simply for clarity's sake as to which port is this.

          There's also something I don't understand but it seems to be working and that is replication. As you can see on the replaced lines on the patch, the replication ports were initialized statically to 3527 and 6666. Yet, they are later on changed in initEnvironment() to 4527 and 8888.

          The way I went about it was to do the actual assignment in the static initialization and disregard the other assignments in initEnvironment. Then I ran the ReplicationSuite and it gave me no errors. I'm doing a suites.All run with this patch tonight.

          Show
          Tiago R. Espinha added a comment - - edited This is a preliminary patch for the idea mentioned before. There's one thing I don't like about it yet and that's the initialization of the lastAssignedPort. It's being done on all constructors and that doesn't seem very elegant. On the other hand, doing a static initialization doesn't work either as I have to initialize it to TestConfiguration.getCurrent().getPort(). (However, I might just as well fetch the derby.tests.port property in the initializer...) This test also contains a change suggested by Kathey - the change of the property name from derby.tests.port to derby.tests.basePort simply for clarity's sake as to which port is this. There's also something I don't understand but it seems to be working and that is replication. As you can see on the replaced lines on the patch, the replication ports were initialized statically to 3527 and 6666. Yet, they are later on changed in initEnvironment() to 4527 and 8888. The way I went about it was to do the actual assignment in the static initialization and disregard the other assignments in initEnvironment. Then I ran the ReplicationSuite and it gave me no errors. I'm doing a suites.All run with this patch tonight.
          Hide
          Tiago R. Espinha added a comment -

          There have been a few errors on my dual run under Windows 7 but I do not think they are related to the patch. My guess is that they showed up because of the load that concurrent testing puts on the machine. Still, I'm open to suggestions. Here goes the output I got on the console:
          -------------------------8<---------------------------
          There were 8 failures:
          1) testEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTest
          )junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suites
          .All\system\dbsqlauth\db.lck
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas
          eSetup.java:130)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba
          seSetup.java:35)
          at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet
          up.java:105)
          at java.security.AccessController.doPrivileged(Native Method)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:102)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:98)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa
          tabaseSetup.java:91)
          at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig
          uration.java:782)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:22)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57
          )
          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 junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          2) testReEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTe
          st)junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suit
          es.All\system\dbsqlauth\db.lck
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas
          eSetup.java:130)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba
          seSetup.java:35)
          at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet
          up.java:105)
          at java.security.AccessController.doPrivileged(Native Method)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:102)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:98)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa
          tabaseSetup.java:91)
          at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig
          uration.java:782)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:22)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57
          )
          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 junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          3) testEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTest
          )junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suites
          .All\system\dbsqlauth\db.lck
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas
          eSetup.java:130)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba
          seSetup.java:35)
          at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet
          up.java:105)
          at java.security.AccessController.doPrivileged(Native Method)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:102)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:98)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa
          tabaseSetup.java:91)
          at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig
          uration.java:782)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:22)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57
          )
          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 junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          4) testReEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTe
          st)junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suit
          es.All\system\dbsqlauth\db.lck
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas
          eSetup.java:130)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba
          seSetup.java:35)
          at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet
          up.java:105)
          at java.security.AccessController.doPrivileged(Native Method)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:102)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:98)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa
          tabaseSetup.java:91)
          at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig
          uration.java:782)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:22)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57
          )
          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 junit.extensions.TestDecorator.basicRun(TestDecorator.java:24)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:21)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          5) testEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTest
          )junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suites
          .All\system\dbsqlauth\db.lck
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas
          eSetup.java:130)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba
          seSetup.java:35)
          at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet
          up.java:105)
          at java.security.AccessController.doPrivileged(Native Method)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:102)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:98)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa
          tabaseSetup.java:91)
          at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig
          uration.java:782)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:22)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57
          )
          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 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 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
          )
          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)
          6) testReEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTe
          st)junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suit
          es.All\system\dbsqlauth\db.lck
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas
          eSetup.java:130)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba
          seSetup.java:35)
          at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet
          up.java:105)
          at java.security.AccessController.doPrivileged(Native Method)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:102)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:98)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa
          tabaseSetup.java:91)
          at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig
          uration.java:782)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:22)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57
          )
          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 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 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
          )
          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)
          7) testEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTest
          )junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suites
          .All\system\dbsqlauth\db.lck
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas
          eSetup.java:130)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba
          seSetup.java:35)
          at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet
          up.java:105)
          at java.security.AccessController.doPrivileged(Native Method)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:102)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:98)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa
          tabaseSetup.java:91)
          at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig
          uration.java:782)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:22)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57
          )
          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 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 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
          )
          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)
          8) testReEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTe
          st)junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suit
          es.All\system\dbsqlauth\db.lck
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas
          eSetup.java:130)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba
          seSetup.java:35)
          at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet
          up.java:105)
          at java.security.AccessController.doPrivileged(Native Method)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:102)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD
          atabaseSetup.java:98)
          at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa
          tabaseSetup.java:91)
          at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig
          uration.java:782)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:22)
          at junit.extensions.TestSetup.run(TestSetup.java:25)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57
          )
          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 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 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
          )
          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)

          FAILURES!!!
          Tests run: 11068, Failures: 8, Errors: 32
          --------------------------8<--------------------------

          This is on one of the runs. The other ran flawlessly:
          Time: 17.805,672

          OK (11161 tests)

          I also have a three simultaneous runs going in an Ubuntu environment.

          Show
          Tiago R. Espinha added a comment - There have been a few errors on my dual run under Windows 7 but I do not think they are related to the patch. My guess is that they showed up because of the load that concurrent testing puts on the machine. Still, I'm open to suggestions. Here goes the output I got on the console: ------------------------- 8< --------------------------- There were 8 failures: 1) testEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTest )junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suites .All\system\dbsqlauth\db.lck at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas eSetup.java:130) at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba seSetup.java:35) at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet up.java:105) at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:102) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:98) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa tabaseSetup.java:91) at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig uration.java:782) at junit.extensions.TestSetup$1.protect(TestSetup.java:22) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57 ) 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 junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) 2) testReEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTe st)junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suit es.All\system\dbsqlauth\db.lck at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas eSetup.java:130) at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba seSetup.java:35) at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet up.java:105) at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:102) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:98) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa tabaseSetup.java:91) at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig uration.java:782) at junit.extensions.TestSetup$1.protect(TestSetup.java:22) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57 ) 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 junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) 3) testEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTest )junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suites .All\system\dbsqlauth\db.lck at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas eSetup.java:130) at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba seSetup.java:35) at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet up.java:105) at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:102) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:98) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa tabaseSetup.java:91) at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig uration.java:782) at junit.extensions.TestSetup$1.protect(TestSetup.java:22) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57 ) 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 junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) 4) testReEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTe st)junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suit es.All\system\dbsqlauth\db.lck at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas eSetup.java:130) at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba seSetup.java:35) at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet up.java:105) at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:102) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:98) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa tabaseSetup.java:91) at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig uration.java:782) at junit.extensions.TestSetup$1.protect(TestSetup.java:22) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57 ) 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 junit.extensions.TestDecorator.basicRun(TestDecorator.java:24) at junit.extensions.TestSetup$1.protect(TestSetup.java:21) at junit.extensions.TestSetup.run(TestSetup.java:25) 5) testEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTest )junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suites .All\system\dbsqlauth\db.lck at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas eSetup.java:130) at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba seSetup.java:35) at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet up.java:105) at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:102) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:98) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa tabaseSetup.java:91) at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig uration.java:782) at junit.extensions.TestSetup$1.protect(TestSetup.java:22) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57 ) 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 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 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 ) 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) 6) testReEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTe st)junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suit es.All\system\dbsqlauth\db.lck at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas eSetup.java:130) at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba seSetup.java:35) at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet up.java:105) at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:102) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:98) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa tabaseSetup.java:91) at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig uration.java:782) at junit.extensions.TestSetup$1.protect(TestSetup.java:22) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57 ) 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 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 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 ) 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) 7) testEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTest )junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suites .All\system\dbsqlauth\db.lck at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas eSetup.java:130) at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba seSetup.java:35) at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet up.java:105) at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:102) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:98) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa tabaseSetup.java:91) at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig uration.java:782) at junit.extensions.TestSetup$1.protect(TestSetup.java:22) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57 ) 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 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 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 ) 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) 8) testReEncrypt(org.apache.derbyTesting.functionTests.tests.jdbcapi.DboPowersTe st)junit.framework.AssertionFailedError: C:\cygwin\home\Tiago\Derby\Testing\suit es.All\system\dbsqlauth\db.lck at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDir(DropDatabas eSetup.java:130) at org.apache.derbyTesting.junit.DropDatabaseSetup.access$000(DropDataba seSetup.java:35) at org.apache.derbyTesting.junit.DropDatabaseSetup$1.run(DropDatabaseSet up.java:105) at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:102) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDirectory(DropD atabaseSetup.java:98) at org.apache.derbyTesting.junit.DropDatabaseSetup.removeDatabase(DropDa tabaseSetup.java:91) at org.apache.derbyTesting.junit.TestConfiguration$7.tearDown(TestConfig uration.java:782) at junit.extensions.TestSetup$1.protect(TestSetup.java:22) at junit.extensions.TestSetup.run(TestSetup.java:25) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57 ) 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 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 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 ) 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) FAILURES!!! Tests run: 11068, Failures: 8, Errors: 32 -------------------------- 8< -------------------------- This is on one of the runs. The other ran flawlessly: Time: 17.805,672 OK (11161 tests) I also have a three simultaneous runs going in an Ubuntu environment.
          Hide
          Kathey Marsden added a comment -

          It is interesting that the last thing these fixtures do is call bringDbDown(), so even if there was a problem where network server wasn't brought down for some reason, the database removal should work ok. I am not sure if database shutdown backgrounds any operations which may not have completed before we attempt to remove the database. It seems that if that were a problem we would have seen this issue with regular runs.

          I spoke to Tiago and he cannot reproduce the problem. I will look at the patch and see if I can spot anything and may go ahead and commit it and then we can file another issue for the database removal problem in parallel runs if it comes up again.

          Show
          Kathey Marsden added a comment - It is interesting that the last thing these fixtures do is call bringDbDown(), so even if there was a problem where network server wasn't brought down for some reason, the database removal should work ok. I am not sure if database shutdown backgrounds any operations which may not have completed before we attempt to remove the database. It seems that if that were a problem we would have seen this issue with regular runs. I spoke to Tiago and he cannot reproduce the problem. I will look at the patch and see if I can spot anything and may go ahead and commit it and then we can file another issue for the database removal problem in parallel runs if it comes up again.
          Hide
          Kathey Marsden added a comment -

          Hi Tiago,

          Here's some comments on the patch DERBY-4217.basePort.patch.
          ReplicationRun:
          On line 69, you changed the masterServerPort from getPort() to getAlternativePort(). Why doesn't getPort() work in this case. Regarding the previous change of ports in initEnvironment() I don't quite understand why that was being done. It would be good if someone familiar with replication could take a look and see if it is ok like you have it or if we need to make yet another call to getNextAlternativePort() in initEnvironment.

          TestConfiguration:
          I think in getNextAvailablePort() we should throw an assertion saying something like "Port x exceeeds expected maximum MAX_PORTS_USED insted of returning -1. You may need to update MAX_PORTS_USED and the Wiki page at http://wiki.apache.org/db-derby/DerbyJUnitTesting if test runs now require more available ports"

          Why do we need to create a NetworkServerControl and try to ping if we are requiring the user allocate enough available ports? This part makes me especially nervous since it might make DERBY-4053 more likely to hit.

          Show
          Kathey Marsden added a comment - Hi Tiago, Here's some comments on the patch DERBY-4217 .basePort.patch. ReplicationRun: On line 69, you changed the masterServerPort from getPort() to getAlternativePort(). Why doesn't getPort() work in this case. Regarding the previous change of ports in initEnvironment() I don't quite understand why that was being done. It would be good if someone familiar with replication could take a look and see if it is ok like you have it or if we need to make yet another call to getNextAlternativePort() in initEnvironment. TestConfiguration: I think in getNextAvailablePort() we should throw an assertion saying something like "Port x exceeeds expected maximum MAX_PORTS_USED insted of returning -1. You may need to update MAX_PORTS_USED and the Wiki page at http://wiki.apache.org/db-derby/DerbyJUnitTesting if test runs now require more available ports" Why do we need to create a NetworkServerControl and try to ping if we are requiring the user allocate enough available ports? This part makes me especially nervous since it might make DERBY-4053 more likely to hit.
          Hide
          Tiago R. Espinha added a comment -

          Hello Kathey,

          1) You are right, the masterServerPort can use the basePort instead, that way we save one port in the total number of ports required.

          Regarding initEnvironment(), it seems to me that that was something along the lines of what was done in the TestConfiguration's empty constructor. The values are initialized to bogus values and they are then set to the actual ones. But I'm not sure, so perhaps Ole can give us some insight on this since he seems to be more familiar with replication?

          I think we can safely get rid of the assignments done in initEnvironment() since apparently they are very much useless. (They are useless on my patch and suites.All runs fine, with the correct ports for the ReplicationSuite)

          2) On TestConfiguration we don't have access to any kind of assertions since it's a standalone class that doesn't extend from any of the test cases. Also, throwing an exception at the getNextAvailablePort() will require that we catch the exception wherever the method is invoked. I'm not sure this is a very good practice here...

          Returning -1 was the way I found to make the test run crash. It isn't very verboseish I admit, but I don't like the solution of throwing an exception either... especially because it won't get dealt with automatically by JUnit.

          3) Regarding the last topic, do note that I don't start a server. I just create an instance of the NetworkServerControl and try to ping it. The goal here is to ensure that there isn't something else bound on that port. If the ping succeeds then an exception isn't thrown and -1 is returned, causing a failure. But this is of course arguable and realistically speaking we probably don't really need it. If the port has something bound on it, then the test requesting the port will naturally crash on its own.

          Show
          Tiago R. Espinha added a comment - Hello Kathey, 1) You are right, the masterServerPort can use the basePort instead, that way we save one port in the total number of ports required. Regarding initEnvironment(), it seems to me that that was something along the lines of what was done in the TestConfiguration's empty constructor. The values are initialized to bogus values and they are then set to the actual ones. But I'm not sure, so perhaps Ole can give us some insight on this since he seems to be more familiar with replication? I think we can safely get rid of the assignments done in initEnvironment() since apparently they are very much useless. (They are useless on my patch and suites.All runs fine, with the correct ports for the ReplicationSuite) 2) On TestConfiguration we don't have access to any kind of assertions since it's a standalone class that doesn't extend from any of the test cases. Also, throwing an exception at the getNextAvailablePort() will require that we catch the exception wherever the method is invoked. I'm not sure this is a very good practice here... Returning -1 was the way I found to make the test run crash. It isn't very verboseish I admit, but I don't like the solution of throwing an exception either... especially because it won't get dealt with automatically by JUnit. 3) Regarding the last topic, do note that I don't start a server. I just create an instance of the NetworkServerControl and try to ping it. The goal here is to ensure that there isn't something else bound on that port. If the ping succeeds then an exception isn't thrown and -1 is returned, causing a failure. But this is of course arguable and realistically speaking we probably don't really need it. If the port has something bound on it, then the test requesting the port will naturally crash on its own.
          Hide
          Kathey Marsden added a comment -

          I think you can access an assertion with a static method by just calling Assert.fail(...) instead of return -1. I think because it is a runtime exception so should all work ok and be handled properly by JUnit. (junit.framework.Assert is already imported into TestConfiguration)

          I really think the NetworkServerControl.ping code should go and we should just let it fail as you describe if someone makes a mistake and uses an in use port. If the ping occurs on some non-derby used port it is likely to hang. Without the code it is more likely to fail when starting the server which will just give a bind error port in use message which will be more helpful.

          My untrained eye sees your analysis of initEnvironment as correct, but I agree it would be good to get input on the Replication changes from Ole, Dag or someone more familiar with replication to make sure.

          Show
          Kathey Marsden added a comment - I think you can access an assertion with a static method by just calling Assert.fail(...) instead of return -1. I think because it is a runtime exception so should all work ok and be handled properly by JUnit. (junit.framework.Assert is already imported into TestConfiguration) I really think the NetworkServerControl.ping code should go and we should just let it fail as you describe if someone makes a mistake and uses an in use port. If the ping occurs on some non-derby used port it is likely to hang. Without the code it is more likely to fail when starting the server which will just give a bind error port in use message which will be more helpful. My untrained eye sees your analysis of initEnvironment as correct, but I agree it would be good to get input on the Replication changes from Ole, Dag or someone more familiar with replication to make sure.
          Hide
          Tiago R. Espinha added a comment -

          Attaching a new patch that has Kathey's considerations implemented. I have also changed the way lastAssignedPort is initialized.

          I was wondering about whether it would be acceptable to encapsulate the following code into a BaseTestCase method.
          static {
          String port = BaseTestCase.getSystemProperty("derby.tests.basePort");
          if (port == null)

          { lastAssignedPort = DEFAULT_PORT; }

          else

          { lastAssignedPort = Integer.parseInt(port); }

          }

          It is repeated three times throughout TestConfiguration and I thought that maybe we could have a getBasePort() in BaseTestCase that simply retrieves that property. Otherwise we can just leave it as is.

          Show
          Tiago R. Espinha added a comment - Attaching a new patch that has Kathey's considerations implemented. I have also changed the way lastAssignedPort is initialized. I was wondering about whether it would be acceptable to encapsulate the following code into a BaseTestCase method. static { String port = BaseTestCase.getSystemProperty("derby.tests.basePort"); if (port == null) { lastAssignedPort = DEFAULT_PORT; } else { lastAssignedPort = Integer.parseInt(port); } } It is repeated three times throughout TestConfiguration and I thought that maybe we could have a getBasePort() in BaseTestCase that simply retrieves that property. Otherwise we can just leave it as is.
          Hide
          Ole Solberg added a comment -

          Re DERBY-4217.basePort.patch:
          ReplicationRun:

          The masterServerPort should be the "normal" port used, i.e. getPort() would be the correct way to set it - as it is now.

          You are correct that the assignments done to slaveServerPort and slaveReplPort in initEnvironment() were unneccessary (and confusing...).

          Show
          Ole Solberg added a comment - Re DERBY-4217 .basePort.patch: ReplicationRun: The masterServerPort should be the "normal" port used, i.e. getPort() would be the correct way to set it - as it is now. You are correct that the assignments done to slaveServerPort and slaveReplPort in initEnvironment() were unneccessary (and confusing...).
          Hide
          Tiago R. Espinha added a comment -

          Thank you Ole for your input.

          I've done another run in Ubuntu with two instances of suites.All (on different ports and folders) and this time I got this error in one of the runs:
          ------------8<-----------------------
          1) testDerbynetJarAttributeAlpha(org.apache.derbyTesting.functionTests.tests.management.VersionMBeanTest)java.security.PrivilegedActionException: javax.management.InstanceNotFoundException: org.apache.derby:type=Version,system=c013800d-0121-f31c-770b-ffff8baa048d,jar=derbynet.jar
          at java.security.AccessController.doPrivileged(Native Method)
          at org.apache.derbyTesting.functionTests.tests.management.MBeanTest.getAttribute(MBeanTest.java:379)
          at org.apache.derbyTesting.functionTests.tests.management.MBeanTest.checkBooleanAttributeValue(MBeanTest.java:431)
          at org.apache.derbyTesting.functionTests.tests.management.VersionMBeanTest.testDerbynetJarAttributeAlpha(VersionMBeanTest.java:124)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
          at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:109)
          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)
          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 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)
          Caused by: javax.management.InstanceNotFoundException: org.apache.derby:type=Version,system=c013800d-0121-f31c-770b-ffff8baa048d,jar=derbynet.jar
          at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getMBean(DefaultMBeanServerInterceptor.java:1094)
          at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getAttribute(DefaultMBeanServerInterceptor.java:662)
          at com.sun.jmx.mbeanserver.JmxMBeanServer.getAttribute(JmxMBeanServer.java:638)
          at org.apache.derbyTesting.functionTests.tests.management.MBeanTest$4.run(MBeanTest.java:382)
          ... 42 more

          FAILURES!!!
          Tests run: 11169, Failures: 0, Errors: 1
          ----------------------------8<-------------------

          I don't think this is related though?

          Show
          Tiago R. Espinha added a comment - Thank you Ole for your input. I've done another run in Ubuntu with two instances of suites.All (on different ports and folders) and this time I got this error in one of the runs: ------------ 8< ----------------------- 1) testDerbynetJarAttributeAlpha(org.apache.derbyTesting.functionTests.tests.management.VersionMBeanTest)java.security.PrivilegedActionException: javax.management.InstanceNotFoundException: org.apache.derby:type=Version,system=c013800d-0121-f31c-770b-ffff8baa048d,jar=derbynet.jar at java.security.AccessController.doPrivileged(Native Method) at org.apache.derbyTesting.functionTests.tests.management.MBeanTest.getAttribute(MBeanTest.java:379) at org.apache.derbyTesting.functionTests.tests.management.MBeanTest.checkBooleanAttributeValue(MBeanTest.java:431) at org.apache.derbyTesting.functionTests.tests.management.VersionMBeanTest.testDerbynetJarAttributeAlpha(VersionMBeanTest.java:124) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at org.apache.derbyTesting.junit.BaseTestCase.runBare(BaseTestCase.java:109) 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) 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 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) Caused by: javax.management.InstanceNotFoundException: org.apache.derby:type=Version,system=c013800d-0121-f31c-770b-ffff8baa048d,jar=derbynet.jar at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getMBean(DefaultMBeanServerInterceptor.java:1094) at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getAttribute(DefaultMBeanServerInterceptor.java:662) at com.sun.jmx.mbeanserver.JmxMBeanServer.getAttribute(JmxMBeanServer.java:638) at org.apache.derbyTesting.functionTests.tests.management.MBeanTest$4.run(MBeanTest.java:382) ... 42 more FAILURES!!! Tests run: 11169, Failures: 0, Errors: 1 ---------------------------- 8< ------------------- I don't think this is related though?
          Hide
          Kathey Marsden added a comment -

          Hi Tiago

          • I think a static variable and single initialization for basePort would be good, but it should be in TestConfiguration.
          • In TestConfiguration there is still an import for NetworkServerControl and UnknownHostException which could be removed.
          • I think DEFAULT_JMX_PORT can go away too.
          • Your failure looks like a known issue DERBY-3689
          Show
          Kathey Marsden added a comment - Hi Tiago I think a static variable and single initialization for basePort would be good, but it should be in TestConfiguration. In TestConfiguration there is still an import for NetworkServerControl and UnknownHostException which could be removed. I think DEFAULT_JMX_PORT can go away too. Your failure looks like a known issue DERBY-3689
          Hide
          Tiago R. Espinha added a comment -

          I seem to keep hitting random failures here. Here's what I got this time with the MAX_PORTS_USED set to its final value:
          -----------------8<----------------------------
          There was 1 failure:
          1) testStressMulti(org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest)junit.framework.AssertionFailedError: Caused by:
          java.sql.SQLNonTransientConnectionException: Network protocol exception: DSS length not 0 at end of same id chain parse. The connection has been terminated.
          at org.apache.derby.client.am.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:70)
          at org.apache.derby.client.am.SqlException.getSQLException(SqlException.java:358)
          at org.apache.derby.client.am.Statement.executeQuery(Statement.java:483)
          at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.select(StressMultiTest.java:535)
          at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.run(StressMultiTest.java:409)
          at java.lang.Thread.run(Thread.java:619)
          Caused by: org.apache.derby.client.am.DisconnectException: Network protocol exception: DSS length not 0 at end of same id chain parse. The connection has been terminated.
          at org.apache.derby.client.net.Reply.endOfSameIdChainData(Reply.java:1174)
          at org.apache.derby.client.net.NetStatementReply.readOpenQuery(NetStatementReply.java:66)
          at org.apache.derby.client.net.StatementReply.readOpenQuery(StatementReply.java:50)
          at org.apache.derby.client.net.NetStatement.readOpenQuery_(NetStatement.java:156)
          at org.apache.derby.client.am.Statement.readOpenQuery(Statement.java:1478)
          at org.apache.derby.client.am.Statement.flowExecute(Statement.java:2095)
          at org.apache.derby.client.am.Statement.executeQueryX(Statement.java:489)
          at org.apache.derby.client.am.Statement.executeQuery(Statement.java:474)
          ... 3 more

          at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest.handleException(StressMultiTest.java:331)
          at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest.access$200(StressMultiTest.java:70)
          at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.run(StressMultiTest.java:425)
          at java.lang.Thread.run(Thread.java:619)

          Show
          Tiago R. Espinha added a comment - I seem to keep hitting random failures here. Here's what I got this time with the MAX_PORTS_USED set to its final value: ----------------- 8< ---------------------------- There was 1 failure: 1) testStressMulti(org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest)junit.framework.AssertionFailedError: Caused by: java.sql.SQLNonTransientConnectionException: Network protocol exception: DSS length not 0 at end of same id chain parse. The connection has been terminated. at org.apache.derby.client.am.SQLExceptionFactory40.getSQLException(SQLExceptionFactory40.java:70) at org.apache.derby.client.am.SqlException.getSQLException(SqlException.java:358) at org.apache.derby.client.am.Statement.executeQuery(Statement.java:483) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.select(StressMultiTest.java:535) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.run(StressMultiTest.java:409) at java.lang.Thread.run(Thread.java:619) Caused by: org.apache.derby.client.am.DisconnectException: Network protocol exception: DSS length not 0 at end of same id chain parse. The connection has been terminated. at org.apache.derby.client.net.Reply.endOfSameIdChainData(Reply.java:1174) at org.apache.derby.client.net.NetStatementReply.readOpenQuery(NetStatementReply.java:66) at org.apache.derby.client.net.StatementReply.readOpenQuery(StatementReply.java:50) at org.apache.derby.client.net.NetStatement.readOpenQuery_(NetStatement.java:156) at org.apache.derby.client.am.Statement.readOpenQuery(Statement.java:1478) at org.apache.derby.client.am.Statement.flowExecute(Statement.java:2095) at org.apache.derby.client.am.Statement.executeQueryX(Statement.java:489) at org.apache.derby.client.am.Statement.executeQuery(Statement.java:474) ... 3 more at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest.handleException(StressMultiTest.java:331) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest.access$200(StressMultiTest.java:70) at org.apache.derbyTesting.functionTests.tests.multi.StressMultiTest$StressMultiRunnable.run(StressMultiTest.java:425) at java.lang.Thread.run(Thread.java:619)
          Hide
          Kathey Marsden added a comment -

          Take a look in the derby.log to see if maybe you can find the root cause of the failure. It may be DERBY-3916 or DERBY-3757.

          Show
          Kathey Marsden added a comment - Take a look in the derby.log to see if maybe you can find the root cause of the failure. It may be DERBY-3916 or DERBY-3757 .
          Hide
          Tiago R. Espinha added a comment -

          Attaching the final patch, with the MAX_PORTS_USED set to 10. This is the number of ports used in suites.All.

          Show
          Tiago R. Espinha added a comment - Attaching the final patch, with the MAX_PORTS_USED set to 10. This is the number of ports used in suites.All.
          Hide
          Kathey Marsden added a comment -

          When I ran suites.All with the patch I saw these two failures:
          There were 2 failures:
          1) JMXTest:clientjunit.framework.AssertionFailedError: Timed out waiting for network server to start
          orkServer exitCode=1
          at org.apache.derbyTesting.junit.NetworkServerTestSetup.setUp(NetworkServerTestSetup.java:20
          at junit.extensions.TestSetup$1.protect(TestSetup.java:18)
          at junit.extensions.TestSetup.run(TestSetup.java:16)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          at junit.extensions.TestSetup.run(TestSetup.java:16)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          at junit.extensions.TestSetup.run(TestSetup.java:16)
          2) ManagementMBeanTest:clientjunit.framework.AssertionFailedError: Timed out waiting for network ser
          SpawnedNetworkServer exitCode=1
          at org.apache.derbyTesting.junit.NetworkServerTestSetup.setUp(NetworkServerTestSetup.java:20
          at junit.extensions.TestSetup$1.protect(TestSetup.java:18)
          at junit.extensions.TestSetup.run(TestSetup.java:16)
          at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          at junit.extensions.TestSetup.run(TestSetup.java:16)
          at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22)
          at junit.extensions.TestSetup$1.protect(TestSetup.java:19)
          at junit.extensions.TestSetup.run(TestSetup.java:16)

          FAILURES!!!
          Tests run: 11166, Failures: 2, Errors: 0

          I don't think it is related, but I am going to run again just to make sure.
          The patch looks fine except that the comment
          /* CHANGE THIS BEFORE FINAL PATCH! Set to actual value! */
          should be changed to something else now that the value has been changed.

          Show
          Kathey Marsden added a comment - When I ran suites.All with the patch I saw these two failures: There were 2 failures: 1) JMXTest:clientjunit.framework.AssertionFailedError: Timed out waiting for network server to start orkServer exitCode=1 at org.apache.derbyTesting.junit.NetworkServerTestSetup.setUp(NetworkServerTestSetup.java:20 at junit.extensions.TestSetup$1.protect(TestSetup.java:18) at junit.extensions.TestSetup.run(TestSetup.java:16) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19) at junit.extensions.TestSetup.run(TestSetup.java:16) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19) at junit.extensions.TestSetup.run(TestSetup.java:16) 2) ManagementMBeanTest:clientjunit.framework.AssertionFailedError: Timed out waiting for network ser SpawnedNetworkServer exitCode=1 at org.apache.derbyTesting.junit.NetworkServerTestSetup.setUp(NetworkServerTestSetup.java:20 at junit.extensions.TestSetup$1.protect(TestSetup.java:18) at junit.extensions.TestSetup.run(TestSetup.java:16) at org.apache.derbyTesting.junit.BaseTestSetup.run(BaseTestSetup.java:57) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19) at junit.extensions.TestSetup.run(TestSetup.java:16) at junit.extensions.TestDecorator.basicRun(TestDecorator.java:22) at junit.extensions.TestSetup$1.protect(TestSetup.java:19) at junit.extensions.TestSetup.run(TestSetup.java:16) FAILURES!!! Tests run: 11166, Failures: 2, Errors: 0 I don't think it is related, but I am going to run again just to make sure. The patch looks fine except that the comment /* CHANGE THIS BEFORE FINAL PATCH! Set to actual value! */ should be changed to something else now that the value has been changed.
          Hide
          Kathey Marsden added a comment -

          On rerun I got just the JMXTest failure. netstat does not show any ports taken that should interfere with 1527 + 9 which I think is what the test would take. management._Suite seems to run fine on its own.

          Show
          Kathey Marsden added a comment - On rerun I got just the JMXTest failure. netstat does not show any ports taken that should interfere with 1527 + 9 which I think is what the test would take. management._Suite seems to run fine on its own.
          Hide
          Kathey Marsden added a comment -

          I was running on Windows XP with IBM 1.6. Can someone else running on Windows XP give the basePort patch a try and I will do another run too.

          Show
          Kathey Marsden added a comment - I was running on Windows XP with IBM 1.6. Can someone else running on Windows XP give the basePort patch a try and I will do another run too.
          Hide
          Tiago R. Espinha added a comment -

          Removed the comment reported by Kathey. I left it in capital letters and still managed to miss it # - luckily I didn't forget to change the constant.

          Show
          Tiago R. Espinha added a comment - Removed the comment reported by Kathey. I left it in capital letters and still managed to miss it # - luckily I didn't forget to change the constant.
          Hide
          Mike Matrigali added a comment -

          i ran against latest base patch and got no errors on a full test suite run against ibm16 jvm on XP.

          Show
          Mike Matrigali added a comment - i ran against latest base patch and got no errors on a full test suite run against ibm16 jvm on XP.
          Hide
          Tiago R. Espinha added a comment -

          Thank you for your testing Mike. Kathey also reported on IRC that her run also got no errors, so I think this patch is ready for prime time.

          Show
          Tiago R. Espinha added a comment - Thank you for your testing Mike. Kathey also reported on IRC that her run also got no errors, so I think this patch is ready for prime time.
          Hide
          Kathey Marsden added a comment -

          Resolving this issue. I might look later at backporting it. Developers can now do concurrent Suites.All runs by specifying derby.tests.basePort as described at
          http://wiki.apache.org/db-derby/DerbyJUnitTesting

          Please give it a try!

          Tiago, maybe we should update the Wiki page in the regular run section to say that by default the tests use ten ports starting with 1527

          Show
          Kathey Marsden added a comment - Resolving this issue. I might look later at backporting it. Developers can now do concurrent Suites.All runs by specifying derby.tests.basePort as described at http://wiki.apache.org/db-derby/DerbyJUnitTesting Please give it a try! Tiago, maybe we should update the Wiki page in the regular run section to say that by default the tests use ten ports starting with 1527
          Hide
          Tiago R. Espinha added a comment -

          I have added some more info on the wiki page.

          Thank you Kathey for committing this! I hope it is of good use for people running concurrent tests.

          Kathey also suggested at some point that we can run parts of suites.All separately which can speed up the whole process. This isn't possible yet through JUnit alone, but we can just fire up several tests manually.

          Show
          Tiago R. Espinha added a comment - I have added some more info on the wiki page. Thank you Kathey for committing this! I hope it is of good use for people running concurrent tests. Kathey also suggested at some point that we can run parts of suites.All separately which can speed up the whole process. This isn't possible yet through JUnit alone, but we can just fire up several tests manually.
          Hide
          Tiago R. Espinha added a comment -

          Reopening this issue. When ports higher than 9999 are used in the SysinfoTest, the test would fail because of a wrong regex search expression.

          Show
          Tiago R. Espinha added a comment - Reopening this issue. When ports higher than 9999 are used in the SysinfoTest, the test would fail because of a wrong regex search expression.
          Hide
          Tiago R. Espinha added a comment -

          Attaching the patch to fix the regex issue.

          Show
          Tiago R. Espinha added a comment - Attaching the patch to fix the regex issue.
          Hide
          Kathey Marsden added a comment -

          Resolving this issue. The final patch to fix the regex issue with large ports has been committed.

          Show
          Kathey Marsden added a comment - Resolving this issue. The final patch to fix the regex issue with large ports has been committed.
          Hide
          Tiago R. Espinha added a comment -

          The patch has been committed and the issue is fixed. Closing.

          Show
          Tiago R. Espinha added a comment - The patch has been committed and the issue is fixed. Closing.

            People

            • Assignee:
              Tiago R. Espinha
              Reporter:
              Tiago R. Espinha
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development