[jira] Created: (LUCENE-2516) More clarification, improvements and correct behaviour of backwards tests

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[jira] Created: (LUCENE-2516) More clarification, improvements and correct behaviour of backwards tests

Nick Burch (Jira)
More clarification, improvements and correct behaviour of backwards tests
-------------------------------------------------------------------------

                 Key: LUCENE-2516
                 URL: https://issues.apache.org/jira/browse/LUCENE-2516
             Project: Lucene - Java
          Issue Type: Test
    Affects Versions: 3.1
            Reporter: Uwe Schindler
            Assignee: Uwe Schindler
             Fix For: 3.1


Backwards tests are used since 2.9 to assert, that the new Lucene version supports drop-in-replacement over the previous version. For this all tests from the previous version are compiled against the old version but then run against the new JAR file.

At the beginning the test suite was checking out another branch and doing this, but this was replaced in 3.1 by directly embedding the previous source tree and the previous tests into the backwards/ subdirectory of the SVN source. The whole idea has several problems:

- Tests not only check *public* APIs, they also check internals and sometimes even fields or package-private methods. This is allowed to change in later versions, so we must be able to change the tests, to support this behaviour. This can be done by modifying the backwards tests to pass, but still use the public API unchanged. Sometimes we simply comment out tests, that test internals and not public APIs. For those tests, I would like to propose a Java Annotation for trunk tests like @LuceneInternalTest - so we can tell the tests runner for backwards (when this test is moved as backwards layer, e.g in 4.1, that it runs all tests *but* not this marked one. This can be done easily with Junit3/4 in LuceneTestCase(J4). This is not part of this issue, but a good idea.
- Sometimes we break backwards compatibility. Currently we do our best to change the tests to reflect this, but this is unneeded and stupi, as it brings two problems. The backwards tests should be compiled against the old version of Lucene. If we change this old Version in the backwards folder, its suddenly becomes nonsense. At least the JAR artifacts of the previous version should stay *unchanged* in all cases! If we break backwards, the correct way to do this, is to simply disable coresponding tests! There is no need to make them work again, as we broke backwards, wy test plugin? The trunk tests already check the functionality, backwards tests only check API. If we fix the break in backwards, we do the contra of what they are for.

So I propose the following and have implemented in a patch for 3.x branch:

- Only include the *tests* and nothing else into the backwards branch, no source files of previous Lucene Core.
- Add the latest released JAR artifact of lucene-core.jar into backwards/lib, optimally with checksum (md5/sh1). This enforces that it is not changed and exactly do what they are for: To compile the previous tests against. This is the only reason for this JAR file.
- If we break backwards, simply *disable* the tests by commenting out, ideally with a short note and the JIRA issue that shows the break.
- If we change inner behaviour of classes, that are not public, dont fix, disable tests. Its simple: backwards tests are only for API compatibility testsing of public APIs. If a test uses internals it should not be run. For that we should use a new annotation in trunk (see above).

This has several good things:

- we can package backwards tests in src ZIP. Its not a full distrib, only the core tests and the JAR file. This enables people that doenloaded the src ZIP files to also run backwrads tests
- Your SVN checkout is not so big and backwards tests run faster!

There are some problems, with one example in the attached patch:

- If we have mock classes in the tests (e.g. MockRAMDirectory) that extend Lucene classes and have access to their internal APIs, a change in these APIs will make them fail to work unchanged. The above example (MockRAMDir) is used in lots of tests and uses a internal RAMDir field that changed type in 3.1. But we cannot disable all tests using this dir (no tests will survive). As we cannot change the previous versions JAR to reflect this, you have to use some trick in this interbal test class. In this case I removed static linking of this field and replaced by reflection. This enables compilation against old JAR, but supports running in new version. This is really a special case, but works good here.

Any comments?

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (LUCENE-2516) More clarification, improvements and correct behaviour of backwards tests

Nick Burch (Jira)

     [ https://issues.apache.org/jira/browse/LUCENE-2516?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Uwe Schindler updated LUCENE-2516:
----------------------------------

    Attachment: LUCENE-2516.patch

Here is the patch.

before apply, remove all subdirs in backwards/src except backwards/src/test.

With this patch test-backwards runs without problems. Most work were the changes that Shai did in core classes (against the above proopsal - the core classes must not change!!!). I simply disabled the tests (it was the MergePolicy stuff, LUCENE-2320). Interestingly at lots of places, setting the merge policy in IW had no effect on the test at all. They also ran with the default policy :-)

In my opinion, this change (LUCENE-2320) is to heavy for a 3.1 release, we should revert this change and only support in 4.0 - any comments? We can reenable all those tests then.

This patch also implements the MockRAMDirectory hack with reflection to enable compilation against old JAR without disableing almost all tests.

> More clarification, improvements and correct behaviour of backwards tests
> -------------------------------------------------------------------------
>
>                 Key: LUCENE-2516
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2516
>             Project: Lucene - Java
>          Issue Type: Test
>    Affects Versions: 3.1
>            Reporter: Uwe Schindler
>            Assignee: Uwe Schindler
>             Fix For: 3.1
>
>         Attachments: LUCENE-2516.patch
>
>
> Backwards tests are used since 2.9 to assert, that the new Lucene version supports drop-in-replacement over the previous version. For this all tests from the previous version are compiled against the old version but then run against the new JAR file.
> At the beginning the test suite was checking out another branch and doing this, but this was replaced in 3.1 by directly embedding the previous source tree and the previous tests into the backwards/ subdirectory of the SVN source. The whole idea has several problems:
> - Tests not only check *public* APIs, they also check internals and sometimes even fields or package-private methods. This is allowed to change in later versions, so we must be able to change the tests, to support this behaviour. This can be done by modifying the backwards tests to pass, but still use the public API unchanged. Sometimes we simply comment out tests, that test internals and not public APIs. For those tests, I would like to propose a Java Annotation for trunk tests like @LuceneInternalTest - so we can tell the tests runner for backwards (when this test is moved as backwards layer, e.g in 4.1, that it runs all tests *but* not this marked one. This can be done easily with Junit3/4 in LuceneTestCase(J4). This is not part of this issue, but a good idea.
> - Sometimes we break backwards compatibility. Currently we do our best to change the tests to reflect this, but this is unneeded and stupi, as it brings two problems. The backwards tests should be compiled against the old version of Lucene. If we change this old Version in the backwards folder, its suddenly becomes nonsense. At least the JAR artifacts of the previous version should stay *unchanged* in all cases! If we break backwards, the correct way to do this, is to simply disable coresponding tests! There is no need to make them work again, as we broke backwards, wy test plugin? The trunk tests already check the functionality, backwards tests only check API. If we fix the break in backwards, we do the contra of what they are for.
> So I propose the following and have implemented in a patch for 3.x branch:
> - Only include the *tests* and nothing else into the backwards branch, no source files of previous Lucene Core.
> - Add the latest released JAR artifact of lucene-core.jar into backwards/lib, optimally with checksum (md5/sh1). This enforces that it is not changed and exactly do what they are for: To compile the previous tests against. This is the only reason for this JAR file.
> - If we break backwards, simply *disable* the tests by commenting out, ideally with a short note and the JIRA issue that shows the break.
> - If we change inner behaviour of classes, that are not public, dont fix, disable tests. Its simple: backwards tests are only for API compatibility testsing of public APIs. If a test uses internals it should not be run. For that we should use a new annotation in trunk (see above).
> This has several good things:
> - we can package backwards tests in src ZIP. Its not a full distrib, only the core tests and the JAR file. This enables people that doenloaded the src ZIP files to also run backwrads tests
> - Your SVN checkout is not so big and backwards tests run faster!
> There are some problems, with one example in the attached patch:
> - If we have mock classes in the tests (e.g. MockRAMDirectory) that extend Lucene classes and have access to their internal APIs, a change in these APIs will make them fail to work unchanged. The above example (MockRAMDir) is used in lots of tests and uses a internal RAMDir field that changed type in 3.1. But we cannot disable all tests using this dir (no tests will survive). As we cannot change the previous versions JAR to reflect this, you have to use some trick in this interbal test class. In this case I removed static linking of this field and replaced by reflection. This enables compilation against old JAR, but supports running in new version. This is really a special case, but works good here.
> Any comments?

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Issue Comment Edited: (LUCENE-2516) More clarification, improvements and correct behaviour of backwards tests

Nick Burch (Jira)
In reply to this post by Nick Burch (Jira)

    [ https://issues.apache.org/jira/browse/LUCENE-2516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12882973#action_12882973 ]

Uwe Schindler edited comment on LUCENE-2516 at 6/27/10 2:35 PM:
----------------------------------------------------------------

Here is the patch.

before apply, remove all subdirs in backwards/src except backwards/src/test. Also add a subfolder backwards/lib and add the lucene-core-3.0.2.jar from the previous release.

With this patch test-backwards runs without problems. Most work were the changes that Shai did in core classes (against the above proopsal - the core classes must not change!!!). I simply disabled the tests (it was the MergePolicy stuff, LUCENE-2320). Interestingly at lots of places, setting the merge policy in IW had no effect on the test at all. They also ran with the default policy :-)

In my opinion, this change (LUCENE-2320) is to heavy for a 3.1 release, we should revert this change and only support in 4.0 - any comments? We can reenable all those tests then.

This patch also implements the MockRAMDirectory hack with reflection to enable compilation against old JAR without disableing almost all tests.

      was (Author: thetaphi):
    Here is the patch.

before apply, remove all subdirs in backwards/src except backwards/src/test.

With this patch test-backwards runs without problems. Most work were the changes that Shai did in core classes (against the above proopsal - the core classes must not change!!!). I simply disabled the tests (it was the MergePolicy stuff, LUCENE-2320). Interestingly at lots of places, setting the merge policy in IW had no effect on the test at all. They also ran with the default policy :-)

In my opinion, this change (LUCENE-2320) is to heavy for a 3.1 release, we should revert this change and only support in 4.0 - any comments? We can reenable all those tests then.

This patch also implements the MockRAMDirectory hack with reflection to enable compilation against old JAR without disableing almost all tests.
 

> More clarification, improvements and correct behaviour of backwards tests
> -------------------------------------------------------------------------
>
>                 Key: LUCENE-2516
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2516
>             Project: Lucene - Java
>          Issue Type: Test
>    Affects Versions: 3.1
>            Reporter: Uwe Schindler
>            Assignee: Uwe Schindler
>             Fix For: 3.1
>
>         Attachments: LUCENE-2516.patch
>
>
> Backwards tests are used since 2.9 to assert, that the new Lucene version supports drop-in-replacement over the previous version. For this all tests from the previous version are compiled against the old version but then run against the new JAR file.
> At the beginning the test suite was checking out another branch and doing this, but this was replaced in 3.1 by directly embedding the previous source tree and the previous tests into the backwards/ subdirectory of the SVN source. The whole idea has several problems:
> - Tests not only check *public* APIs, they also check internals and sometimes even fields or package-private methods. This is allowed to change in later versions, so we must be able to change the tests, to support this behaviour. This can be done by modifying the backwards tests to pass, but still use the public API unchanged. Sometimes we simply comment out tests, that test internals and not public APIs. For those tests, I would like to propose a Java Annotation for trunk tests like @LuceneInternalTest - so we can tell the tests runner for backwards (when this test is moved as backwards layer, e.g in 4.1, that it runs all tests *but* not this marked one. This can be done easily with Junit3/4 in LuceneTestCase(J4). This is not part of this issue, but a good idea.
> - Sometimes we break backwards compatibility. Currently we do our best to change the tests to reflect this, but this is unneeded and stupi, as it brings two problems. The backwards tests should be compiled against the old version of Lucene. If we change this old Version in the backwards folder, its suddenly becomes nonsense. At least the JAR artifacts of the previous version should stay *unchanged* in all cases! If we break backwards, the correct way to do this, is to simply disable coresponding tests! There is no need to make them work again, as we broke backwards, wy test plugin? The trunk tests already check the functionality, backwards tests only check API. If we fix the break in backwards, we do the contra of what they are for.
> So I propose the following and have implemented in a patch for 3.x branch:
> - Only include the *tests* and nothing else into the backwards branch, no source files of previous Lucene Core.
> - Add the latest released JAR artifact of lucene-core.jar into backwards/lib, optimally with checksum (md5/sh1). This enforces that it is not changed and exactly do what they are for: To compile the previous tests against. This is the only reason for this JAR file.
> - If we break backwards, simply *disable* the tests by commenting out, ideally with a short note and the JIRA issue that shows the break.
> - If we change inner behaviour of classes, that are not public, dont fix, disable tests. Its simple: backwards tests are only for API compatibility testsing of public APIs. If a test uses internals it should not be run. For that we should use a new annotation in trunk (see above).
> This has several good things:
> - we can package backwards tests in src ZIP. Its not a full distrib, only the core tests and the JAR file. This enables people that doenloaded the src ZIP files to also run backwrads tests
> - Your SVN checkout is not so big and backwards tests run faster!
> There are some problems, with one example in the attached patch:
> - If we have mock classes in the tests (e.g. MockRAMDirectory) that extend Lucene classes and have access to their internal APIs, a change in these APIs will make them fail to work unchanged. The above example (MockRAMDir) is used in lots of tests and uses a internal RAMDir field that changed type in 3.1. But we cannot disable all tests using this dir (no tests will survive). As we cannot change the previous versions JAR to reflect this, you have to use some trick in this interbal test class. In this case I removed static linking of this field and replaced by reflection. This enables compilation against old JAR, but supports running in new version. This is really a special case, but works good here.
> Any comments?

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (LUCENE-2516) More clarification, improvements and correct behaviour of backwards tests

Nick Burch (Jira)
In reply to this post by Nick Burch (Jira)

    [ https://issues.apache.org/jira/browse/LUCENE-2516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12882978#action_12882978 ]

Robert Muir commented on LUCENE-2516:
-------------------------------------

I like this idea, I think it will simplify backwards tests.

also, now that we have 4.0 (trunk) and 3.x, maybe we should review all the breaks still in 3.x and consider if some should be reverted to only be in trunk... then 3.1 would be more stable and keep more backwards compatibility, but trunk gets to still keep moving forward.

such 'breaks' could then be listed in the migration guide for trunk, as they are really just changes that could not be done easily in a backwards-compatible way.


> More clarification, improvements and correct behaviour of backwards tests
> -------------------------------------------------------------------------
>
>                 Key: LUCENE-2516
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2516
>             Project: Lucene - Java
>          Issue Type: Test
>    Affects Versions: 3.1
>            Reporter: Uwe Schindler
>            Assignee: Uwe Schindler
>             Fix For: 3.1
>
>         Attachments: LUCENE-2516.patch
>
>
> Backwards tests are used since 2.9 to assert, that the new Lucene version supports drop-in-replacement over the previous version. For this all tests from the previous version are compiled against the old version but then run against the new JAR file.
> At the beginning the test suite was checking out another branch and doing this, but this was replaced in 3.1 by directly embedding the previous source tree and the previous tests into the backwards/ subdirectory of the SVN source. The whole idea has several problems:
> - Tests not only check *public* APIs, they also check internals and sometimes even fields or package-private methods. This is allowed to change in later versions, so we must be able to change the tests, to support this behaviour. This can be done by modifying the backwards tests to pass, but still use the public API unchanged. Sometimes we simply comment out tests, that test internals and not public APIs. For those tests, I would like to propose a Java Annotation for trunk tests like @LuceneInternalTest - so we can tell the tests runner for backwards (when this test is moved as backwards layer, e.g in 4.1, that it runs all tests *but* not this marked one. This can be done easily with Junit3/4 in LuceneTestCase(J4). This is not part of this issue, but a good idea.
> - Sometimes we break backwards compatibility. Currently we do our best to change the tests to reflect this, but this is unneeded and stupi, as it brings two problems. The backwards tests should be compiled against the old version of Lucene. If we change this old Version in the backwards folder, its suddenly becomes nonsense. At least the JAR artifacts of the previous version should stay *unchanged* in all cases! If we break backwards, the correct way to do this, is to simply disable coresponding tests! There is no need to make them work again, as we broke backwards, wy test plugin? The trunk tests already check the functionality, backwards tests only check API. If we fix the break in backwards, we do the contra of what they are for.
> So I propose the following and have implemented in a patch for 3.x branch:
> - Only include the *tests* and nothing else into the backwards branch, no source files of previous Lucene Core.
> - Add the latest released JAR artifact of lucene-core.jar into backwards/lib, optimally with checksum (md5/sh1). This enforces that it is not changed and exactly do what they are for: To compile the previous tests against. This is the only reason for this JAR file.
> - If we break backwards, simply *disable* the tests by commenting out, ideally with a short note and the JIRA issue that shows the break.
> - If we change inner behaviour of classes, that are not public, dont fix, disable tests. Its simple: backwards tests are only for API compatibility testsing of public APIs. If a test uses internals it should not be run. For that we should use a new annotation in trunk (see above).
> This has several good things:
> - we can package backwards tests in src ZIP. Its not a full distrib, only the core tests and the JAR file. This enables people that doenloaded the src ZIP files to also run backwrads tests
> - Your SVN checkout is not so big and backwards tests run faster!
> There are some problems, with one example in the attached patch:
> - If we have mock classes in the tests (e.g. MockRAMDirectory) that extend Lucene classes and have access to their internal APIs, a change in these APIs will make them fail to work unchanged. The above example (MockRAMDir) is used in lots of tests and uses a internal RAMDir field that changed type in 3.1. But we cannot disable all tests using this dir (no tests will survive). As we cannot change the previous versions JAR to reflect this, you have to use some trick in this interbal test class. In this case I removed static linking of this field and replaced by reflection. This enables compilation against old JAR, but supports running in new version. This is really a special case, but works good here.
> Any comments?

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (LUCENE-2516) More clarification, improvements and correct behaviour of backwards tests

Nick Burch (Jira)
In reply to this post by Nick Burch (Jira)

    [ https://issues.apache.org/jira/browse/LUCENE-2516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12883020#action_12883020 ]

Shai Erera commented on LUCENE-2516:
------------------------------------

I don't think that LUCENE-2320 was such a big break. It's changes resulted in many tests changes, but most users are not affected by it at all. And it allows us to consolidate MP setting in IWC ...

I agree it's annoying to have to fix the backwards/src folder, but I'm not sure we can avoid it. Consider a change to a package private class like SegmentMerger, and a test which references that API. The test may be testing other things and use piblic APIs as well - we cannot just remove it ...

> More clarification, improvements and correct behaviour of backwards tests
> -------------------------------------------------------------------------
>
>                 Key: LUCENE-2516
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2516
>             Project: Lucene - Java
>          Issue Type: Test
>    Affects Versions: 3.1
>            Reporter: Uwe Schindler
>            Assignee: Uwe Schindler
>             Fix For: 3.1
>
>         Attachments: LUCENE-2516.patch
>
>
> Backwards tests are used since 2.9 to assert, that the new Lucene version supports drop-in-replacement over the previous version. For this all tests from the previous version are compiled against the old version but then run against the new JAR file.
> At the beginning the test suite was checking out another branch and doing this, but this was replaced in 3.1 by directly embedding the previous source tree and the previous tests into the backwards/ subdirectory of the SVN source. The whole idea has several problems:
> - Tests not only check *public* APIs, they also check internals and sometimes even fields or package-private methods. This is allowed to change in later versions, so we must be able to change the tests, to support this behaviour. This can be done by modifying the backwards tests to pass, but still use the public API unchanged. Sometimes we simply comment out tests, that test internals and not public APIs. For those tests, I would like to propose a Java Annotation for trunk tests like @LuceneInternalTest - so we can tell the tests runner for backwards (when this test is moved as backwards layer, e.g in 4.1, that it runs all tests *but* not this marked one. This can be done easily with Junit3/4 in LuceneTestCase(J4). This is not part of this issue, but a good idea.
> - Sometimes we break backwards compatibility. Currently we do our best to change the tests to reflect this, but this is unneeded and stupi, as it brings two problems. The backwards tests should be compiled against the old version of Lucene. If we change this old Version in the backwards folder, its suddenly becomes nonsense. At least the JAR artifacts of the previous version should stay *unchanged* in all cases! If we break backwards, the correct way to do this, is to simply disable coresponding tests! There is no need to make them work again, as we broke backwards, wy test plugin? The trunk tests already check the functionality, backwards tests only check API. If we fix the break in backwards, we do the contra of what they are for.
> So I propose the following and have implemented in a patch for 3.x branch:
> - Only include the *tests* and nothing else into the backwards branch, no source files of previous Lucene Core.
> - Add the latest released JAR artifact of lucene-core.jar into backwards/lib, optimally with checksum (md5/sh1). This enforces that it is not changed and exactly do what they are for: To compile the previous tests against. This is the only reason for this JAR file.
> - If we break backwards, simply *disable* the tests by commenting out, ideally with a short note and the JIRA issue that shows the break.
> - If we change inner behaviour of classes, that are not public, dont fix, disable tests. Its simple: backwards tests are only for API compatibility testsing of public APIs. If a test uses internals it should not be run. For that we should use a new annotation in trunk (see above).
> This has several good things:
> - we can package backwards tests in src ZIP. Its not a full distrib, only the core tests and the JAR file. This enables people that doenloaded the src ZIP files to also run backwrads tests
> - Your SVN checkout is not so big and backwards tests run faster!
> There are some problems, with one example in the attached patch:
> - If we have mock classes in the tests (e.g. MockRAMDirectory) that extend Lucene classes and have access to their internal APIs, a change in these APIs will make them fail to work unchanged. The above example (MockRAMDir) is used in lots of tests and uses a internal RAMDir field that changed type in 3.1. But we cannot disable all tests using this dir (no tests will survive). As we cannot change the previous versions JAR to reflect this, you have to use some trick in this interbal test class. In this case I removed static linking of this field and replaced by reflection. This enables compilation against old JAR, but supports running in new version. This is really a special case, but works good here.
> Any comments?

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (LUCENE-2516) More clarification, improvements and correct behaviour of backwards tests

Nick Burch (Jira)
In reply to this post by Nick Burch (Jira)

    [ https://issues.apache.org/jira/browse/LUCENE-2516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12883071#action_12883071 ]

Uwe Schindler commented on LUCENE-2516:
---------------------------------------

bq. I agree it's annoying to have to fix the backwards/src folder, but I'm not sure we can avoid it. Consider a change to a package private class like SegmentMerger, and a test which references that API. The test may be testing other things and use piblic APIs as well - we cannot just remove it ...

This is exactly what I listed in the last section of my proposal. There are different ways to solve it:

- If the test is also testing global APIs, maybe its enough to disable the "private" part - like i did: Most IndexWriter tests did not rely on a specific MergePolicy, so I changed to use the default. If the test itsself relys on the private API to succeed, then its better to disable it completely. There is always another test, accessing the public API - and I just repeat: we are not really testing functionality in the backwards tests, we test API compatibility. In most cases, access to private fields is only done in asserts, that can be easily disabled.
- If the private API is used in many tests (like my example with the MockRAMDir, you can always add a hack - but mark this hack as such and explain in the comment why. Only this way, we can be sure that one can understand why a test was modified / disabled.

In the future we should really write tests that do separate testing for public APIs and internal behaviour (e.g. by asserting package-protected fields and so on). By adding a Java Annotation to those tests (see above) we can automatically disable those tests in backwards (the test runner does this for us). I will open another issue, that proposes that for trunk tests, where we have no backwards at the moment, so when 4.1 development gehts started we can simply enable the backwards tests and all "internals teststing" is automatically disabled.

> More clarification, improvements and correct behaviour of backwards tests
> -------------------------------------------------------------------------
>
>                 Key: LUCENE-2516
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2516
>             Project: Lucene - Java
>          Issue Type: Test
>    Affects Versions: 3.1
>            Reporter: Uwe Schindler
>            Assignee: Uwe Schindler
>             Fix For: 3.1
>
>         Attachments: LUCENE-2516.patch
>
>
> Backwards tests are used since 2.9 to assert, that the new Lucene version supports drop-in-replacement over the previous version. For this all tests from the previous version are compiled against the old version but then run against the new JAR file.
> At the beginning the test suite was checking out another branch and doing this, but this was replaced in 3.1 by directly embedding the previous source tree and the previous tests into the backwards/ subdirectory of the SVN source. The whole idea has several problems:
> - Tests not only check *public* APIs, they also check internals and sometimes even fields or package-private methods. This is allowed to change in later versions, so we must be able to change the tests, to support this behaviour. This can be done by modifying the backwards tests to pass, but still use the public API unchanged. Sometimes we simply comment out tests, that test internals and not public APIs. For those tests, I would like to propose a Java Annotation for trunk tests like @LuceneInternalTest - so we can tell the tests runner for backwards (when this test is moved as backwards layer, e.g in 4.1, that it runs all tests *but* not this marked one. This can be done easily with Junit3/4 in LuceneTestCase(J4). This is not part of this issue, but a good idea.
> - Sometimes we break backwards compatibility. Currently we do our best to change the tests to reflect this, but this is unneeded and stupi, as it brings two problems. The backwards tests should be compiled against the old version of Lucene. If we change this old Version in the backwards folder, its suddenly becomes nonsense. At least the JAR artifacts of the previous version should stay *unchanged* in all cases! If we break backwards, the correct way to do this, is to simply disable coresponding tests! There is no need to make them work again, as we broke backwards, wy test plugin? The trunk tests already check the functionality, backwards tests only check API. If we fix the break in backwards, we do the contra of what they are for.
> So I propose the following and have implemented in a patch for 3.x branch:
> - Only include the *tests* and nothing else into the backwards branch, no source files of previous Lucene Core.
> - Add the latest released JAR artifact of lucene-core.jar into backwards/lib, optimally with checksum (md5/sh1). This enforces that it is not changed and exactly do what they are for: To compile the previous tests against. This is the only reason for this JAR file.
> - If we break backwards, simply *disable* the tests by commenting out, ideally with a short note and the JIRA issue that shows the break.
> - If we change inner behaviour of classes, that are not public, dont fix, disable tests. Its simple: backwards tests are only for API compatibility testsing of public APIs. If a test uses internals it should not be run. For that we should use a new annotation in trunk (see above).
> This has several good things:
> - we can package backwards tests in src ZIP. Its not a full distrib, only the core tests and the JAR file. This enables people that doenloaded the src ZIP files to also run backwrads tests
> - Your SVN checkout is not so big and backwards tests run faster!
> There are some problems, with one example in the attached patch:
> - If we have mock classes in the tests (e.g. MockRAMDirectory) that extend Lucene classes and have access to their internal APIs, a change in these APIs will make them fail to work unchanged. The above example (MockRAMDir) is used in lots of tests and uses a internal RAMDir field that changed type in 3.1. But we cannot disable all tests using this dir (no tests will survive). As we cannot change the previous versions JAR to reflect this, you have to use some trick in this interbal test class. In this case I removed static linking of this field and replaced by reflection. This enables compilation against old JAR, but supports running in new version. This is really a special case, but works good here.
> Any comments?

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (LUCENE-2516) More clarification, improvements and correct behaviour of backwards tests

Nick Burch (Jira)
In reply to this post by Nick Burch (Jira)

    [ https://issues.apache.org/jira/browse/LUCENE-2516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12885021#action_12885021 ]

Michael McCandless commented on LUCENE-2516:
--------------------------------------------

This looks great!

It's compelling to have only a JAR (not its sources) representing the
past release.

But I don't think we should necessarily judge the
back-compat-break-cost of an issue by how many tests it affects.  EG,
I think it's fine for LUCENE-2320 to be in 3.x (few users change the
merge policy, and those that do are very much "expert" already, and
merge policy setting really does belong on IWC).  It's also good that
it's in 3.x because it gives a deprecation path forward, making app
migration easier (vs, in 4.0 only, where you have to read MIGRATE.txt
to figure out how to fix your code).

In fact, for that issue we did not have to change the tests, right?
(The old API is still there, deprecated).  It's just that Shai was
fixing tests to not use deprecated APIs...

The patch is surprisingly small!  (Not many "real" breaks).

I agree we can handle package-private situations in the ways you
propose...

Looks good!


> More clarification, improvements and correct behaviour of backwards tests
> -------------------------------------------------------------------------
>
>                 Key: LUCENE-2516
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2516
>             Project: Lucene - Java
>          Issue Type: Test
>    Affects Versions: 3.1
>            Reporter: Uwe Schindler
>            Assignee: Uwe Schindler
>             Fix For: 3.1
>
>         Attachments: LUCENE-2516.patch
>
>
> Backwards tests are used since 2.9 to assert, that the new Lucene version supports drop-in-replacement over the previous version. For this all tests from the previous version are compiled against the old version but then run against the new JAR file.
> At the beginning the test suite was checking out another branch and doing this, but this was replaced in 3.1 by directly embedding the previous source tree and the previous tests into the backwards/ subdirectory of the SVN source. The whole idea has several problems:
> - Tests not only check *public* APIs, they also check internals and sometimes even fields or package-private methods. This is allowed to change in later versions, so we must be able to change the tests, to support this behaviour. This can be done by modifying the backwards tests to pass, but still use the public API unchanged. Sometimes we simply comment out tests, that test internals and not public APIs. For those tests, I would like to propose a Java Annotation for trunk tests like @LuceneInternalTest - so we can tell the tests runner for backwards (when this test is moved as backwards layer, e.g in 4.1, that it runs all tests *but* not this marked one. This can be done easily with Junit3/4 in LuceneTestCase(J4). This is not part of this issue, but a good idea.
> - Sometimes we break backwards compatibility. Currently we do our best to change the tests to reflect this, but this is unneeded and stupi, as it brings two problems. The backwards tests should be compiled against the old version of Lucene. If we change this old Version in the backwards folder, its suddenly becomes nonsense. At least the JAR artifacts of the previous version should stay *unchanged* in all cases! If we break backwards, the correct way to do this, is to simply disable coresponding tests! There is no need to make them work again, as we broke backwards, wy test plugin? The trunk tests already check the functionality, backwards tests only check API. If we fix the break in backwards, we do the contra of what they are for.
> So I propose the following and have implemented in a patch for 3.x branch:
> - Only include the *tests* and nothing else into the backwards branch, no source files of previous Lucene Core.
> - Add the latest released JAR artifact of lucene-core.jar into backwards/lib, optimally with checksum (md5/sh1). This enforces that it is not changed and exactly do what they are for: To compile the previous tests against. This is the only reason for this JAR file.
> - If we break backwards, simply *disable* the tests by commenting out, ideally with a short note and the JIRA issue that shows the break.
> - If we change inner behaviour of classes, that are not public, dont fix, disable tests. Its simple: backwards tests are only for API compatibility testsing of public APIs. If a test uses internals it should not be run. For that we should use a new annotation in trunk (see above).
> This has several good things:
> - we can package backwards tests in src ZIP. Its not a full distrib, only the core tests and the JAR file. This enables people that doenloaded the src ZIP files to also run backwrads tests
> - Your SVN checkout is not so big and backwards tests run faster!
> There are some problems, with one example in the attached patch:
> - If we have mock classes in the tests (e.g. MockRAMDirectory) that extend Lucene classes and have access to their internal APIs, a change in these APIs will make them fail to work unchanged. The above example (MockRAMDir) is used in lots of tests and uses a internal RAMDir field that changed type in 3.1. But we cannot disable all tests using this dir (no tests will survive). As we cannot change the previous versions JAR to reflect this, you have to use some trick in this interbal test class. In this case I removed static linking of this field and replaced by reflection. This enables compilation against old JAR, but supports running in new version. This is really a special case, but works good here.
> Any comments?

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[jira] Resolved: (LUCENE-2516) More clarification, improvements and correct behaviour of backwards tests

Nick Burch (Jira)
In reply to this post by Nick Burch (Jira)

     [ https://issues.apache.org/jira/browse/LUCENE-2516?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Uwe Schindler resolved LUCENE-2516.
-----------------------------------

    Resolution: Fixed

Committed 3x revision: 960490

I will now try in a second step to do a clean checkout of 3.0 tests again to minimize changes made in backwards tests.

> More clarification, improvements and correct behaviour of backwards tests
> -------------------------------------------------------------------------
>
>                 Key: LUCENE-2516
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2516
>             Project: Lucene - Java
>          Issue Type: Test
>    Affects Versions: 3.1
>            Reporter: Uwe Schindler
>            Assignee: Uwe Schindler
>             Fix For: 3.1
>
>         Attachments: LUCENE-2516.patch
>
>
> Backwards tests are used since 2.9 to assert, that the new Lucene version supports drop-in-replacement over the previous version. For this all tests from the previous version are compiled against the old version but then run against the new JAR file.
> At the beginning the test suite was checking out another branch and doing this, but this was replaced in 3.1 by directly embedding the previous source tree and the previous tests into the backwards/ subdirectory of the SVN source. The whole idea has several problems:
> - Tests not only check *public* APIs, they also check internals and sometimes even fields or package-private methods. This is allowed to change in later versions, so we must be able to change the tests, to support this behaviour. This can be done by modifying the backwards tests to pass, but still use the public API unchanged. Sometimes we simply comment out tests, that test internals and not public APIs. For those tests, I would like to propose a Java Annotation for trunk tests like @LuceneInternalTest - so we can tell the tests runner for backwards (when this test is moved as backwards layer, e.g in 4.1, that it runs all tests *but* not this marked one. This can be done easily with Junit3/4 in LuceneTestCase(J4). This is not part of this issue, but a good idea.
> - Sometimes we break backwards compatibility. Currently we do our best to change the tests to reflect this, but this is unneeded and stupi, as it brings two problems. The backwards tests should be compiled against the old version of Lucene. If we change this old Version in the backwards folder, its suddenly becomes nonsense. At least the JAR artifacts of the previous version should stay *unchanged* in all cases! If we break backwards, the correct way to do this, is to simply disable coresponding tests! There is no need to make them work again, as we broke backwards, wy test plugin? The trunk tests already check the functionality, backwards tests only check API. If we fix the break in backwards, we do the contra of what they are for.
> So I propose the following and have implemented in a patch for 3.x branch:
> - Only include the *tests* and nothing else into the backwards branch, no source files of previous Lucene Core.
> - Add the latest released JAR artifact of lucene-core.jar into backwards/lib, optimally with checksum (md5/sh1). This enforces that it is not changed and exactly do what they are for: To compile the previous tests against. This is the only reason for this JAR file.
> - If we break backwards, simply *disable* the tests by commenting out, ideally with a short note and the JIRA issue that shows the break.
> - If we change inner behaviour of classes, that are not public, dont fix, disable tests. Its simple: backwards tests are only for API compatibility testsing of public APIs. If a test uses internals it should not be run. For that we should use a new annotation in trunk (see above).
> This has several good things:
> - we can package backwards tests in src ZIP. Its not a full distrib, only the core tests and the JAR file. This enables people that doenloaded the src ZIP files to also run backwrads tests
> - Your SVN checkout is not so big and backwards tests run faster!
> There are some problems, with one example in the attached patch:
> - If we have mock classes in the tests (e.g. MockRAMDirectory) that extend Lucene classes and have access to their internal APIs, a change in these APIs will make them fail to work unchanged. The above example (MockRAMDir) is used in lots of tests and uses a internal RAMDir field that changed type in 3.1. But we cannot disable all tests using this dir (no tests will survive). As we cannot change the previous versions JAR to reflect this, you have to use some trick in this interbal test class. In this case I removed static linking of this field and replaced by reflection. This enables compilation against old JAR, but supports running in new version. This is really a special case, but works good here.
> Any comments?

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]