628204 - Finish off about:support tests

Status: RESOLVED FIXED

(Thunderbird :: Testing Infrastructure, defect)

Description

[Rain [:rain1] (inactive)]

The compose window in particular needs some testing.

Comment 1

[Rain [:rain1] (inactive)]

Attachment: [checked in] tests for send via email and get file system type

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.

Comment 2

[Blake Winton (: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.

Comment 3

[Rain [:rain1] (inactive)]

(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 4

[Rain [:rain1] (inactive)]

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

Comment 5

[Rain [:rain1] (inactive)]

I had to disable the xpcshell test on linux due to test failures on tinderbox: https://hg.mozilla.org/comm-central/rev/566ea47fce36

Comment 6

[Rain [:rain1] (inactive)]

Attachment: tests for get account details and showing them in order

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.

Comment 7

[Rain [:rain1] (inactive)]

https://hg.mozilla.org/comm-central/rev/61974ce9eb84

That should be it for this bug, I think.

Comment 8

[Rain [:rain1] (inactive)]

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