You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@abdera.apache.org by Ugo Cei <ug...@gmail.com> on 2006/08/08 19:48:45 UTC

Failing tests (was Re: )

More info about the failing tests. The following test methods fail:

FOMTest#testTextFilter
FOMTest#testXPath
FOMTest#testRoundTrip

Now, hold yourself tight, but if I run each of those tests *alone* in  
Eclipse, they succeed!

This tells me there is some interaction between tests. What stands  
out is this, apparently:


     ParserOptions options = Parser.INSTANCE.getDefaultParserOptions();
     options.setTextFilter(filter);

If the object returned by getDefaultParserOptions is a singleton,  
it's possible that one of the tests sets a filter and a subsequent  
tests fails because it expects the options to be empty. Just a  
suggestion.

	Ugo


-- 
Ugo Cei
Blog: http://agylen.com/
Open Source Zone: http://oszone.org/
Evil or Not?: http://evilornot.info/
Company: http://www.sourcesense.com/



Re: Failing tests (was Re: )

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/8/06, Garrett Rooney <ro...@electricjellyfish.net> wrote:

> Ahh, recent change, I was looking at an older working copy.
>
> I'd say that a more correct change would be to return a copy of the
> options, instead of a reference to the default one, but that's just
> me.  It seems like modifying the ParserOptions you get from
> getDefaultParserOptions shouldn't change the defaults for everyone
> else.

Also, the fact that FOMParser includes a lazily initialized variable
like that is kind of disconcerting from a thread safety manner,
considering that it's likely to be accessed via Parser.INSTANCE...

-garrett

Re: Failing tests (was Re: )

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/8/06, Ugo Cei <ug...@gmail.com> wrote:
>
> On Aug 8, 2006, at 8:04 PM, Garrett Rooney wrote:
>
> > I'd say that a more correct change would be to return a copy of the
> > options, instead of a reference to the default one, but that's just
> > me.  It seems like modifying the ParserOptions you get from
> > getDefaultParserOptions shouldn't change the defaults for everyone
> > else.
>
> In this case, better leave the test as is and not apply my patch,
> since it's effect would be to hide what is effectively a bug.
>
> Besides, I forgot to remove some System.out.print statements before
> doing the diff.

For those not watching the commits list, these failing tests should be
fixed now.

-garrett

Re: Failing tests (was Re: )

Posted by Ugo Cei <ug...@gmail.com>.
On Aug 8, 2006, at 8:04 PM, Garrett Rooney wrote:

> I'd say that a more correct change would be to return a copy of the
> options, instead of a reference to the default one, but that's just
> me.  It seems like modifying the ParserOptions you get from
> getDefaultParserOptions shouldn't change the defaults for everyone
> else.

In this case, better leave the test as is and not apply my patch,  
since it's effect would be to hide what is effectively a bug.

Besides, I forgot to remove some System.out.print statements before  
doing the diff.

	Ugo


-- 
Ugo Cei
Blog: http://agylen.com/
Open Source Zone: http://oszone.org/
Evil or Not?: http://evilornot.info/
Company: http://www.sourcesense.com/



Re: Failing tests (was Re: )

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/8/06, Ugo Cei <ug...@gmail.com> wrote:
>
> On Aug 8, 2006, at 7:53 PM, Garrett Rooney wrote:
>
> > Sounds promising, but...
> >
> >  @Override
> >  public ParserOptions getDefaultParserOptions() {
> >    return new FOMParserOptions();
> >  }
> >
> > That's from FOMParser.java, and FOMParserOptions doesn't seem to have
> > anything odd in it.
>
> Huh? My copy (rev. 429578) has:
>
>    @Override
>    public ParserOptions getDefaultParserOptions() {
>      if (options == null)
>        options = new FOMParserOptions();
>      return options;
>    }
>
> And this is indeed the problem, since I've modified the tests (patch
> attached) to make sure options are reset after every test (should
> probably use tearDown, though) and the tests now succeeed.

Ahh, recent change, I was looking at an older working copy.

I'd say that a more correct change would be to return a copy of the
options, instead of a reference to the default one, but that's just
me.  It seems like modifying the ParserOptions you get from
getDefaultParserOptions shouldn't change the defaults for everyone
else.

-garrett

Re: Failing tests (was Re: )

Posted by James M Snell <ja...@gmail.com>.
Right, I made this change to make it possible to reuse the default
parser options. Calling Parser.setDefaultParserOptions(null) on each
test should effectively reset the internals.

- James

