Derby
  1. Derby
  2. DERBY-4089

It should be possible to run unit tests right after "ant all"

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.4.2.0
    • Fix Version/s: 10.9.1.0
    • Component/s: Build tools
    • Labels:
      None

      Description

      Right now, the property "derby.junit.classpath" is empty by default. There should be an ant target which sets the correct classpath to run all tests after an initial checkout and "ant all".

      The current situation is very confusing to beginners and people who try to build Derby for the first time. For example, when running the tests, I got this exception:

      java.lang.ClassNotFoundException: org.apache.derbyTesting.junit.EnvTest

      but that class was there, the file was there, everything was correct. Googling for the error didn't turn anything up, either. It took me a while to believe that build.xml just wouldn't try to setup a classpath for the tests.

      1. derby-4089-1b-set_classpath.diff
        9 kB
        Kristian Waagan
      2. derby-4089-1a-set_classpath.diff
        9 kB
        Kristian Waagan

        Issue Links

          Activity

          Hide
          Kristian Waagan added a comment -

          I agree this problem should be addressed.
          A few of the targets do set the classpath, most don't.
          It should be possible to easily override the classpath, as there are three sources; classes, sane jars and insane jars.

          The classes directory is nice to use when doing development, as you don't have to wait for the jars to be built, but additional problems can be detected when running with the product jars.

          Show
          Kristian Waagan added a comment - I agree this problem should be addressed. A few of the targets do set the classpath, most don't. It should be possible to easily override the classpath, as there are three sources; classes, sane jars and insane jars. The classes directory is nice to use when doing development, as you don't have to wait for the jars to be built, but additional problems can be detected when running with the product jars.
          Hide
          Aaron Digulla added a comment -

          I suggest to make "classes" the default (because a novice user will use that first and it fits 95% of the use cases). Experienced developers can then override the default with sane and insane jars (nice name , as they see fit because they have the information to make such a decision.

          Show
          Aaron Digulla added a comment - I suggest to make "classes" the default (because a novice user will use that first and it fits 95% of the use cases). Experienced developers can then override the default with sane and insane jars (nice name , as they see fit because they have the information to make such a decision.
          Hide
          Kristian Waagan added a comment -

          Sounds like a good approach to me.
          A problem with using the jars, is that novice users may end up running the tests against stale code as the 'ant all' target doesn't rebuild the jar files.

          Show
          Kristian Waagan added a comment - Sounds like a good approach to me. A problem with using the jars, is that novice users may end up running the tests against stale code as the 'ant all' target doesn't rebuild the jar files.
          Hide
          Rick Hillegas added a comment -

          Hi Aaron and Kristian,

          I think that the jar files are a better default than the classpath. I think that the tests are more meaningful when they run against the production configuration (jar files)--and building the jar files does not seem like a big burden to me. I would go so far as to say that testing against the production configuration is the only meaningful barrier to checkin. Some tests only run against jar files (like the autoloading of the jdbc drivers and the automatic installation of the network security manager). The production configuration catches tricky packaging issues in which developers forget to update the build logic to supplement the jars with classes which are loaded by reflection rather than reference.

          Show
          Rick Hillegas added a comment - Hi Aaron and Kristian, I think that the jar files are a better default than the classpath. I think that the tests are more meaningful when they run against the production configuration (jar files)--and building the jar files does not seem like a big burden to me. I would go so far as to say that testing against the production configuration is the only meaningful barrier to checkin. Some tests only run against jar files (like the autoloading of the jdbc drivers and the automatic installation of the network security manager). The production configuration catches tricky packaging issues in which developers forget to update the build logic to supplement the jars with classes which are loaded by reflection rather than reference.
          Hide
          Kristian Waagan added a comment -

          Rick,

          Do you consider both the sane and the insane jars as a production configuration?

          I actually find that building the jars is a small burden when I do quick iterative development. If we choose to use the jars as the default, I think we should consider including the build target for the jars in the default target (currently 'buildsource') or 'all'.
          It's acceptable that the subsequent test run fails the first time when I haven't built the jars, but after that first time my new code will be ignored unless the jars are automatically updated. We could of course make the build detect stale jars.
          If we aim for testing of the production configuration, also for novice users / casual developers, shouldn't we also include the 'clobber' target in the default target?

          Even experienced developers have checked in code that breaks the build or tests because they didn't run the tests against jars, so I'm thinking there must be a reason why [some] people choose the classes directory over the jars. I agree that running with jars catches more issues, I feel the question is more whether it should be the default configuration to do so or not ("out-of-the-box experience"?).

          Show
          Kristian Waagan added a comment - Rick, Do you consider both the sane and the insane jars as a production configuration? I actually find that building the jars is a small burden when I do quick iterative development. If we choose to use the jars as the default, I think we should consider including the build target for the jars in the default target (currently 'buildsource') or 'all'. It's acceptable that the subsequent test run fails the first time when I haven't built the jars, but after that first time my new code will be ignored unless the jars are automatically updated. We could of course make the build detect stale jars. If we aim for testing of the production configuration, also for novice users / casual developers, shouldn't we also include the 'clobber' target in the default target? Even experienced developers have checked in code that breaks the build or tests because they didn't run the tests against jars, so I'm thinking there must be a reason why [some] people choose the classes directory over the jars. I agree that running with jars catches more issues, I feel the question is more whether it should be the default configuration to do so or not ("out-of-the-box experience"?).
          Hide
          Aaron Digulla added a comment -

          The default should work for most people, that's why it's the "default". As soon as you know your way around the source, a developer can define his own property file and override the default. But that simply doesn't work for novices.

          Also, novices have no commit rights. So I can't see how making life harder for them is going to help solve deeper development issues with Java and the Derby team.

          Show
          Aaron Digulla added a comment - The default should work for most people, that's why it's the "default". As soon as you know your way around the source, a developer can define his own property file and override the default. But that simply doesn't work for novices. Also, novices have no commit rights. So I can't see how making life harder for them is going to help solve deeper development issues with Java and the Derby team.
          Hide
          Rick Hillegas added a comment -

          Hi Kristian,

          Technically, I think that we should run the tests against both the sane and the insane jars. One configuration verifies the production configuration. The other verifies some assumptions. But over the last 4 years we haven't seen a lot of difference in tests run against sane vs. insane jars. I would say that we have seen a number of cases where the jar tests broke even though the classpath tests succeeded.

          Show
          Rick Hillegas added a comment - Hi Kristian, Technically, I think that we should run the tests against both the sane and the insane jars. One configuration verifies the production configuration. The other verifies some assumptions. But over the last 4 years we haven't seen a lot of difference in tests run against sane vs. insane jars. I would say that we have seen a number of cases where the jar tests broke even though the classpath tests succeeded.
          Hide
          Kristian Waagan added a comment -

          Attaching patch 1a, which tries to make everyone happier

          The classpath for the junit targets will be set automatically, with the following preferences:
          o if derby.junit.classpath is specified by the user, don't do anything
          o if derby.junit.classpath is unspecified
          o look for insane jars
          o look for sane jars
          o if either of them are found, set derby.junit.classpath to empty string and set derby.junit.test.jars to point to the directory with the jars
          o if no jars are located, set derby.junit.classpath to the classes-directory
          o additionally, ant will append the user's CLASSPATH environment variable to the junit classpath

          I also added an informational printout telling which classpath is being used.

          Patch ready for review, although it might be wise to see the outcome of the discussion under this issue and/or "Refactoring ant junit target to automatically set classpath" on derby-dev first.

          Show
          Kristian Waagan added a comment - Attaching patch 1a, which tries to make everyone happier The classpath for the junit targets will be set automatically, with the following preferences: o if derby.junit.classpath is specified by the user, don't do anything o if derby.junit.classpath is unspecified o look for insane jars o look for sane jars o if either of them are found, set derby.junit.classpath to empty string and set derby.junit.test.jars to point to the directory with the jars o if no jars are located, set derby.junit.classpath to the classes-directory o additionally, ant will append the user's CLASSPATH environment variable to the junit classpath I also added an informational printout telling which classpath is being used. Patch ready for review, although it might be wise to see the outcome of the discussion under this issue and/or "Refactoring ant junit target to automatically set classpath" on derby-dev first.
          Hide
          Kristian Waagan added a comment -

          Attaching patch 1b (minor changes, i.e. comments).

          Show
          Kristian Waagan added a comment - Attaching patch 1b (minor changes, i.e. comments).
          Hide
          Kristian Waagan added a comment -

          Committed patch 1b to trunk with revision 1136011.

          Show
          Kristian Waagan added a comment - Committed patch 1b to trunk with revision 1136011.
          Hide
          Knut Anders Hatlen added a comment -

          [bulk update] Close all resolved issues that haven't been updated for more than one year.

          Show
          Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development