Lucene - Core
  1. Lucene - Core
  2. LUCENE-1529

back-compat tests ("ant test-tag") should test JAR drop-in-ability

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.9
    • Fix Version/s: 2.9
    • Component/s: general/build
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      We now test back-compat with "ant test-tag", which is very useful for
      catching breaks in back compat before committing.

      However, that currently checks out "src/test" sources and then
      compiles them against the trunk JAR, and runs the tests. Whereas our
      back compat policy:

      http://wiki.apache.org/lucene-java/BackwardsCompatibility

      states that no recompilation is required on upgrading to a new JAR.
      Ie you should be able to drop in the new JAR in place of your old one
      and things should work fine.

      So... we should fix "ant test-tag" to:

      • Do full checkout of core sources & tests from the back-compat-tag
      • Compile the JAR from the back-compat sources
      • Compile the tests against that back-compat JAR
      • Swap in the trunk JAR
      • Run the tests
      1. lucene-1529.patch
        3 kB
        Michael Busch

        Activity

        Hide
        Michael Busch added a comment -

        Changes 'test-tag' in build.xml to executes the compatibility tests like Mike described here.

        Show
        Michael Busch added a comment - Changes 'test-tag' in build.xml to executes the compatibility tests like Mike described here.
        Hide
        Michael Busch added a comment -

        Committed revision 756374.

        Show
        Michael Busch added a comment - Committed revision 756374.
        Hide
        Michael McCandless added a comment -

        I think something is wrong with this fix. Over in LUCENE-1593 we hit a case where (I think) the test should have failed, because we changed a method from returning void to returning something else.

        EG if you change IndexWriter.addDocument to (say) return int instead of void, then I think ant test-tag should fail, but it doesn't.

        Show
        Michael McCandless added a comment - I think something is wrong with this fix. Over in LUCENE-1593 we hit a case where (I think) the test should have failed, because we changed a method from returning void to returning something else. EG if you change IndexWriter.addDocument to (say) return int instead of void, then I think ant test-tag should fail, but it doesn't.
        Hide
        Shai Erera added a comment -

        Perhaps that's related: target test-tag has this step:

        	  <!-- compile tag tests against tag jar -->	
        	  <compile-test-macro srcdir="${tags.dir}/${tag}/src/test" destdir="${build.dir}/${tag}/classes/test"
        			  			  test.classpath="tag.test.classpath"/>
        

        Notice that test.classpath is set to tag.test.classpath, which is defined as:

          <path id="tag.test.classpath">
            <path refid="demo.classpath"/>
            <path refid="junit-path"/>
            <pathelement location="${build.dir}/${tag}/classes/test"/>
          	<pathelement location="${build.dir}/${tag}/${tag}.jar"/>
          </path>
        

        "demo.classpath" includes build/classes/demo as well as build/classes/java, which includes the current trunk's build classes.

        If I change the definition to rely only on tag.jar, demo classes and junit jar:

          <path id="tag.test.classpath">
            <path refid="junit-path"/>
            <pathelement location="${build.dir}/classes/demo"/>
            <pathelement location="${build.dir}/${tag}/${tag}.jar"/>
          </path>
        

        I get errors like this:

            [javac] Compiling 128 source files to D:\dev\lucene\lucene-trunk\build\lucene_2_4_back_compat_tests_20090320\classes\test
            [javac] D:\dev\lucene\lucene-trunk\tags\lucene_2_4_back_compat_tests_20090320\src\test\org\apache\lucene\index\TestIndexReaderReopen.java:323: cannot find symbol
            [javac] symbol  : method getSequentialSubReaders()
            [javac] location: class org.apache.lucene.index.MultiSegmentReader
            [javac]       IndexReader[] subReaders0 = ((MultiSegmentReader) reader0).getSequentialSubReaders();
            [javac]                                   ^
            [javac] D:\dev\lucene\lucene-trunk\tags\lucene_2_4_back_compat_tests_20090320\src\test\org\apache\lucene\index\TestIndexReaderReopen.java:335: cannot find symbol
            [javac] symbol  : method getSequentialSubReaders()
            [javac] location: class org.apache.lucene.index.MultiSegmentReader
            [javac]       IndexReader[] subReaders1 = ((MultiSegmentReader) reader1).getSequentialSubReaders();
            [javac]                                   ^
        

        That's because tag.jar's MultiSegmentReader does not have a getSequentialSubReaders method, however TestIndexReaderReopen calls it. Is that ok?

        Anyway, can it be that the classes/java in the classpath cause this change to not fail?

        Show
        Shai Erera added a comment - Perhaps that's related: target test-tag has this step: <!-- compile tag tests against tag jar --> <compile-test-macro srcdir= "${tags.dir}/${tag}/src/test" destdir= "${build.dir}/${tag}/classes/test" test.classpath= "tag.test.classpath" /> Notice that test.classpath is set to tag.test.classpath, which is defined as: <path id= "tag.test.classpath" > <path refid= "demo.classpath" /> <path refid= "junit-path" /> <pathelement location= "${build.dir}/${tag}/classes/test" /> <pathelement location= "${build.dir}/${tag}/${tag}.jar" /> </path> "demo.classpath" includes build/classes/demo as well as build/classes/java, which includes the current trunk's build classes. If I change the definition to rely only on tag.jar, demo classes and junit jar: <path id= "tag.test.classpath" > <path refid= "junit-path" /> <pathelement location= "${build.dir}/classes/demo" /> <pathelement location= "${build.dir}/${tag}/${tag}.jar" /> </path> I get errors like this: [javac] Compiling 128 source files to D:\dev\lucene\lucene-trunk\build\lucene_2_4_back_compat_tests_20090320\classes\test [javac] D:\dev\lucene\lucene-trunk\tags\lucene_2_4_back_compat_tests_20090320\src\test\org\apache\lucene\index\TestIndexReaderReopen.java:323: cannot find symbol [javac] symbol : method getSequentialSubReaders() [javac] location: class org.apache.lucene.index.MultiSegmentReader [javac] IndexReader[] subReaders0 = ((MultiSegmentReader) reader0).getSequentialSubReaders(); [javac] ^ [javac] D:\dev\lucene\lucene-trunk\tags\lucene_2_4_back_compat_tests_20090320\src\test\org\apache\lucene\index\TestIndexReaderReopen.java:335: cannot find symbol [javac] symbol : method getSequentialSubReaders() [javac] location: class org.apache.lucene.index.MultiSegmentReader [javac] IndexReader[] subReaders1 = ((MultiSegmentReader) reader1).getSequentialSubReaders(); [javac] ^ That's because tag.jar's MultiSegmentReader does not have a getSequentialSubReaders method, however TestIndexReaderReopen calls it. Is that ok? Anyway, can it be that the classes/java in the classpath cause this change to not fail?
        Hide
        Michael McCandless added a comment -

        If I change the definition to rely only on tag.jar, demo classes and junit jar:

        Ooh that's good progress!

        Hmm, yes we will now see compilation errors because over time we've made "legal" fixes to the test source code, corresponding to changes in 2.9. Eg, since FieldInfo is package private, we're allowed to rename omitTf to OmitTermFreqAndPositions, and if a test (TestOmitTf) is using package-private access to access omitTf, it's OK to fix that test to match the new name.

        So I think the only way to make "legal" changes to the back-compat tests is to backport [a subset of, or perhaps compile-time emulation of] the 2.9 changes onto the 2.4 branch src/java/*. Or, to fix tests not to rely on package private APIs. I think that's OK (we make such changes rarely)? I'll go and fix the back-compat branch accordingly...

        Show
        Michael McCandless added a comment - If I change the definition to rely only on tag.jar, demo classes and junit jar: Ooh that's good progress! Hmm, yes we will now see compilation errors because over time we've made "legal" fixes to the test source code, corresponding to changes in 2.9. Eg, since FieldInfo is package private, we're allowed to rename omitTf to OmitTermFreqAndPositions, and if a test (TestOmitTf) is using package-private access to access omitTf, it's OK to fix that test to match the new name. So I think the only way to make "legal" changes to the back-compat tests is to backport [a subset of, or perhaps compile-time emulation of] the 2.9 changes onto the 2.4 branch src/java/*. Or, to fix tests not to rely on package private APIs. I think that's OK (we make such changes rarely)? I'll go and fix the back-compat branch accordingly...
        Hide
        Michael McCandless added a comment -

        OK I think it's all fixed now. Thanks Shai!

        Show
        Michael McCandless added a comment - OK I think it's all fixed now. Thanks Shai!
        Hide
        Michael Busch added a comment -

        Good catch! Thanks for fixing this, Shai and Mike!

        Show
        Michael Busch added a comment - Good catch! Thanks for fixing this, Shai and Mike!
        Hide
        Shai Erera added a comment -

        Mike - you 'almost' fixed it

        I "ant clean test-tag" and still sees failures. So I checked and common-build.xml still references lucene_2_4_back_compat_tests_20090320. I do see your changes on the lucene_2_4_back_compat_tests branch, but you didn't tag those changes?

        We can either move to checkout this code from the branch (thus preventing such issues in the future), in build.xml target=download-tag, or tag the changes and update common-build.xml.

        Show
        Shai Erera added a comment - Mike - you 'almost' fixed it I "ant clean test-tag" and still sees failures. So I checked and common-build.xml still references lucene_2_4_back_compat_tests_20090320. I do see your changes on the lucene_2_4_back_compat_tests branch, but you didn't tag those changes? We can either move to checkout this code from the branch (thus preventing such issues in the future), in build.xml target=download-tag, or tag the changes and update common-build.xml.
        Hide
        Michael McCandless added a comment -

        Yes, indeed – should be fixed now (I retagged it). We can't change it to checkout the branch since people with older checkouts will suddenly see the the back-compat tests failing. That's why we switched to a tag a while back.

        Show
        Michael McCandless added a comment - Yes, indeed – should be fixed now (I retagged it). We can't change it to checkout the branch since people with older checkouts will suddenly see the the back-compat tests failing. That's why we switched to a tag a while back.

          People

          • Assignee:
            Michael Busch
            Reporter:
            Michael McCandless
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development