Skip to content
This repository has been archived by the owner. It is now read-only.

Tests: Password reset tests sometimes ask for sign-in #2483

Closed
vladikoff opened this issue May 27, 2015 · 18 comments
Closed

Tests: Password reset tests sometimes ask for sign-in #2483

vladikoff opened this issue May 27, 2015 · 18 comments
Assignees

Comments

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented May 27, 2015

Shane linked the following code that we may investigate:

Refs: https://github.com/mozilla/fxa-content-server/blob/master/app/scripts/views/confirm_reset_password.js#L76

Refs: https://github.com/mozilla/fxa-content-server/blob/master/app/scripts/views/confirm_reset_password.js#L227

Happens only on TeamCity. Regular password reset and webchannel password reset tests are affected.

No clear to reproduce this yet. This error happens more when running a lot of tests (i.e using more memory). Slowing down the network does not help. Running this test on its own threw an error once out of ~12 times tried.

@pdehaan pdehaan added the tests label May 27, 2015
@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Jun 3, 2015

I updated the vnc driver, firefox and rebooted the test machine and this issue went away

@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Jul 23, 2015

This still happens:
image

Which causes this failure:

× firefox on any platform - reset_password flow - reset password, verify same browser (34.779s)
NoSuchElement: [POST http://localhost:4444/wd/hub/session/f0e91fb7-b642-4b55-88c7-f0895737a6ba/element / {"using":"css selector","value":"#fxa-settings-header"}] Unable to locate element: {"method":"css selector","selector":"#fxa-settings-header"}
For documentation on this error, please visit: http://seleniumhq.org/exceptions/no_such_element.html
Build info: version: '2.45.0', revision: '5017cb8', time: '2015-02-26 23:59:50'
System info: host: 'vladikoff', ip: '192.168.1.174', os.name: 'Mac OS X', os.arch: 'x86_64', os.version: '10.10.3', java.version: '1.7.0_67'
Driver info: driver.version: unknown
  at ProxiedSession._post  <node_modules/intern/node_modules/leadfoot/Session.js:59:30>
  at ProxiedSession.Session.find  <node_modules/intern/node_modules/leadfoot/Session.js:1055:15>
  at Command.<anonymous>  <node_modules/intern/node_modules/leadfoot/Command.js:42:36>
  at <node_modules/intern/node_modules/dojo/Promise.ts:393:15>
  at Object.run [as _onImmediate]  <node_modules/intern/node_modules/dojo/Promise.ts:237:7>
  at processImmediate [as _immediateCallback]  <timers.js:354:15>
  at Command.find  <node_modules/intern/node_modules/leadfoot/Command.js:23:10>
  at Command.prototype.(anonymous function) [as findByCssSelector]  <node_modules/intern/node_modules/leadfoot/lib/strategies.js:24:16>
  at testAtSettingsWithVerifiedMessage  <tests/functional/reset_password.js:104:8>
  at Command.<anonymous>  <tests/functional/reset_password.js:373:18>
  at runCallback  <node_modules/intern/node_modules/leadfoot/Command.js:517:31>
  at Command.<anonymous>  <node_modules/intern/node_modules/leadfoot/Command.js:534:11>
  at <node_modules/intern/node_modules/dojo/Promise.ts:393:15>
  at Object.run [as _onImmediate]  <node_modules/intern/node_modules/dojo/Promise.ts:237:7>
  at processImmediate [as _immediateCallback]  <timers.js:354:15>
  at Command.then  <node_modules/intern/node_modules/leadfoot/Command.js:533:10>
  at Command.<anonymous>  <tests/functional/reset_password.js:106:48>
  at runCallback  <node_modules/intern/node_modules/leadfoot/Command.js:517:31>
  at Command.<anonymous>  <node_modules/intern/node_modules/leadfoot/Command.js:536:11>
  at <node_modules/intern/node_modules/dojo/Promise.ts:393:15>
  at Object.run [as _onImmediate]  <node_modules/intern/node_modules/dojo/Promise.ts:237:7>
  at processImmediate [as _immediateCallback]  <timers.js:354:15>
  at Command.then  <node_modules/intern/node_modules/leadfoot/Command.js:533:10>
  at testAtSettingsWithVerifiedMessage  <tests/functional/reset_password.js:105:8>
  at Command.<anonymous>  <tests/functional/reset_password.js:373:18>
  at runCallback  <node_modules/intern/node_modules/leadfoot/Command.js:517:31>
  at Command.<anonymous>  <node_modules/intern/node_modules/leadfoot/Command.js:534:11>
  at <node_modules/intern/node_modules/dojo/Promise.ts:393:15>
  at Object.run [as _onImmediate]  <node_modules/intern/node_modules/dojo/Promise.ts:237:7>
  at processImmediate [as _immediateCallback]  <timers.js:354:15>
  at Command.then  <node_modules/intern/node_modules/leadfoot/Command.js:533:10>
  at Test.registerSuite.reset password, verify same browser [as test]  <tests/functional/reset_password.js:372:10>
  at start.then.finally.self.timeElapsed  <node_modules/intern/lib/Test.js:211:24>
  at <node_modules/intern/node_modules/dojo/Promise.ts:393:15>
  at runCallbacks  <node_modules/intern/node_modules/dojo/Promise.ts:11:11>
  at <node_modules/intern/node_modules/dojo/Promise.ts:317:4>
  at Object.run [as _onImmediate]  <node_modules/intern/node_modules/dojo/Promise.ts:237:7>
  at processImmediate [as _immediateCallback]  <timers.js:354:15>
@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Jul 23, 2015

@shane-tomlinson , I need help tracking this down

@vladikoff vladikoff added the label Jul 23, 2015
@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Jul 23, 2015

By taking screenshots on test failures we are able to see what went wrong, here is another one from TeamCity:
2015_07_23_081323_4305

Log: http://tests.dev.lcip.org:8111/viewLog.html?buildId=4464&buildTypeId=FxaLatest_FunctionalTests&tab=buildLog#_focus=388

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Jul 26, 2015

Let's put this on the priority queue to set aside time, I think this could
easily turn into a time sink. I wonder if this is related to the other
cached credential problems we see.

On Thu, Jul 23, 2015 at 4:14 PM, Vlad Filippov notifications@github.com
wrote:

By taking screenshots on test failures we are able to see what went wrong,
here is another one from TeamCity:
[image: 2015_07_23_081323_4305]
https://cloud.githubusercontent.com/assets/128755/8854065/f1c754ae-312b-11e5-9555-11a3eee81ff5.png


Reply to this email directly or view it on GitHub
#2483 (comment)
.

@vladikoff vladikoff added this to the train 44 milestone Aug 7, 2015
@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Aug 7, 2015

fixed via #2895

@vladikoff vladikoff closed this Aug 7, 2015
@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Sep 8, 2015

This still happens :(

@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Sep 8, 2015

If we cannot find a fix we should modify tests to expect both a "sign in" and "settings views instead of settings

@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Sep 17, 2015

@shane-tomlinson seems like 10 seconds is not enough of a timeout for functional tests running against a remote browser: https://github.com/mozilla/fxa-content-server/blob/master/app/scripts/views/confirm_reset_password.js#L24

What should we do?

shane-tomlinson pushed a commit that referenced this issue Sep 21, 2015
…sts.

There were several problems:

1. Because promises are async, setTimeout could be called after the module was
   torn down, which caused completion logic to be run after it was expected.
1. The timer-mixin called clearAllTimeouts without a context when used as
   a `destroy` handler, which meant outstanding timeouts were not cleared on
   view destroy.
1. The unit tests had a lot of cross test pollution. New dependencies are set
   up for each test now.
1. The logic did not take into account that the `login` message could arrive
   from the other tab before, during, or after the XHR request that checks
   whether the user has verified. There were no tests for all of these cases
   either.

This fix takes a new approach to polling:

When the user attempts a password reset, we have no idea whether the
user is going to verify in the same browser. The only way we know if
the user verified in this browser is if a `login` message is received
from the inter-tab channel.

Because we have no idea if the user will verify in this browser,
assume they will not. Start polling the server to see if the user has
verified. If the server eventually reports the user has successfully
reset their password, assume the user has completed in a different
browser. In this case the best we can do is ask the user to sign in
again. Once the user has entered their updated creds, we can then
notify the browser.

If a `complete_reset_password_tab_open` message is received, hooray,
the user has opened the password reset link in this browser. At this
point we can assume the user will complete verification in this
browser. It's not 100% certain the user will complete, but most
likely. Stop polling the server. The server poll is no longer needed,
and in fact makes things more complex. Instead, wait for the `login`
message that will arrive once the user finishes the password reset.

Once the `login` message has arrived, notify the browser. BOOM.

fixes #2483
shane-tomlinson pushed a commit that referenced this issue Sep 21, 2015
…sts.

There were several problems:

1. Because promises are async, setTimeout could be called after the module was
   torn down, which caused completion logic to be run after it was expected.
1. The timer-mixin called clearAllTimeouts without a context when used as
   a `destroy` handler, which meant outstanding timeouts were not cleared on
   view destroy.
1. The unit tests had a lot of cross test pollution. New dependencies are set
   up for each test now.
1. The logic did not take into account that the `login` message could arrive
   from the other tab before, during, or after the XHR request that checks
   whether the user has verified. There were no tests for all of these cases
   either.

This fix takes a new approach to polling:

When the user attempts a password reset, we have no idea whether the
user is going to verify in the same browser. The only way we know if
the user verified in this browser is if a `login` message is received
from the inter-tab channel.

Because we have no idea if the user will verify in this browser,
assume they will not. Start polling the server to see if the user has
verified. If the server eventually reports the user has successfully
reset their password, assume the user has completed in a different
browser. In this case the best we can do is ask the user to sign in
again. Once the user has entered their updated creds, we can then
notify the browser.

If a `complete_reset_password_tab_open` message is received, hooray,
the user has opened the password reset link in this browser. At this
point we can assume the user will complete verification in this
browser. It's not 100% certain the user will complete, but most
likely. Stop polling the server. The server poll is no longer needed,
and in fact makes things more complex. Instead, wait for the `login`
message that will arrive once the user finishes the password reset.

Once the `login` message has arrived, notify the browser. BOOM.

fixes #2483
shane-tomlinson pushed a commit that referenced this issue Sep 21, 2015
…sts.

There were several problems:

1. Because promises are async, setTimeout could be called after the module was
   torn down, which caused completion logic to be run after it was expected.
1. The timer-mixin called clearAllTimeouts without a context when used as
   a `destroy` handler, which meant outstanding timeouts were not cleared on
   view destroy.
1. The unit tests had a lot of cross test pollution. New dependencies are set
   up for each test now.
1. The logic did not take into account that the `login` message could arrive
   from the other tab before, during, or after the XHR request that checks
   whether the user has verified. There were no tests for all of these cases
   either.

This fix takes a new approach to polling:

When the user attempts a password reset, we have no idea whether the
user is going to verify in the same browser. The only way we know if
the user verified in this browser is if a `login` message is received
from the inter-tab channel.

Because we have no idea if the user will verify in this browser,
assume they will not. Start polling the server to see if the user has
verified. If the server eventually reports the user has successfully
reset their password, assume the user has completed in a different
browser. In this case the best we can do is ask the user to sign in
again. Once the user has entered their updated creds, we can then
notify the browser.

If a `complete_reset_password_tab_open` message is received, hooray,
the user has opened the password reset link in this browser. At this
point we can assume the user will complete verification in this
browser. It's not 100% certain the user will complete, but most
likely. Stop polling the server. The server poll is no longer needed,
and in fact makes things more complex. Instead, wait for the `login`
message that will arrive once the user finishes the password reset.

Once the `login` message has arrived, notify the browser. BOOM.

fixes #2483
@vladikoff vladikoff reopened this Sep 24, 2015
@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Sep 24, 2015

Seems like #3083 fix was not enough

Running more tests...

@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Sep 29, 2015

Still happens both on Teamcity 8111 and Travis :(

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Sep 30, 2015

Huh, and I just spent the day thinking "Seems like travis is mostly fixed." What a PITA.

@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Oct 23, 2015

wooo so greeeeen

@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Nov 2, 2015

This was fixed by Shane! @shane-tomlinson woo hoo

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Nov 2, 2015

FIXEEEDDDD!!!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.