Ugo Cei wrote:
> 
> On Aug 8, 2006, at 7:53 PM, Garrett Rooney wrote:
> 
>> Sounds promising, but...
>>
>>  @Override
>>  public ParserOptions getDefaultParserOptions() {
>>    return new FOMParserOptions();
>>  }
>>
>> That's from FOMParser.java, and FOMParserOptions doesn't seem to have
>> anything odd in it.
> 
> Huh? My copy (rev. 429578) has:
> 
>   @Override
>   public ParserOptions getDefaultParserOptions() {
>     if (options == null)
>       options = new FOMParserOptions();
>     return options;
>   }
> 
> And this is indeed the problem, since I've modified the tests (patch
> attached) to make sure options are reset after every test (should
> probably use tearDown, though) and the tests now succeeed.
> 
>     Ugo
> 
> 
> ------------------------------------------------------------------------
> 
> Index: java/trunk/parser/src/test/java/org/apache/abdera/test/parser/stax/FOMTest.java
> ===================================================================
> --- java/trunk/parser/src/test/java/org/apache/abdera/test/parser/stax/FOMTest.java	(revision 429743)
> +++ java/trunk/parser/src/test/java/org/apache/abdera/test/parser/stax/FOMTest.java	(working copy)
> @@ -148,30 +148,35 @@
>      filter.add(Constants.TITLE);
>      filter.add(Constants.ID);
>      ParserOptions options = Parser.INSTANCE.getDefaultParserOptions();
> -    options.setParseFilter(filter);
> +    try {
> +        options.setParseFilter(filter);
> +        
> +        URL url = FOMTest.class.getResource("/simple.xml");
> +        InputStream in = url.openStream();
>      
> -    URL url = FOMTest.class.getResource("/simple.xml");
> -    InputStream in = url.openStream();
> -
> -    Document<Feed> doc = Parser.INSTANCE.parse(in, url.toURI(), options);
> -    Feed feed = doc.getRoot();
> +        Document<Feed> doc = Parser.INSTANCE.parse(in, url.toURI(), options);
> +        Feed feed = doc.getRoot();
> +        
> +        assertEquals(feed.getTitle(),"Example Feed");
> +        assertEquals(feed.getTitleType(), Text.Type.TEXT);
> +        assertNull(feed.getAlternateLink());
> +        assertNull(feed.getUpdated());
> +        assertNull(feed.getAuthor());
> +        assertEquals(feed.getId().toString(), "urn:uuid:60a76c80-d399-11d9-b93C-0003939e0af6");
> +        
> +        Entry entry = feed.getEntries().get(0);
>      
> -    assertEquals(feed.getTitle(),"Example Feed");
> -    assertEquals(feed.getTitleType(), Text.Type.TEXT);
> -    assertNull(feed.getAlternateLink());
> -    assertNull(feed.getUpdated());
> -    assertNull(feed.getAuthor());
> -    assertEquals(feed.getId().toString(), "urn:uuid:60a76c80-d399-11d9-b93C-0003939e0af6");
> -    
> -    Entry entry = feed.getEntries().get(0);
> -
> -    assertEquals(entry.getTitle(),"Atom-Powered Robots Run Amok");
> -    assertEquals(entry.getTitleType(), Text.Type.TEXT);
> -    assertNull(entry.getAlternateLink());
> -    assertEquals(entry.getId().toString(),"urn:uuid:1225c695-cfb8-4ebb-aaaa-80da344efa6a");
> -    assertNull(entry.getUpdated());
> -    assertNull(entry.getSummary());
> -    assertNull(entry.getSummaryType());
> +        assertEquals(entry.getTitle(),"Atom-Powered Robots Run Amok");
> +        assertEquals(entry.getTitleType(), Text.Type.TEXT);
> +        assertNull(entry.getAlternateLink());
> +        assertEquals(entry.getId().toString(),"urn:uuid:1225c695-cfb8-4ebb-aaaa-80da344efa6a");
> +        assertNull(entry.getUpdated());
> +        assertNull(entry.getSummary());
> +        assertNull(entry.getSummaryType());
> +    }
> +    finally {
> +        options.setParseFilter(null);
> +    }
>    }
>    
>    public void testBlackListParseFilter() throws Exception {
> @@ -179,31 +184,35 @@
>      ParseFilter filter = new BlackListParseFilter();
>      filter.add(Constants.UPDATED);
>      ParserOptions options = Parser.INSTANCE.getDefaultParserOptions();
> -    options.setParseFilter(filter);
> +    try {
> +        options.setParseFilter(filter);
> +        
> +        URL url = FOMTest.class.getResource("/simple.xml");
> +        InputStream in = url.openStream();
>      
> -    URL url = FOMTest.class.getResource("/simple.xml");
> -    InputStream in = url.openStream();
> -
> -    Document<Feed> doc = Parser.INSTANCE.parse(in, url.toURI(), options);
> -    Feed feed = doc.getRoot();
> +        Document<Feed> doc = Parser.INSTANCE.parse(in, url.toURI(), options);
> +        Feed feed = doc.getRoot();
> +        
> +        assertEquals(feed.getTitle(),"Example Feed");
> +        assertEquals(feed.getTitleType(), Text.Type.TEXT);
> +        assertEquals(feed.getAlternateLink().getResolvedHref().toString(), "http://example.org/");
> +        assertNull(feed.getUpdated());
> +        assertEquals(feed.getAuthor().getName(), "John Doe");
> +        assertEquals(feed.getId().toString(), "urn:uuid:60a76c80-d399-11d9-b93C-0003939e0af6");
> +        
> +        Entry entry = feed.getEntries().get(0);
>      
> -    assertEquals(feed.getTitle(),"Example Feed");
> -    assertEquals(feed.getTitleType(), Text.Type.TEXT);
> -    assertEquals(feed.getAlternateLink().getResolvedHref().toString(), "http://example.org/");
> -    assertNull(feed.getUpdated());
> -    assertEquals(feed.getAuthor().getName(), "John Doe");
> -    assertEquals(feed.getId().toString(), "urn:uuid:60a76c80-d399-11d9-b93C-0003939e0af6");
> -    
> -    Entry entry = feed.getEntries().get(0);
> -
> -    assertEquals(entry.getTitle(),"Atom-Powered Robots Run Amok");
> -    assertEquals(entry.getTitleType(), Text.Type.TEXT);
> -    assertEquals(entry.getAlternateLink().getResolvedHref().toString(), "http://example.org/2003/12/13/atom03");
> -    assertEquals(entry.getId().toString(),"urn:uuid:1225c695-cfb8-4ebb-aaaa-80da344efa6a");
> -    assertNull(entry.getUpdated());
> -    assertEquals(entry.getSummary(), "Some text.");
> -    assertEquals(entry.getSummaryType(), Text.Type.TEXT);
> -
> +        assertEquals(entry.getTitle(),"Atom-Powered Robots Run Amok");
> +        assertEquals(entry.getTitleType(), Text.Type.TEXT);
> +        assertEquals(entry.getAlternateLink().getResolvedHref().toString(), "http://example.org/2003/12/13/atom03");
> +        assertEquals(entry.getId().toString(),"urn:uuid:1225c695-cfb8-4ebb-aaaa-80da344efa6a");
> +        assertNull(entry.getUpdated());
> +        assertEquals(entry.getSummary(), "Some text.");
> +        assertEquals(entry.getSummaryType(), Text.Type.TEXT);
> +    }
> +    finally {    
> +        options.setParseFilter(null);
> +    }
>    }
>    
>    public void testTextFilter() throws Exception {
> @@ -223,30 +232,39 @@
>      };
>      
>      ParserOptions options = Parser.INSTANCE.getDefaultParserOptions();
> -    options.setTextFilter(filter);
> +    try {
> +        options.setTextFilter(filter);
> +        
> +        URL url = FOMTest.class.getResource("/simple.xml");
> +        InputStream in = url.openStream();
> +        Document<Feed> doc = Parser.INSTANCE.parse(in, url.toURI(), options);
> +        Feed feed = doc.getRoot();
> +        
> +        assertEquals(feed.getTitle(),"Example Feed");
> +        assertEquals(feed.getTitleType(), Text.Type.TEXT);
> +        assertEquals(feed.getAlternateLink().getResolvedHref().toString(), "http://example.org/");
> +        
> +        feed.writeTo(System.out);
> +        System.out.println("Updated string: " + feed.getUpdatedString());
> +        System.out.println("Updated: " + feed.getUpdated());
> +        
> +        assertNotNull(feed.getUpdated());
> +        assertEquals(feed.getAuthor().getName(), "Jane Doe");
> +        assertEquals(feed.getId().toString(), "urn:uuid:60a76c80-d399-11d9-b93C-0003939e0af6");
> +        
> +        Entry entry = feed.getEntries().get(0);
>      
> -    URL url = FOMTest.class.getResource("/simple.xml");
> -    InputStream in = url.openStream();
> -    Document<Feed> doc = Parser.INSTANCE.parse(in, url.toURI(), options);
> -    Feed feed = doc.getRoot();
> -    
> -    assertEquals(feed.getTitle(),"Example Feed");
> -    assertEquals(feed.getTitleType(), Text.Type.TEXT);
> -    assertEquals(feed.getAlternateLink().getResolvedHref().toString(), "http://example.org/");
> -    assertNotNull(feed.getUpdated());
> -    assertEquals(feed.getAuthor().getName(), "Jane Doe");
> -    assertEquals(feed.getId().toString(), "urn:uuid:60a76c80-d399-11d9-b93C-0003939e0af6");
> -    
> -    Entry entry = feed.getEntries().get(0);
> -
> -    assertEquals(entry.getTitle(),"Atom-Powered Robots Run Crazy");
> -    assertEquals(entry.getTitleType(), Text.Type.TEXT);
> -    assertEquals(entry.getAlternateLink().getResolvedHref().toString(), "http://example.org/2003/12/13/atom03");
> -    assertEquals(entry.getId().toString(),"urn:uuid:1225c695-cfb8-4ebb-aaaa-80da344efa6a");
> -    assertNotNull(entry.getUpdated());
> -    assertEquals(entry.getSummary(), "Some text.");
> -    assertEquals(entry.getSummaryType(), Text.Type.TEXT);
> -    
> +        assertEquals(entry.getTitle(),"Atom-Powered Robots Run Crazy");
> +        assertEquals(entry.getTitleType(), Text.Type.TEXT);
> +        assertEquals(entry.getAlternateLink().getResolvedHref().toString(), "http://example.org/2003/12/13/atom03");
> +        assertEquals(entry.getId().toString(),"urn:uuid:1225c695-cfb8-4ebb-aaaa-80da344efa6a");
> +        assertNotNull(entry.getUpdated());
> +        assertEquals(entry.getSummary(), "Some text.");
> +        assertEquals(entry.getSummaryType(), Text.Type.TEXT);
> +    }
> +    finally {    
> +        options.setTextFilter(null);
> +    }
>    }
>    
>    public void testXPath() throws Exception {
> 
> 
> ------------------------------------------------------------------------
> 
> 

Re: Failing tests (was Re: )

Posted by Ugo Cei <ug...@gmail.com>.
On Aug 8, 2006, at 7:53 PM, Garrett Rooney wrote:

> Sounds promising, but...
>
>  @Override
>  public ParserOptions getDefaultParserOptions() {
>    return new FOMParserOptions();
>  }
>
> That's from FOMParser.java, and FOMParserOptions doesn't seem to have
> anything odd in it.

Huh? My copy (rev. 429578) has:

   @Override
   public ParserOptions getDefaultParserOptions() {
     if (options == null)
       options = new FOMParserOptions();
     return options;
   }

And this is indeed the problem, since I've modified the tests (patch  
attached) to make sure options are reset after every test (should  
probably use tearDown, though) and the tests now succeeed.

	Ugo


