1339163 - Make TPS check restmail for verification emails

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P2)

Description

[Thom Chiovoloni [:tcsc] (ex-moco)]

This plan was the outcome of some email discussion and https://github.com/mozilla/fxa/issues/216.

Comment 1

[Thom Chiovoloni [:tcsc] (ex-moco)]

Attachment: Bug 1339163 - Make TPS tests attempt to automatically verify fxa emails when using a restmail account

Review commit: https://reviewboard.mozilla.org/r/121506/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/121506/

Comment 2

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8848614 [details]
Bug 1339163 - Make TPS tests attempt to automatically verify fxa emails when using a restmail account

https://reviewboard.mozilla.org/r/121506/#review124324

Looks great, thanks! Can you please ping Karl and see what steps he thinks we need to take to get this stood up?

::: services/sync/tps/extensions/tps/resource/auth/fxaccounts.jsm:60
(Diff revision 1)
> +    userData = this.getSignedInUser();
> +    return userData && userData.verified;
> +  },
> +
> +  async _openVerificationPage(uri) {
> +    // Better way of doing this?

nope - although Services.wm is probably preferred to the .getService() dance.

::: services/sync/tps/extensions/tps/resource/auth/fxaccounts.jsm:69
(Diff revision 1)
> +    let newtab = mainWindow.getBrowser().addTab(uri);
> +    let win = mainWindow.getBrowser().getBrowserForTab(newtab);
> +    let resolve;
> +    await new Promise(res => {
> +      resolve = res;
> +      win.addEventListener("loadend", resolve, true);

You can use win.addEventListener("loadend", resolve, { once: true }); and skip the remove.

::: services/sync/tps/extensions/tps/resource/auth/fxaccounts.jsm:76
(Diff revision 1)
> +    win.removeEventListener("loadend", resolve);
> +    if (await this.shortWaitForVerification(10000)) {
> +      mainWindow.getBrowser().removeTab(newtab);
> +      return true;
> +    }
> +    return false;

Can you add a comment saying why you don't close the tab - I guess we leave the tab open to help with diagnosis?

::: services/sync/tps/extensions/tps/resource/auth/fxaccounts.jsm:191
(Diff revision 1)
>      Async.promiseSpinningly(FxAccountsConfig.ensureConfigured());
>  
>      let client = new FxAccountsClient();
>      client.signIn(account["username"], account["password"], true).then(credentials => {
>        return fxAccounts.setSignedInUser(credentials);
> -    }).then(() => {
> +    })

I really prefer |)}.then...|, but I don't mind if you feel strongly the leading . is better :)

Comment 3

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment on attachment 8848614 [details]
Bug 1339163 - Make TPS tests attempt to automatically verify fxa emails when using a restmail account

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/121506/diff/1-2/

Comment 4

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment on attachment 8848614 [details]
Bug 1339163 - Make TPS tests attempt to automatically verify fxa emails when using a restmail account

https://reviewboard.mozilla.org/r/121506/#review124324

> nope - although Services.wm is probably preferred to the .getService() dance.

That was the part I meant.

> You can use win.addEventListener("loadend", resolve, { once: true }); and skip the remove.

I had recently learned I could do this, then promptly forgot, thanks.

> Can you add a comment saying why you don't close the tab - I guess we leave the tab open to help with diagnosis?

It should have always closed the tab, I think. Rewrote so that it will.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/81e0dfa05b19
Make TPS tests attempt to automatically verify fxa emails when using a restmail account r=markh

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/81e0dfa05b19