lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Earwin Burrfoot <ear...@gmail.com>
Subject Re: API Semantics and Backwards
Date Tue, 30 Nov 2010 10:12:14 GMT
Oh, Shai already said this, so +1.

On Tue, Nov 30, 2010 at 13:11, Earwin Burrfoot <earwin@gmail.com> wrote:
> We can try writing tests that only check binary compatibility for
> public/protected members?
> And use these for back-compat testing.
>
> On Tue, Nov 30, 2010 at 12:47, Shai Erera <serera@gmail.com> wrote:
>> I realize the benefits of not storing the backwards source -- I don't care
>> too much about the size of the checkout, but more about it making
>> introducing bw breaks easier.
>>
>> And that that it happened w/ MockRAMDir only so far, doesn't mean it won't
>> bite us somewhere else too. But it's not a good enough reason to create a
>> bw/src/java either.
>>
>> It would be great if we can remove all the unrelated tests from backwards.
>> As I see it, we should have two types of tests - those that check that
>> *public* API hasn't changed, and this can be in the form of reflection or
>> simply creating classes that call/extend the public API. Also, we want to
>> have tests that assert *runtime* backwards support, such as for Tokenizers,
>> QueryParser etc. For those we can have special test cases that assert
>> exactly that.
>>
>> Like you said, the rest of the tests just increase the test running time.
>>
>> Shai
>>
>> On Tue, Nov 30, 2010 at 9:50 AM, Uwe Schindler <uwe@thetaphi.de> wrote:
>>>
>>> Hi Shai,
>>>
>>>
>>>
>>> there are few comments:
>>>
>>>
>>>
>>> You are right with „we should not check backwards” for internal or
>>> *experimental* apis, the problem is that some tests use these internal APIs
>>> to check some internals. But as the idea behind backwards tests say: we test
>>> only drop in replacements. The functionality behind the tests is just
>>> nonsense, as the functionality is already tested by the main tests. So we
>>> just run the tests two times. Because of this, we can simply disable all
>>> tests that *explicitely* check internal APIs. An example would be a test
>>> that checks the exact class names of some returned objects (e.g. in
>>> MultiTermQuery rewrites).
>>>
>>>
>>>
>>> The problem are Mock classes in the backwards tests, that check internal
>>> apis (the famous example is MockRAMDirectory) as it is used by almost every
>>> test. If we would disable all these tests, we would not have the possibility
>>> to test any of them. For the current problem (and another one with this
>>> exact same class), we have a solution, I attached it to the issue. It’s a 10
>>> lines patch, it’s a hack, but its better than living with the cruft of
>>> having a modifiable backwards branch. If you really have to change these
>>> mock classes, you can do it like in the patch – but then you know you are
>>> doing something special and you can *mark* it as such (like I did in my
>>> patch).
>>>
>>>
>>>
>>> I am against reinserting the previous version’s classes for several
>>> reasons:
>>>
>>> -          The checkout gets big
>>>
>>> -          When we release a bugfix release of the previous version,
we
>>> should be able to replace the old jar file by the bugfix one. I will sonn
>>> replace lucene-core-3.0.2.jar with 3.0.3.
>>>
>>> -          We should really don’t ever change the core test, because
it
>>> contradicts the sense of backwards tests. If we really need to fix it (like
>>> for mock classes that are used by every test), it can be done in various
>>> ways: Remove the extra check code from the mock classes (this is often
>>> easiest) or use a reflection hack (we only have two of them now – but you
>>> changed the backwards branch much more often before I reverted it when
>>> adding the previous jar file).
>>>
>>> -          For tests that simply test some internal apis and nothing
else
>>> important (for plugin compatibility): lets comment out the test. In the bw
>>> folder we did this with a special comment to leave the code intact.
>>>
>>>
>>>
>>> Uwe
>>>
>>>
>>>
>>> -----
>>>
>>> Uwe Schindler
>>>
>>> H.-H.-Meier-Allee 63, D-28213 Bremen
>>>
>>> http://www.thetaphi.de
>>>
>>> eMail: uwe@thetaphi.de
>>>
>>>
>>>
>>> From: Shai Erera [mailto:serera@gmail.com]
>>> Sent: Tuesday, November 30, 2010 7:46 AM
>>> To: dev@lucene.apache.org
>>> Subject: API Semantics and Backwards
>>>
>>>
>>>
>>> Hi
>>>
>>> I'd like to discuss the semantics of our API and how backwards tests
>>> relate to it. First, I'd like to confirm my understanding - currently it
>>> relates to 3x, but it will apply to 4x after 4.0 will be released:
>>>
>>> Public/Protected -- this API is 'public' and we should maintain
>>> back-compat, in the form of jar drop-in. That is, we cannot rename or modify
>>> it, w/o deprecating first (I leave *exceptions* deliberately outside the
>>> discussion).
>>>
>>> Package-private -- this is not public API and while users can use it if
>>> they declare their classes under the relevant package, they should not
>>> expect jar drop-in support.
>>>
>>> Public @internal -- this is public API following Java language, however
>>> not public to the users. We need to make this API public so that Lucene can
>>> access it, but it's used for internal purposes only. Users can still use it,
>>> however cannot expect jar drop-in support.
>>>
>>> Public @experimental -- this API is intended to be made 'public' one day,
>>> however we're still working on it, and even though it's checked-in or even
>>> released, it may change unexpectedly. Not sure we want to say that jar
>>> drop-in support cannot be expected, though according to the definition we
>>> are allowed to change it ... so perhaps it's like @internal, only w/ the
>>> intention to make it public one day.
>>>
>>> Both @internal and @experimental tags should be removed if they do not
>>> apply anymore.
>>>
>>> Now comes the question about backwards tests -- our tests touch all the
>>> API types above, however they are not resilient to changes in 3 out of 4 of
>>> them. In the past this wasn't a problem - the backwards layer had both
>>> src/java and src/test, tests we compiled against src/java and then executed
>>> against core.jar. This allowed changing the source code of the "non public"
>>> API and make the same changes to backwards/src/java, and tests would still
>>> run. This had a disadvantage too - it was 'easier' to break back-compat on
>>> the first API type (the *true* public API) because you could still change
>>> bw/src/java and be done w/ it.
>>>
>>> Today though bw tests are compiled against the previous release source.
>>> But if you make changes to the non public API, they break while they
>>> shouldn't. So the question is what can we do about the backwards tests so
>>> that we can still make allowed changes to the API w/o them breaking?
>>>
>>> * We can say that unit tests should not test package-private / @internal /
>>> @experimental classes, but I don't believe in it.
>>>
>>> * We can re-introduce bw/src/java and ask all committers to make careful
>>> changes to it. If we're careful, we won't introduce any *true* public API
>>> break.
>>>
>>> The second is the more realistic solution IMO, but since this was the
>>> situation in the past and changed to how it is today, I don't know if it's
>>> acceptable.
>>>
>>> Whatever we do though, we cannot have backwards tests dictate what is
>>> public API and what isn't, because bw tests are compiled following Java
>>> semantics, that have nothing to do w/ Lucene's 'public' notion.
>>>
>>> Shai
>>
>
>
>
> --
> Kirill Zakharenko/Кирилл Захаренко (earwin@gmail.com)
> Phone: +7 (495) 683-567-4
> ICQ: 104465785
>



-- 
Kirill Zakharenko/Кирилл Захаренко (earwin@gmail.com)
Phone: +7 (495) 683-567-4
ICQ: 104465785

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message