Re: Failing tests (was Re: )

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 8/8/06, Ugo Cei <ug...@gmail.com> wrote:
> More info about the failing tests. The following test methods fail:
>
> FOMTest#testTextFilter
> FOMTest#testXPath
> FOMTest#testRoundTrip
>
> Now, hold yourself tight, but if I run each of those tests *alone* in
> Eclipse, they succeed!
>
> This tells me there is some interaction between tests. What stands
> out is this, apparently:
>
>
>      ParserOptions options = Parser.INSTANCE.getDefaultParserOptions();
>      options.setTextFilter(filter);
>
> If the object returned by getDefaultParserOptions is a singleton,
> it's possible that one of the tests sets a filter and a subsequent
> tests fails because it expects the options to be empty. Just a
> suggestion.

Sounds promising, but...

  @Override
  public ParserOptions getDefaultParserOptions() {
    return new FOMParserOptions();
  }

That's from FOMParser.java, and FOMParserOptions doesn't seem to have
anything odd in it.

-garrett

Re: Failing tests (was Re: )

Posted by James M Snell <ja...@gmail.com>.
Ah! This is the change I made yesterday for persistently setting the
default parser options! doh!  Mea culpa.

- James

Ugo Cei wrote:
> More info about the failing tests. The following test methods fail:
> 
> FOMTest#testTextFilter
> FOMTest#testXPath
> FOMTest#testRoundTrip
> 
> Now, hold yourself tight, but if I run each of those tests *alone* in
> Eclipse, they succeed!
> 
> This tells me there is some interaction between tests. What stands out
> is this, apparently:
> 
> 
>     ParserOptions options = Parser.INSTANCE.getDefaultParserOptions();
>     options.setTextFilter(filter);
> 
> If the object returned by getDefaultParserOptions is a singleton, it's
> possible that one of the tests sets a filter and a subsequent tests
> fails because it expects the options to be empty. Just a suggestion.
> 
>     Ugo
> 
> 
> --Ugo Cei
> Blog: http://agylen.com/
> Open Source Zone: http://oszone.org/
> Evil or Not?: http://evilornot.info/
> Company: http://www.sourcesense.com/
> 
> 
>