lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uwe Schindler" <...@thetaphi.de>
Subject RE: svn commit: r824611 - in /lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/lucene/search/spans: FieldMaskingSpanQuery.java SpanFirstQuery.java SpanNearQuery.java SpanNotQuery.java SpanOrQuery.java
Date Tue, 13 Oct 2009 08:05:21 GMT
Hi Michael,

I fixed it here, should I commit?

You problem was maybe that you thought, the backwards test code must compile
against trunk. But it's vice versa. I reverted everything and only removed
the getTerms() checks in the backwards branch. Now it works and the
backwards testing is correct.

Here the general rule applied: The backwards test code was checking against
a deprecated API, just remove it. No need to rewrite the test for that. It
is tested by the main tests. The main case of the backwards branch is to
test drop in binary compatibility.

Uwe

-----
Uwe Schindler
H.-H.-Meier-Allee 63, D-28213 Bremen
http://www.thetaphi.de
eMail: uwe@thetaphi.de


> -----Original Message-----
> From: Uwe Schindler [mailto:uwe@thetaphi.de]
> Sent: Tuesday, October 13, 2009 9:49 AM
> To: java-dev@lucene.apache.org
> Subject: RE: svn commit: r824611 - in
> /lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luc
> ene/search/spans: FieldMaskingSpanQuery.java SpanFirstQuery.java
> SpanNearQuery.java SpanNotQuery.java SpanOrQuery.java
> 
> I found the reason why it broke:
> 
> You changed in the backwards branch main code in your first commit the
> following:
> 
> +    Set<Term> terms = new HashSet<Term>();
> +    qr.extractTerms(terms);
> +    assertEquals(1, terms.size());
> 
> And the backwards branch core and test is compiled with Java 1.4 - bumm.
> So
> general rule: Never change the main code branch, only the tests in
> backwards
> and use where possible only the old *public* API. If you have to change
> the
> main code you have a backwards break. If you only test some internal
> implementations in 2.9 (not public API), remove the tests in 2.9.
> 
> Uwe
> 
> -----
> Uwe Schindler
> H.-H.-Meier-Allee 63, D-28213 Bremen
> http://www.thetaphi.de
> eMail: uwe@thetaphi.de
> 
> 
> > -----Original Message-----
> > From: Uwe Schindler [mailto:uwe@thetaphi.de]
> > Sent: Tuesday, October 13, 2009 9:43 AM
> > To: java-dev@lucene.apache.org
> > Subject: RE: svn commit: r824611 - in
> >
> /lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luc
> > ene/search/spans: FieldMaskingSpanQuery.java SpanFirstQuery.java
> > SpanNearQuery.java SpanNotQuery.java SpanOrQuery.java
> >
> > Yes, thats why we do the tests. By this it is possible to test compiled
> > Java
> > 1.4 code against new Java 1.5 lucene core with generics and test, that
> no
> > upper generics boundaries (e.g. by things like <? extends Something>)
> are
> > violated.
> >
> > But if you rewrite the tests to only use the API of lucene 3.0 and no
> > deprecated methods it should pass and it has no effect, if an additional
> > deprecated method is still available in the branch's code. If we have to
> > remove all deprecated code also from the backwards branch, we would not
> > need
> > the branch at all. So this commit is definitely not needed (and I tested
> > it,
> > it works without). In the backwards branch we should only fix the tests,
> > never the core code. If we do it, it is contra-productive.
> >
> > There were some edge cases, when we have backwards-incompatible changes
> in
> > 2.9. But this is definitely not a backwards break.
> >
> > -----
> > Uwe Schindler
> > H.-H.-Meier-Allee 63, D-28213 Bremen
> > http://www.thetaphi.de
> > eMail: uwe@thetaphi.de
> >
> > > -----Original Message-----
> > > From: Michael Busch [mailto:buschmic@gmail.com]
> > > Sent: Tuesday, October 13, 2009 9:30 AM
> > > To: java-dev@lucene.apache.org
> > > Subject: Re: svn commit: r824611 - in
> > >
> >
> /lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luc
> > > ene/search/spans: FieldMaskingSpanQuery.java SpanFirstQuery.java
> > > SpanNearQuery.java SpanNotQuery.java SpanOrQuery.java
> > >
> > > Yes that's indeed the case, see LUCENE-1529.
> > >
> > >   Michael
> > >
> > > On 10/13/09 12:25 AM, Michael Busch wrote:
> > > > It was weird - I ran all the tests before I did the previous commit
> > > > and it worked fine. Then after committing I wanted to doublecheck by
> > > > running 'ant test-tag' and got the compile errors.
> > > >
> > > > I think something is wrong with my eclipse and/or svn. But I also
> > > > switched from tortoise to command-line recently - so maybe I'm just
> > > > clumsy. Anyway, the new tag is working now, sorry for the noise.
> > > >
> > > > To your question: Wasn't there a fix recently to test-tag to test
> > > > drop-in backwards-compatibility? Which means that it compiles the
> > > > tests first against the sources of the back-compat branch, but then
> > > > runs them against the new trunk JAR? That's why this commit is
> > > > necessary I think.
> > > >
> > > >  Michael
> > > >
> > > > On 10/13/09 12:18 AM, Uwe Schindler wrote:
> > > >> I wonder why this commit is needed. It only affects the core
> classes,
> > > >> not th
> > > >> tests. To compile correct backwards tests it should not be
> important
> > > >> if the
> > > >> methods exist or not.
> > > >>
> > > >> -----
> > > >> Uwe Schindler
> > > >> H.-H.-Meier-Allee 63, D-28213 Bremen
> > > >> http://www.thetaphi.de
> > > >> eMail: uwe@thetaphi.de
> > > >>
> > > >>
> > > >>> -----Original Message-----
> > > >>> From: buschmi@apache.org [mailto:buschmi@apache.org]
> > > >>> Sent: Tuesday, October 13, 2009 9:00 AM
> > > >>> To: java-commits@lucene.apache.org
> > > >>> Subject: svn commit: r824611 - in
> > > >>>
> > >
> >
> /lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luc
> > > >>>
> > > >>> ene/search/spans: FieldMaskingSpanQuery.java SpanFirstQuery.java
> > > >>> SpanNearQuery.java SpanNotQuery.java SpanOrQuery.java
> > > >>>
> > > >>> Author: buschmi
> > > >>> Date: Tue Oct 13 06:59:40 2009
> > > >>> New Revision: 824611
> > > >>>
> > > >>> URL: http://svn.apache.org/viewvc?rev=824611&view=rev
> > > >>> Log:
> > > >>> More fixes that were accidentially left out in the previous commit
> > > >>>
> > > >>> Modified:
> > > >>>
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/FieldMaskingSpanQuery.java
> > > >>>
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanFirstQuery.java
> > > >>>
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanNearQuery.java
> > > >>>
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanNotQuery.java
> > > >>>
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanOrQuery.java
> > > >>>
> > > >>> Modified:
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/FieldMaskingSpanQuery.java
> > > >>> URL:
> > > >>>
> > >
> >
> http://svn.apache.org/viewvc/lucene/java/branches/lucene_2_9_back_compat_t
> > > >>>
> > > >>>
> > >
> >
> ests/src/java/org/apache/lucene/search/spans/FieldMaskingSpanQuery.java?re
> > > >>>
> > > >>> v=824611&r1=824610&r2=824611&view=diff
> > > >>>
> > >
> >
> ==========================================================================
> > > >>>
> > > >>> ====
> > > >>> ---
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/FieldMaskingSpanQuery.java (original)
> > > >>> +++
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/FieldMaskingSpanQuery.java Tue Oct 13 06:59:40
> 2009
> > > >>> @@ -94,11 +94,6 @@
> > > >>>       return maskedQuery.getSpans(reader);
> > > >>>     }
> > > >>>
> > > >>> -  /** @deprecated use {@link #extractTerms(Set)} instead. */
> > > >>> -  public Collection getTerms() {
> > > >>> -    return maskedQuery.getTerms();
> > > >>> -  }
> > > >>> -
> > > >>>     public void extractTerms(Set terms) {
> > > >>>       maskedQuery.extractTerms(terms);
> > > >>>     }
> > > >>>
> > > >>> Modified:
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanFirstQuery.java
> > > >>> URL:
> > > >>>
> > >
> >
> http://svn.apache.org/viewvc/lucene/java/branches/lucene_2_9_back_compat_t
> > > >>>
> > > >>>
> > >
> >
> ests/src/java/org/apache/lucene/search/spans/SpanFirstQuery.java?rev=82461
> > > >>>
> > > >>> 1&r1=824610&r2=824611&view=diff
> > > >>>
> > >
> >
> ==========================================================================
> > > >>>
> > > >>> ====
> > > >>> ---
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanFirstQuery.java (original)
> > > >>> +++
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanFirstQuery.java Tue Oct 13 06:59:40 2009
> > > >>> @@ -47,12 +47,6 @@
> > > >>>
> > > >>>     public String getField() { return match.getField(); }
> > > >>>
> > > >>> -  /** Returns a collection of all terms matched by this query.
> > > >>> -   * @deprecated use extractTerms instead
> > > >>> -   * @see #extractTerms(Set)
> > > >>> -   */
> > > >>> -  public Collection getTerms() { return match.getTerms(); }
> > > >>> -
> > > >>>     public String toString(String field) {
> > > >>>       StringBuffer buffer = new StringBuffer();
> > > >>>       buffer.append("spanFirst(");
> > > >>>
> > > >>> Modified:
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanNearQuery.java
> > > >>> URL:
> > > >>>
> > >
> >
> http://svn.apache.org/viewvc/lucene/java/branches/lucene_2_9_back_compat_t
> > > >>>
> > > >>>
> > >
> >
> ests/src/java/org/apache/lucene/search/spans/SpanNearQuery.java?rev=824611
> > > >>>
> > > >>> &r1=824610&r2=824611&view=diff
> > > >>>
> > >
> >
> ==========================================================================
> > > >>>
> > > >>> ====
> > > >>> ---
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanNearQuery.java (original)
> > > >>> +++
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanNearQuery.java Tue Oct 13 06:59:40 2009
> > > >>> @@ -80,20 +80,6 @@
> > > >>>
> > > >>>     public String getField() { return field; }
> > > >>>
> > > >>> -  /** Returns a collection of all terms matched by this query.
> > > >>> -   * @deprecated use extractTerms instead
> > > >>> -   * @see #extractTerms(Set)
> > > >>> -   */
> > > >>> -  public Collection getTerms() {
> > > >>> -    Collection terms = new ArrayList();
> > > >>> -    Iterator i = clauses.iterator();
> > > >>> -    while (i.hasNext()) {
> > > >>> -      SpanQuery clause = (SpanQuery)i.next();
> > > >>> -      terms.addAll(clause.getTerms());
> > > >>> -    }
> > > >>> -    return terms;
> > > >>> -  }
> > > >>> -
> > > >>>     public void extractTerms(Set terms) {
> > > >>>           Iterator i = clauses.iterator();
> > > >>>           while (i.hasNext()) {
> > > >>>
> > > >>> Modified:
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanNotQuery.java
> > > >>> URL:
> > > >>>
> > >
> >
> http://svn.apache.org/viewvc/lucene/java/branches/lucene_2_9_back_compat_t
> > > >>>
> > > >>>
> > >
> >
> ests/src/java/org/apache/lucene/search/spans/SpanNotQuery.java?rev=824611&
> > > >>>
> > > >>> r1=824610&r2=824611&view=diff
> > > >>>
> > >
> >
> ==========================================================================
> > > >>>
> > > >>> ====
> > > >>> ---
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanNotQuery.java (original)
> > > >>> +++
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanNotQuery.java Tue Oct 13 06:59:40 2009
> > > >>> @@ -49,12 +49,6 @@
> > > >>>
> > > >>>     public String getField() { return include.getField(); }
> > > >>>
> > > >>> -  /** Returns a collection of all terms matched by this query.
> > > >>> -   * @deprecated use extractTerms instead
> > > >>> -   * @see #extractTerms(Set)
> > > >>> -   */
> > > >>> -  public Collection getTerms() { return include.getTerms(); }
> > > >>> -
> > > >>>     public void extractTerms(Set terms) {
> > > >>> include.extractTerms(terms); }
> > > >>>
> > > >>>     public String toString(String field) {
> > > >>>
> > > >>> Modified:
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanOrQuery.java
> > > >>> URL:
> > > >>>
> > >
> >
> http://svn.apache.org/viewvc/lucene/java/branches/lucene_2_9_back_compat_t
> > > >>>
> > > >>>
> > >
> >
> ests/src/java/org/apache/lucene/search/spans/SpanOrQuery.java?rev=824611&r
> > > >>>
> > > >>> 1=824610&r2=824611&view=diff
> > > >>>
> > >
> >
> ==========================================================================
> > > >>>
> > > >>> ====
> > > >>> ---
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanOrQuery.java (original)
> > > >>> +++
> > > >>>
> > >
> >
> lucene/java/branches/lucene_2_9_back_compat_tests/src/java/org/apache/luce
> > > >>>
> > > >>> ne/search/spans/SpanOrQuery.java Tue Oct 13 06:59:40 2009
> > > >>> @@ -58,20 +58,6 @@
> > > >>>
> > > >>>     public String getField() { return field; }
> > > >>>
> > > >>> -  /** Returns a collection of all terms matched by this query.
> > > >>> -   * @deprecated use extractTerms instead
> > > >>> -   * @see #extractTerms(Set)
> > > >>> -   */
> > > >>> -  public Collection getTerms() {
> > > >>> -    Collection terms = new ArrayList();
> > > >>> -    Iterator i = clauses.iterator();
> > > >>> -    while (i.hasNext()) {
> > > >>> -      SpanQuery clause = (SpanQuery)i.next();
> > > >>> -      terms.addAll(clause.getTerms());
> > > >>> -    }
> > > >>> -    return terms;
> > > >>> -  }
> > > >>> -
> > > >>>     public void extractTerms(Set terms) {
> > > >>>       Iterator i = clauses.iterator();
> > > >>>       while (i.hasNext()) {
> > > >>>
> > > >>
> > > >>
> > > >> -------------------------------------------------------------------
> --
> > > >> To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
> > > >> For additional commands, e-mail: java-dev-help@lucene.apache.org
> > > >>
> > > >>
> > > >
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
> > > For additional commands, e-mail: java-dev-help@lucene.apache.org
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
> > For additional commands, e-mail: java-dev-help@lucene.apache.org
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: java-dev-help@lucene.apache.org


Mime
View raw message