Lucene - Core
  1. Lucene - Core
  2. LUCENE-2248

Tests using Version.LUCENE_CURRENT will produce problems in backwards branch, when development for 3.2 starts

    Details

    • Lucene Fields:
      New

      Description

      A lot of tests for the most-recent functionality in Lucene use Version.LUCENE_CURRENT, which is fine in trunk, as we use the most recent version without hassle and changing this in later versions.

      The problem is, if we copy these tests to backwards branch after 3.1 is out and then start to improve analyzers, we then will have the maintenance hell for backwards tests. And we loose backward compatibility testing for older versions. If we would specify a specific version like LUCENE_31 in our tests, after moving to backwards they must work without any changes!

      To not always modify all tests after a new version comes out (e.g. after switching to 3.2 dev), I propose to do the following:

      • declare a static final Version TEST_VERSION = Version.LUCENE_CURRENT (or better) Version.LUCENE_31 in LuceneTestCase(4J).
      • change all tests that use Version.LUCENE_CURRENT using eclipse refactor to use this constant and remove unneeded import statements.

      When we then move the tests to backward we must only change one line, depending on how we define this constant:

      • If in trunk LuceneTestCase it's Version.LUCENE_CURRENT, we just change the backwards branch to use the version numer of the released thing.
      • If trunk already uses the LUCENE_31 constant (I prefer this), we do not need to change backwards, but instead when switching version numbers we just move trunk forward to the next major version (after added to Version enum).
      1. LUCENE-2248.patch
        367 kB
        Uwe Schindler
      2. LUCENE-2248.patch
        366 kB
        Uwe Schindler
      3. LUCENE-2248.patch
        365 kB
        Uwe Schindler
      4. LUCENE-2248.patch
        10 kB
        Uwe Schindler
      5. LUCENE-2248.patch
        10 kB
        Uwe Schindler

        Activity

        Uwe Schindler made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Shai Erera made changes -
        Component/s contrib/wikipedia [ 12312097 ]
        Shai Erera made changes -
        Component/s contrib/analyzers [ 12312333 ]
        Mark Thomas made changes -
        Workflow Default workflow, editable Closed status [ 12564255 ] jira [ 12584047 ]
        Mark Thomas made changes -
        Workflow jira [ 12497995 ] Default workflow, editable Closed status [ 12564255 ]
        Uwe Schindler made changes -
        Attachment LUCENE-2248.patch [ 12435442 ]
        Hide
        Uwe Schindler added a comment -

        I forgot to add the latest patch.

        Show
        Uwe Schindler added a comment - I forgot to add the latest patch.
        Uwe Schindler made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Uwe Schindler added a comment -

        Committed revision: 908496

        Show
        Uwe Schindler added a comment - Committed revision: 908496
        Uwe Schindler made changes -
        Attachment LUCENE-2248.patch [ 12435165 ]
        Hide
        Uwe Schindler added a comment -

        Updated patch, I missed some LUCENE_CURRENT occurences. Now all are catched and changed. My sed-command was wrong (missed to replace all occurences in one line)

        All test pass still. Will commit if nobody objects this evening.

        Show
        Uwe Schindler added a comment - Updated patch, I missed some LUCENE_CURRENT occurences. Now all are catched and changed. My sed-command was wrong (missed to replace all occurences in one line) All test pass still. Will commit if nobody objects this evening.
        Uwe Schindler made changes -
        Attachment LUCENE-2248.patch [ 12435163 ]
        Hide
        Uwe Schindler added a comment -

        Here the patch for all core tests.

        I also did a eclipse import cleanup to remove the unused Version imports. Now the code is also clean from a full-specified class name (o.a.l.util.Version.LUCENE_CURRENT), I introduced during my 3.0 refactoring (I did not add the import statements at this time).

        I would like to commit this soon, if nobody objects, because the patch could get out of sync very fast.

        Show
        Uwe Schindler added a comment - Here the patch for all core tests. I also did a eclipse import cleanup to remove the unused Version imports. Now the code is also clean from a full-specified class name (o.a.l.util.Version.LUCENE_CURRENT), I introduced during my 3.0 refactoring (I did not add the import statements at this time). I would like to commit this soon, if nobody objects, because the patch could get out of sync very fast.
        Hide
        Simon Willnauer added a comment -

        Simon, if you like you can use it as basis and start with contrib.

        will do...

        Show
        Simon Willnauer added a comment - Simon, if you like you can use it as basis and start with contrib. will do...
        Uwe Schindler made changes -
        Attachment LUCENE-2248.patch [ 12435085 ]
        Hide
        Uwe Schindler added a comment -

        Patch with updated constant name.

        Simon, if you like you can use it as basis and start with contrib.

        Show
        Uwe Schindler added a comment - Patch with updated constant name. Simon, if you like you can use it as basis and start with contrib.
        Uwe Schindler made changes -
        Assignee Uwe Schindler [ thetaphi ]
        Hide
        Uwe Schindler added a comment -

        Simon: I opened a sub-issue for contrib and assigned you to it!

        I will change to TEST_VERSION_CURRENT and then run eclipse to do the refactoring.

        Show
        Uwe Schindler added a comment - Simon: I opened a sub-issue for contrib and assigned you to it! I will change to TEST_VERSION_CURRENT and then run eclipse to do the refactoring.
        Hide
        Simon Willnauer added a comment -

        Patch looks good uwe, I think we should reflect its purpose in the name maybe TEST_VERSION_LATEST or TEST_VERSION_CURRENT

        simon

        Show
        Simon Willnauer added a comment - Patch looks good uwe, I think we should reflect its purpose in the name maybe TEST_VERSION_LATEST or TEST_VERSION_CURRENT simon
        Uwe Schindler made changes -
        Attachment LUCENE-2248.patch [ 12435083 ]
        Hide
        Uwe Schindler added a comment -

        Here my first patch. Please tell me which name for the static constant should be used, I use CURRENT_VERSION. Maybe something with "test" in it?

        I transformed TestCharArraySet as a demonstation.

        Show
        Uwe Schindler added a comment - Here my first patch. Please tell me which name for the static constant should be used, I use CURRENT_VERSION. Maybe something with "test" in it? I transformed TestCharArraySet as a demonstation.
        Hide
        Simon Willnauer added a comment -

        Uwe, as I already said while we where discussion this, I would add the version to LuceneTestCase (or equivalent for JU4) and then we can do the tests in sub-issues which prevents those super huge patches.

        thoughts?!

        Show
        Simon Willnauer added a comment - Uwe, as I already said while we where discussion this, I would add the version to LuceneTestCase (or equivalent for JU4) and then we can do the tests in sub-issues which prevents those super huge patches. thoughts?!
        Hide
        Michael McCandless added a comment -

        +1

        Show
        Michael McCandless added a comment - +1
        Uwe Schindler made changes -
        Field Original Value New Value
        Summary Tests using Version.LUCENE_CURRENT will produce problems in backwards branch, wen development for 3.2 starts Tests using Version.LUCENE_CURRENT will produce problems in backwards branch, when development for 3.2 starts
        Description A lot of tests for the most-recent functionality in Lucene use Version.LUCENE_CURRENT, which is fine in trunk, as we use the most recent version without hassle and changing this in later versions.

        The problem is, if we copy these tests to backwards branch after 3.1 is out and then start to improve analyzers, we then will have the maintenance hell for backwards tests. And we loose backward compatibility testing for older versions. If we would specify a specific version like LUCENE_31 in our tests, after moving to backwards they must work without any changes!

        To not always modify all tests after a new version comes out (e.g. after switching to 3.2 dev), I propse to do the following:
        - declare a static final Version TEST_VERSION = Version.LUCENE_CURRENT (or better) Version.LUCENE_31 in LuceneTestCase(4J).
        - change all tests that use Version.LUCENE_CURRENT using eclipse refactor to use this constant and remove unneeded import statements.

        When we then move the tests to backward we must only change one line, depending on how we define this constant:
        - If in trunk LuceneTestCase it's Version.LUCENE_CURRENT, we just change the backwards branch to use the version numer of the released thing.
        - If trunk already uses the LUCENE_31 constant (I prefer this), we do not need to change backwards, but instead when switching version numbers we just move trunk forward to the next major version (after added to Version enum).
        A lot of tests for the most-recent functionality in Lucene use Version.LUCENE_CURRENT, which is fine in trunk, as we use the most recent version without hassle and changing this in later versions.

        The problem is, if we copy these tests to backwards branch after 3.1 is out and then start to improve analyzers, we then will have the maintenance hell for backwards tests. And we loose backward compatibility testing for older versions. If we would specify a specific version like LUCENE_31 in our tests, after moving to backwards they must work without any changes!

        To not always modify all tests after a new version comes out (e.g. after switching to 3.2 dev), I propose to do the following:
        - declare a static final Version TEST_VERSION = Version.LUCENE_CURRENT (or better) Version.LUCENE_31 in LuceneTestCase(4J).
        - change all tests that use Version.LUCENE_CURRENT using eclipse refactor to use this constant and remove unneeded import statements.

        When we then move the tests to backward we must only change one line, depending on how we define this constant:
        - If in trunk LuceneTestCase it's Version.LUCENE_CURRENT, we just change the backwards branch to use the version numer of the released thing.
        - If trunk already uses the LUCENE_31 constant (I prefer this), we do not need to change backwards, but instead when switching version numbers we just move trunk forward to the next major version (after added to Version enum).
        Uwe Schindler created issue -

          People

          • Assignee:
            Uwe Schindler
            Reporter:
            Uwe Schindler
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development