479976 - Feed Unit Tests do not follow convention established in Bug443090

Status: RESOLVED WONTFIX

(Firefox Graveyard :: RSS Discovery and Preview, defect)

Description

[John Ford [:jhford] CET/CEST Berlin Time]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-GB; rv:1.9.1b2) Gecko/20081201 Firefox/3.1b2
Build Identifier: 

As specified in Bug 443090 there is a new convention for testing output.  The tests in mozilla-central/toolkit/components/feeds/test/ do not follow this convention.

Reproducible: Always

Steps to Reproduce:
1. run make -k check in a a fully built objdir
Actual Results:  
There are a bunch of tests with output like:
PASS | xml/rss2/feed_pubDate.xml | Test was: "channel pubDate works" | feed.fields.getProperty('pubDate') == 'Sat, 07 Sep 2002 00:00:01 GMT' |

Expected Results:  
1234567890 | TEST-PASS | xml/rss2/feed_pubDate.xml | Test was: "channel pubDate works" | feed.fields.getProperty('pubDate') == 'Sat, 07 Sep 2002 00:00:01 GMT' |

I have written a patch which fixes this and am in the process of testing it.

Comment 1

[Jeff Walden [:Waldo]]

Bugs to fix tests go in the component most closely related to those tests, FYI.

Comment 2

[John Ford [:jhford] CET/CEST Berlin Time]

1235495890.009 | TEST-PASS | xml/rss2/item_guid_bogus_url.xml | Test was: "item copes with bogus guid" | feed.items.queryElementAt(0, Components.interfaces.nsIFeedEntry).link == null; |
1235495890.015 | TEST-PASS | xml/rss2/item_guid_isPermaLink.xml | Test was: "item guid works" | var link = feed.items.queryElementAt(0, Components.interfaces.nsIFeedEntry).link; link.spec == 'http://www.example.org/'; |
Here is some sample output with my patch.  I thought I had put it in a different component,  I will change that if I can.

Comment 3

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: Correct feed test output

I would like to have this respect a mozconfig ac_add_options that I have added in another patch for bug 443329 but I don't know how to get something from autoconf into a JavaScript file.  My immediate thought is to turn the javascript into a .in file.  I will look into this

Comment 4

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: Version 2

Comment 5

[John Ford [:jhford] CET/CEST Berlin Time]

Comment on attachment 366711 [details] [diff] [review]
Version 2

Did not properly correct PASS and FAIL in old version of patch

Comment 6

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: feed-test-cleanup-v3

Cleaned up output

Comment 8

[Serge Gautherie (:sgautherie)]

Comment on attachment 367145 [details] [diff] [review]
feed-test-cleanup-v3


I think we want just
TEST FAILURE | TEST NAME | ADDITIONAL INFORMATION
(everywhere) for now.

Especially, forget about the time part.

Jeff, do you confirm ?

Comment 9

[Serge Gautherie (:sgautherie)]

(In reply to comment #8)

At least, I'd like the 'TEST-*' part to be fixed here (asap),
then the timestamp addition moved to bug 443329 (later).

Comment 10

[(not currently active) Ted Mielczarek]

I'd rather see these tests ported to be real xpcshell tests, FWIW. Will make it easier to maintain in the long run, instead of the current one-off test framework it's using. (Which predates our xpcshell framework.)

Comment 11

[Serge Gautherie (:sgautherie)]

John, why WontFix?
Fixing the format then/or porting to xpcshell looks still wanted to me.