Daniel Boelzle [:dbo] Assignee
	Description • 16 years ago

The units are broken, and take quite long (longer than on 1.8 branch) to execute. Tested on comm-central Mac x86 debug build:

1. I see most of the js tests passing, but bus errors occur on every test, e.g.
*** exiting
*** PASS ***
WARNING: nsExceptionService ignoring thread destruction after shutdown: file /Volumes/data/Users/dbo/moz_cc/src/mozilla/xpcom/base/nsExceptionService.cpp, line 194

<<<<<<<
/Volumes/data/Users/dbo/moz_cc/src/mozilla/tools/test-harness/xpcshell-simple/test_all.sh: line 111: 19790 Bus error               NATIVE_TOPSRCDIR="$native_topsrcdir" TOPSRCDIR="$topsrcdir" $xpcshell -s $headfiles -f $t $tailfiles 2> $t.log 1>&2
TEST-UNEXPECTED-FAIL | ../../mozilla/_tests/xpcshell-simple/test_calendar/unit/test_recur.js | test failed, see log
../../mozilla/_tests/xpcshell-simple/test_calendar/unit/test_recur.js.log:
>>>>>>>

2. I see some strict warnings:
../../mozilla/_tests/xpcshell-simple/test_calendar/unit/test_recur.js:207: strict warning: variable item redeclares argument:
../../mozilla/_tests/xpcshell-simple/test_calendar/unit/test_recur.js:207: strict warning:     var item = makeEvent("DTSTART:20020402T114500Z\n" +

../../mozilla/_tests/xpcshell-simple/test_calendar/unit/test_recur.js:207: strict warning: ........^

3. I see test_recur.js failing:
*** TEST-UNEXPECTED-FAIL | ../../mozilla/_tests/xpcshell-simple/test_calendar/unit/test_recur.js | RRULE:FREQ=WEEKLY;COUNT=6;BYDAY=TU,WE
 == RRULE:FREQ=WEEKLY;COUNT=6;INTERVAL=1;BYDAY=TU,WE

I assume this is due to the libical update we've discussed (DTSTART not matching RRULE).

	Sebastian Schwieger
	Comment 1 • 16 years ago

Additionally to the reported error 1, I get a core dump of the test_all.sh script (with a linux-x64 build). I narrowed the cause down to the providers test.

	Martin Schröder [:mschroeder]
	Comment 4 • 16 years ago

Comment on attachment 342073 [details] [diff] [review]
[checked in] Patch (remove strict warning)

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/e7012863d522>

	Stefan Sitter [:ssitter]
	Comment 5 • 16 years ago

When trying to run unittests for sunbird win32 debug build I get:

- A popup "xpcshell.exe - The instruction at '0x...' referenced memory at '0x...'. The memory could not be read. - Click on OK to terminate the program" for each test.

- The nsExceptionService warning mentioned above for each test

- test_alarm.js:

Checking repeatOffset...OK!
WARNING: Declared InterfaceInfo not found: file e:/dev/comm-central/src/mozilla/xpcom/reflect/xptinfo/src/xptiInterfaceInfo.cpp, line 434

- test_datetime.js and test_providers.js:
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "Cc['@mozilla.org/xre/app-info;1'] is undefined" {file: "file:///e:/obj/sb/mozilla/dist/bin/components/nsExtensionManager.js" line: 2425}]' when calling method: [nsIFactory::createInstance]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: file:///e:/obj/sb/mozilla/dist/bin/components/calItemModule.js -> file:///e:/obj/sb/mozilla/dist/bin/js/calTimezoneService.js :: calTimezoneService_ensureInitialized :: line 143"  data: yes]
************************************************************

- test_datetime.js:
line 67: strict warning: assignment to undeclared variable cd_allday

	Stefan Sitter [:ssitter]
	Comment 6 • 16 years ago

The xpcshell crash seems to be the same as in Bug 457024 [@nsXPCWrappedJS::Release] [@nsCOMPtr<calITimezone>::~nsCOMPtr<calITimezone>]

	Daniel Boelzle [:dbo] Assignee
	Comment 7 • 16 years ago

libical 0.40 seems to omit INTERVAL=1

	Stefan Sitter [:ssitter]
	Comment 8 • 16 years ago

I can confirm that the crash and assertions mentioned before are now gone. All tests except test_recur.js pass.

	Daniel Boelzle [:dbo] Assignee
	Comment 9 • 16 years ago

test_recur.js will be fixed with the patch attached to this bug.

BTW: do we still need the homegrown calendar tests (http://mxr.mozilla.org/comm-central/source/calendar/test/homegrown/)? Opinions?

	Martin Schröder [:mschroeder]
	Comment 10 • 16 years ago

Comment on attachment 345474 [details] [diff] [review]
omit INTERVAL=1

I don't think removing 'INTERVAL=1' is the right thing to do. IMO you shouldn't correct an unit test if it's basically correct and valid iCalendar data. Maybe there is some lack of knowledge on my side, so please correct my if I'm wrong.

	Martin Schröder [:mschroeder]
	Comment 11 • 16 years ago

(In reply to comment #9)
> BTW: do we still need the homegrown calendar tests
> (http://mxr.mozilla.org/comm-central/source/calendar/test/homegrown/)?
> Opinions?

We don't need them anymore if we cover the tested functionality with XPCShell unit tests, otherwise we should convert the remaining homegrown calendar tests to XPCShell unit tests.

	Daniel Boelzle [:dbo] Assignee
	Comment 12 • 16 years ago

(In reply to comment #10)
> (From update of attachment 345474 [details] [diff] [review])
> I don't think removing 'INTERVAL=1' is the right thing to do. IMO you shouldn't
> correct an unit test if it's basically correct and valid iCalendar data. Maybe
> there is some lack of knowledge on my side, so please correct my if I'm wrong.

INTERVAL=1 is semantically equal to just omitting it which libical does. Aside from this I think the recurring unit tests are a bit weak, because they rely on a certain order of the RRULE params.

(In reply to comment #11)
> (In reply to comment #9)
> > BTW: do we still need the homegrown calendar tests
> > (http://mxr.mozilla.org/comm-central/source/calendar/test/homegrown/)?
> > Opinions?
> 
> We don't need them anymore if we cover the tested functionality with XPCShell
> unit tests, otherwise we should convert the remaining homegrown calendar tests
> to XPCShell unit tests.

Sure. My question is more like what tests are yet uncovered.

	Martin Schröder [:mschroeder]
	Comment 13 • 16 years ago

(In reply to comment #12)
> (In reply to comment #10)
> INTERVAL=1 is semantically equal to just omitting it which libical does. Aside
> from this I think the recurring unit tests are a bit weak, because they rely on
> a certain order of the RRULE params.

Agreed. Please file a follow-up bug for making the |test_interface()| function in |test_recur.js| more robust.

> (In reply to comment #11)
> > (In reply to comment #9)
> > We don't need them anymore if we cover the tested functionality with XPCShell
> > unit tests, otherwise we should convert the remaining homegrown calendar tests
> > to XPCShell unit tests.
> 
> Sure. My question is more like what tests are yet uncovered.

Okay, we just need a follow-up bug, which you can assign to me.

	Martin Schröder [:mschroeder]
	Comment 14 • 16 years ago

Comment on attachment 345474 [details] [diff] [review]
omit INTERVAL=1

r=mschroeder

	Daniel Boelzle [:dbo] Assignee
	Comment 15 • 16 years ago

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/f2d8cfd2bf2d>

-> FIXED

bug 462823 and bug 462824 filed

	Philipp Kewisch [:Fallen] [:📆][:🧩]
	Comment 16 • 13 years ago

These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".