Closed Bug 628204 Opened 11 years ago Closed 11 years ago

Finish off about:support tests

Categories

(Thunderbird :: Testing Infrastructure, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.3a3

People

(Reporter: rain1, Assigned: rain1)

References

Details

Attachments

(2 files)

The compose window in particular needs some testing.
Tests for getting the account details are going to require a bit more work, and I think getting at least the compose tests in is important.
Attachment #507465 - Flags: review?(bwinton)
Comment on attachment 507465 [details] [diff] [review]
[checked in] tests for send via email and get file system type

>+++ b/mail/components/about-support/content/aboutSupport.xhtml
>@@ -82,17 +82,17 @@
>     <div>
>       <button onclick="copyToClipboard()">
>         &aboutSupport.copyToClipboard.label;
>       </button>
>-      <button onclick="sendViaEmail()">
>+      <button id="button-send-via-email" onclick="sendViaEmail()">

Should we add an id for the copyToClipboard button too, or will that be in a separate patch?

>+++ b/mail/test/mozmill/content-tabs/test-about-support.js
>@@ -89,16 +95,29 @@ function open_about_support() {
>+/**
>+ * Opens a compose window containing the troubleshooting information.
>+ *
>+ * @param aTab The about:support tab.
>+ */
>+function open_send_via_email(aTab) {
>+  let button = content_tab_e(aTab, "button-send-via-email");
>+  plan_for_new_window("msgcompose");
>+  mc.click(new elib.Elem(button));

So, the other controllers seem to add both "e" and "eid", one of which will return the element, and the other return the elib.Elem-wrapped element, so that you don't need to import elib into your tests.  I'm not going to insist you change this patch, but if you wanted to, I would certainly prefer that.

Finally, test_send_via_email_public and test_send_via_email_private seem kind of similar, but I guess they’re different enough not to merge into one function if you would prefer not to…

So, in summary, r=me as long as you think about what I said.  ;)

Later,
Blake.
Attachment #507465 - Flags: review?(bwinton) → review+
(In reply to comment #2)
> Comment on attachment 507465 [details] [diff] [review]
> tests for send via email and get file system type
> 
> >+++ b/mail/components/about-support/content/aboutSupport.xhtml
> >@@ -82,17 +82,17 @@
> >     <div>
> >       <button onclick="copyToClipboard()">
> >         &aboutSupport.copyToClipboard.label;
> >       </button>
> >-      <button onclick="sendViaEmail()">
> >+      <button id="button-send-via-email" onclick="sendViaEmail()">
> 
> Should we add an id for the copyToClipboard button too, or will that be in a
> separate patch?

Won't be necessary -- we don't use that button because we don't want to overwrite the clipboard.

> So, the other controllers seem to add both "e" and "eid", one of which will
> return the element, and the other return the elib.Elem-wrapped element, so that
> you don't need to import elib into your tests.  I'm not going to insist you
> change this patch, but if you wanted to, I would certainly prefer that.

OK, I've added a content_tab_eid.
 
> Finally, test_send_via_email_public and test_send_via_email_private seem kind
> of similar, but I guess they’re different enough not to merge into one function
> if you would prefer not to…

Yeah, I'd prefer not to: it generally makes debugging easier.
Comment on attachment 507465 [details] [diff] [review]
[checked in] tests for send via email and get file system type

https://hg.mozilla.org/comm-central/rev/2b2a8d9a56a8
Attachment #507465 - Attachment description: tests for send via email and get file system type → [checked in] tests for send via email and get file system type
I had to disable the xpcshell test on linux due to test failures on tinderbox: https://hg.mozilla.org/comm-central/rev/566ea47fce36
Depends on: 630963
Depends on: 632494
Depends on: 633346
Blake's a little busy right now, so passing the review on to bienvenu :)

This patch requires the following patches, in order: bug 630963, bug 633345, bug 633346.
Attachment #507465 - Attachment is obsolete: true
Attachment #511682 - Flags: review?(bienvenu)
Attachment #507465 - Attachment is obsolete: false
Attachment #511682 - Flags: review?(bienvenu) → review+
https://hg.mozilla.org/comm-central/rev/61974ce9eb84

That should be it for this bug, I think.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a3
The tests were failing on Linux so I disabled them there. I've pushed a followup to fix the issue:
https://hg.mozilla.org/comm-central/rev/ed86a911f353
You need to log in before you can comment on or make changes to this bug.