590700 - Test failures in test-display-names.js

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Jim Porter (:squib)]

From bug 474721 comment 77:

Some of the tests were failing on Windows, so I've disabled them for now:

http://hg.mozilla.org/comm-central/rev/746fa9d763e5

Jim, can you file a follow-up bug to fix those please?

Tinderbox log:
http://tinderbox.mozilla.org/showlog.cgi?log=Thunderbird/1282734469.1282740617.17017.gz

TEST-UNEXPECTED-FAIL |
e:\buildbot\win32-comm-central-check\build\mail\test\mozmill\message-header\test-display-names.js
| test_single_identity
  EXCEPTION: got 'You <[e-mail]>' but expected 'You'
    at: test-display-names.js line 119
       Error("got 'You <sender  0
       help_test_display_name(0,"to","You") test-display-names.js 119
       test_single_identity() test-display-names.js 125
            frame.js 474
            frame.js 530
            frame.js 573
            frame.js 417
            frame.js 579
            server.js 164
            server.js 168
TEST-PASS |  test_single_identity_in_abook
TEST-UNEXPECTED-FAIL |
e:\buildbot\win32-comm-central-check\build\mail\test\mozmill\message-header\test-display-names.js
| test_single_identity_in_abook_no_pdn
  EXCEPTION: got 'You <[e-mail]>' but expected 'You'
    at: test-display-names.js line 119
       Error("got 'You <sender  0
       help_test_display_name(0,"to","You") test-display-names.js 119
       test_single_identity_in_abook_no_pdn() test-display-names.js 137
            frame.js 474
            frame.js 530
            frame.js 573
            frame.js 417
            frame.js 579
            server.js 164
            server.js 168

Comment 1

[Jim Porter (:squib)]

It looks like the Windows tests aren't properly deleting all-but-one identity, which is causing problems. I'm not sure why this is happening, though.

Comment 2

[Jim Porter (:squib)]

Attachment: Possibly fix tests

This might possibly fix the tests on Windows...

Comment 3

[Jim Porter (:squib)]

:standard8, do you think you could push this to the try server (maybe Windows-only) to see if it fixes things?

Comment 4

[Mark Banner (:standard8)]

Pushed to try, follow revision 99e54ee3e62d.

Comment 5

[Jim Porter (:squib)]

Attachment: This should work

I'm pretty confident this will work. I changed how I delete extra accounts/identities to go in reverse order so that the array indices don't shift all over the place. Probably worth another push to try first...

Comment 6

[Justin Wood (:Callek)]

new patch Pushed to try, follow rev bdf25aa28663

Comment 7

[Jim Porter (:squib)]

Hooray, it looks like this finally fixed the issues in comment 0: http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=bdf25aa28663

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 505500 [details] [diff] [review]
This should work

Per discussion on irc, I think this test should be cleaning up after itself if that is reasonably possible. Therefore cancelling review for now whilst this is investigated.

Comment 9

[Jim Porter (:squib)]

Attachment: Restore deleted accounts/identities on teardown

Ok, this version keeps track of the deleted accounts/identities and recreates them once the test is done.

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 507291 [details] [diff] [review]
Restore deleted accounts/identities on teardown

Try server tells me that this seems to cause this failure:

test-reply-to-list-from-address-selection.js | test_Reply_To_List_From_Address
  EXCEPTION: Timed out waiting for window open!
    at: test-window-helpers.js line 193
       WindowWatcher_waitForWindowOpen("msgcompose") test-window-helpers.js 193
       wait_for_new_window("msgcompose") test-window-helpers.js 504
       wait_for_compose_window() test-compose-helpers.js 207
       open_compose_with_reply_to_list() test-compose-helpers.js 125
       test_Reply_To_List_From_Address() test-reply-to-list-from-address-selection.js 132
            frame.js 474
            frame.js 530
            frame.js 573
            frame.js 417
            frame.js 579
            server.js 164
            server.js 168

Definitely on Mac and probably on Linux. The Window's failure I'm not too sure about because I've got a test commented out there.

Otherwise the general idea looks reasonable, although I'm not sure the folder pane likes the way we're removing and re-adding those too much.

I wonder if it would just be better to have this test in a separate directory where we somehow flag that the tests affect the default set-up.

Comment 11

[Jim Porter (:squib)]

(In reply to comment #10)
> I wonder if it would just be better to have this test in a separate directory
> where we somehow flag that the tests affect the default set-up.

This is probably a good idea, since it would be simpler, and I'm not sure there really is a good way to temporarily delete accounts (doing it for identities is pretty simple, though).

Comment 12

[Jim Porter (:squib)]

Attachment: Give up and run the tests in their own directory

Ok, I moved the tests to their own directory, which should eliminate the problems.

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 510361 [details] [diff] [review]
Give up and run the tests in their own directory

It might be an idea to add a readme.txt to say that things in this directory don't necessarily tidy up after themselves.

Comment 14

[Jim Porter (:squib)]

Attachment: Add a readme telling people to be careful when writing new tests

Ok, I added a readme. Pulling r+ forward.

Comment 15

[Jim Porter (:squib)]

... and checkin-needed, finally!

Comment 16

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/2aa02a11be39

Thanks Jim.