Closed Bug 1365882 Opened 8 years ago Closed 8 years ago

Massive Mozmill failure on 2017-05-18 - about 100 failing tests - caused by removal of old download manager code in bug 1364050

Categories

(Thunderbird :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 55.0

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

References

Details

(Whiteboard: [Thunderbird-testfailure: Z all])

Attachments

(2 files)

About 100 failing tests, quoting 20 here: TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-menus.js | test-attachment-menus.js::test_regular_attachment [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-menus.js | test-attachment-menus.js::test_detached_attachment [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-menus.js | test-attachment-menus.js::test_multiple_attachments [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-menus.js | test-attachment-menus.js::test_multiple_attachments_one_detached [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-menus.js | test-attachment-menus.js::test_multiple_attachments_one_detached_with_missing_file [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-menus.js | test-attachment-menus.js::test_multiple_attachments_one_deleted [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-menus.js | test-attachment-menus.js::test_multiple_attachments_all_detached [log…] TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-forward-headers.js | test-forward-headers.js::test_forward_as_attachments [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-forward-rfc822-attach.js | test-forward-rfc822-attach.js::test_forwarding_long_html_line_as_attachment [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-forward-rfc822-attach.js | test-forward-rfc822-attach.js::test_forwarding_feed_message_as_attachment [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-forward-utf8.js | test-forward-utf8.js::test_utf8_forwarding_from_via_folder [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-forwarded-content.js | test-forwarded-content.js::test_forwarded_subj [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-forwarded-eml-actions.js | test-forwarded-eml-actions.js::test_reply_to_attached_eml [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-forwarded-eml-actions.js | test-forwarded-eml-actions.js::test_forward_attached_eml [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-image-display.js | test-image-display.js::test_cid_image_view [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-multipart-related.js | test-multipart-related.js::test_basic_multipart_related [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-newmsg-compose-identity.js | test-newmsg-compose-identity.js::test_display_of_identities [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-reply-addresses.js | test-reply-addresses.js::testReplyToMungedReplyToList [log…] TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-reply-addresses.js | test-reply-addresses.js::testToCcReply [log…] M-C last good: 2c783a7b6d05b4b2b417bc5f21b7e40cbf M-C first bad: baf05f61bc14fdf45511bc1165ce76daa0 https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2c783a7b6d05b4b2b417bc5f21b7e40cbf&tochange=baf05f61bc14fdf45511bc1165ce76daa0 Here some of the errors in the log: JavaScript error: chrome://messenger/content/messageDisplay.js, line 55: TypeError: gSummaryFrameManager is undefined INFO - SUMMARY-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-menus.js | test-attachment-menus.js::test_regular_attachment INFO - EXCEPTION: "context-openAllAttachments" should be enabled INFO - at: test-attachment-menus.js line 218 INFO - assert_enabled test-attachment-menus.js:218 11 INFO - check_menu_states_all test-attachment-menus.js:350 5 INFO - help_test_attachment_menus test-attachment-menus.js:375 3 INFO - SUMMARY-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-display\test-archive-messages.js | test-archive-messages.js::test_batch_archiver INFO - EXCEPTION: Timed out waiting for message display completion. INFO - at: utils.js line 447 INFO - TimeoutError utils.js:447 13 INFO - waitFor utils.js:485 11 INFO - wait_for_message_display_completion test-folder-display-helpers.js:1649 3 INFO - toggle_thread_row test-folder-display-helpers.js:1012 3 INFO - test_batch_archiver test-archive-messages.js:58 3 I'd say that with all these failures, Daily will be completely busted.
Whiteboard: [Thunderbird-testfailure: Z all]
Hmm, I ran mozmake SOLO_TEST=composition/test-forward-rfc822-attach.js mozmill-one mozmake SOLO_TEST=composition/test-forward-utf8.js mozmill-one locally and they passed. I saw these: JavaScript error: chrome://messenger/content/msgMail3PaneWindow.js, line 1291: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get] JavaScript error: resource://mozmill/modules/frame.js -> file:///c:/mozilla-source/comm-central/mail/test/mozmill/shared-modules/test-folder-display-helpers.js, line 108: Error: exception in summarizeFolder ex: [[object Object]] JavaScript error: chrome://messenger/content/messageDisplay.js, line 55: TypeError: gSummaryFrameManager is undefined JavaScript error: chrome://lightning/content/imip-bar.js, line 67: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver] JavaScript error: chrome://messenger/content/specialTabs.js, line 1324: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIObserverService.removeObserver] msgMail3PaneWindow.js:1291 is |var downloadsFile = Services.dirsvc.get("DLoads", Components.interfaces.nsIFile);| so this starts looking like bug 1364050, https://hg.mozilla.org/mozilla-central/rev/ed20cba19d7e. Hmm. Marco, can you see a connection? Sorry if it's a false track, I'm struggling to understand where all those failures come from that don't even happen locally.
Flags: needinfo?(mak77)
it's possibly related to the removal of the obsolete downloads APIs, likely also related to bug 888915. FWIW, the code you linked looks very obsolete, about migrating from downloads.rdf to downloads.sqlite... you may want to remove MigrateAttachmentDownloadStore completely...
Flags: needinfo?(mak77)
mozmake SOLO_TEST=attachment/test-attachment-menus.js mozmill-one mozmake SOLO_TEST=composition/test-forward-headers.js mozmill-one fail locally. I've double checked test-forward-rfc822-attach.js, it still doesn't fails despite some JS warnings.
I went back to M-C rev 78fa048f1254, so backing out bug 1364050, an now test-attachment-menus.js passes. Just to rule out the other four bugs that also got backed out that way, I'll now go forward to rev 43fd4e710342. Aceman, do you have any spare cycles for this?
Flags: needinfo?(acelists)
It's definitely bug 1364050, I've just spotted: https://hg.mozilla.org/mozilla-central/rev/779c698025a0 where "DLoads" got removed which we're using in msgMail3PaneWindow.js:1291 as per comment #1. Sadly bug 888915 only has a patch for SM :-( but maybe that can be made to apply for TB as well.
>> Sadly bug 888915 only has a patch for SM :-( This is an old one from Neil and not complete yet. Just unbitrotted it. SM is currently completly broken. Need to check if this is only because of the download manager removal. Is TB really using the old download manager?
I think I wrote nonsense here. None of the files in the SM patch exist in TB and furthermore I see in msgMail3PaneWindow.js: // Thunderbird has been storing old attachment download meta data in downloads.rdf // even though there was no way to show or clean up this data. Now that we are using // the new download manager in toolkit, we don't want to present this old data. // To migrate to the new download manager, remove downloads.rdf. function MigrateAttachmentDownloadStore() { var attachmentStoreVersion = Services.prefs.getIntPref("mail.attachment.store.version"); if (!attachmentStoreVersion) { var downloadsFile = Services.dirsvc.get("DLoads", Components.interfaces.nsIFile); if (downloadsFile && downloadsFile.exists()) downloadsFile.remove(false); // bump the version so we don't bother doing this again. Services.prefs.setIntPref("mail.attachment.store.version", 1); } } Somehow we still have pref("mail.attachment.store.version", 0); so this migration code is still run and fails as we can see. I wonder what would happen if I set the preference to 1 ;-) But first I have to wait for the build to finish :-( Oh, damn, that's what Marco suggested in comment #2: "you may want to remove MigrateAttachmentDownloadStore completely".
(In reply to Frank-Rainer Grahl from comment #6) > Is TB really using the old download manager? No, it's not, I'll just remove the migration and see what happens. I think I recall some discussion not packaging downloads.rdf somewhere. Now I can't find it, can you refresh my memory?
Doesn't have bug 1087233 implemented the conversion for TB?
Yes, I think so, maybe I just have to remove the migration code as was suggested. I'll try in the next 60 minutes.
https://hg.mozilla.org/comm-central/rev/a403c92783c69b934feab6328d869ae56255d2af Landed that as a bustage fix since it seemed to fix test-attachment-menus.js. Let's see.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Flags: needinfo?(acelists)
Mozmill failures gone, so we're done here.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 55.0
Summary: Massive Mozmill failure on 2017-05-18 - about 100 failing tests → Massive Mozmill failure on 2017-05-18 - about 100 failing tests - caused by removal of old download manager code in bug 1364050
(In reply to Jorg K (GMT+2) from comment #8) > I think I recall some discussion not packaging downloads.rdf somewhere. Now > I can't find it, can you refresh my memory? I got confused, I saw bug 1365585 which is about localstore.rdf.
https://hg.mozilla.org/comm-central/rev/2d11053abd07c6d0736258a09da4f6d6b0e6c3b8 I landed this follow-up since there were 41 JavaScript errors JavaScript error: resource://gre/components/mailGlue.js, line 76: NS_ERROR_FACTORY_NOT_REGISTERED: Component returned failure code: 0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED) [nsIComponentRegistrar.registerFactory] in the Mozmill run. Aceman, you can give your blessing post-landing, also for the other patch.
Attachment #8869340 - Flags: review?(acelists)
Comment on attachment 8869156 [details] [diff] [review] 1365882-mozmill-download-failure.patch Review of attachment 8869156 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, we can't do much when toolkit does not say us where "DLoads" is. Unless we hardcode it in c-c. But at least TB52 already has the downloads.rdf migrated to downloads.json.
Attachment #8869156 - Flags: review+
Comment on attachment 8869340 [details] [diff] [review] 1365882-remove-registration.patch Review of attachment 8869340 [details] [diff] [review]: ----------------------------------------------------------------- I wonder why is removing this "{b02be33b-d47c-4bd3-afd9-402a942426b0}" needed? This ID is still defined in https://dxr.mozilla.org/comm-central/source/mozilla/toolkit/components/downloads/nsIDownload.idl .
(In reply to :aceman from comment #15) > Comment on attachment 8869156 [details] [diff] [review] > 1365882-mozmill-download-failure.patch > > Review of attachment 8869156 [details] [diff] [review]: > ----------------------------------------------------------------- > > Yeah, we can't do much when toolkit does not say us where "DLoads" is. > Unless we hardcode it in c-c. > But at least TB52 already has the downloads.rdf migrated to downloads.json. Actually there is very much you can do. You can explain why you need this information in a Toolkit bug.
(In reply to :aceman from comment #16) > I wonder why is removing this "{b02be33b-d47c-4bd3-afd9-402a942426b0}" > needed? > This ID is still defined in > https://dxr.mozilla.org/comm-central/source/mozilla/toolkit/components/ > downloads/nsIDownload.idl . All I can say is that we had many JS errors like shown in comment #14 here: https://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-central-win32-debug/1495137473/comm-central_win7_ix-debug_test-mozmill-bm119-tests1-windows-build32.txt.gz So I remove the offending code: https://hg.mozilla.org/comm-central/rev/2d11053abd07c6d0736258a09da4f6d6b0e6c3b8#l1.14 which had a comment "This will eventually be removed when we use Downloads.jsm". We use Downloads.jsm in TB, so was that not the right thing to do, Marco?
Flags: needinfo?(mak77)
(In reply to Bill Gianopoulos [:WG9s] from comment #17) > > Yeah, we can't do much when toolkit does not say us where "DLoads" is. > > Unless we hardcode it in c-c. > > But at least TB52 already has the downloads.rdf migrated to downloads.json. > > Actually there is very much you can do. You can explain why you need this > information in a Toolkit bug. It's unlikely to come back, "Dloads" was just a pointer to downloads.rdf in the profile folder, that is unsupported from years. It would also be trivial to make your own, if needed, since you know where the profile folder is. (In reply to :aceman from comment #16) > I wonder why is removing this "{b02be33b-d47c-4bd3-afd9-402a942426b0}" > needed? > This ID is still defined in > https://dxr.mozilla.org/comm-central/source/mozilla/toolkit/components/ > downloads/nsIDownload.idl . Looks like we just forgot to remove that, or maybe Paolo left it in for compat purposes. Regardless, if you are using jsdownloads you should not need that.
Flags: needinfo?(mak77)
So that looks like an r+ for the removal patch then ;-)
Comment on attachment 8869340 [details] [diff] [review] 1365882-remove-registration.patch Review of attachment 8869340 [details] [diff] [review]: ----------------------------------------------------------------- Let's try it.
Attachment #8869340 - Flags: review?(acelists) → review+
Landed already in comment #14, being tried ever since ;-)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: