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 <[email protected]> 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 <[email protected]> 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: [email protected] >> >> >> >> From: Shai Erera [mailto:[email protected]] >> Sent: Tuesday, November 30, 2010 7:46 AM >> To: [email protected] >> 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/Кирилл Захаренко ([email protected]) Phone: +7 (495) 683-567-4 ICQ: 104465785 --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
