Closed Bug 1396358 Opened 2 years ago Closed 2 years ago

Check why whole test suite was affected when test-drafts.js::test_remove_space_stuffing_format_flowed() failed

Categories

(Thunderbird :: Testing Infrastructure, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 57.0

People

(Reporter: jorgk-bmo, Assigned: aceman)

Details

Attachments

(1 file)

In bug 1396218 we had test-drafts.js::test_remove_space_stuffing_format_flowed() fail and it affected 15 subsequent tests in the same directory. That made the diagnosis of the real issue harder than necessary.

Let's fix that.
Flags: needinfo?(acelists)
All tests putting messages in Drafts or Outbox expects their created message to be the first one in the folder (notice all the select_click_row(0); calls), they do not bother to find their message by some other means (e.g. by message-Id). It would be a bit tedious. So if any previous test fails and does not cleanup these folders after itself, all tests following it using these folders will fail.

Thankfully I have consolidated getting hold of these special folders via get_special_folder() in the past, so it should be easy to cleanup the folders before first use.
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Flags: needinfo?(acelists)
OS: Unspecified → All
Hardware: Unspecified → All
I'm not sure why the failing test doesn't clean up. Like in this example
  if (!bodyText.includes("NoSpace<br> OneSpace<br>  TwoSpaces")) {
    assert_true(false, "Something went wrong with space stuffing");
  }
  press_delete(mc);
the assert failed. Does that stop execution?
Yes I think so. assert_true throws an exception if the condition is not satisfied.
https://dxr.mozilla.org/comm-central/source/mail/test/mozmill/shared-modules/test-folder-display-helpers.js#2888
Hmm. That's not how, for example, M-C Mochitests work. There you can test things and the test counts the failures. Sure, you might want to have an assert so the test cannot continue, but in the case I quoted, it could run on. Anyway, too late now to change the concept here.
But often it makes no sense for the particular test function to continue running, it would break in the next step.
Yes, here the next step is press_delete that could work, but that is a really trivial example.
You can split logical parts that are independent into separate test_ functions. If one function fails, then next still runs.
Maybe we could split out cleanup code into separate functions, but that would again need to inspect all tests as is non-trivial.
Comment on attachment 8904080 [details] [diff] [review]
patch

This looks good :-)
Attachment #8904080 - Flags: review?(jorgk) → review+
Keywords: checkin-needed
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/c3e90be7d781
clean up special folders (e.g. Inbox, Drafts, Outbox) at start to remove potential leftovers of prior failed tests. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 57.0
You need to log in before you can comment on or make changes to this bug.