owncloud / twofactor_totp

🔑 Second factor TOTP (Google Authenticator) provider for ownCloud
GNU Affero General Public License v3.0
9 stars 9 forks source link

Wait till verify msg is visible #116

Closed phil-davis closed 5 years ago

phil-davis commented 5 years ago

Issue #117 webUI acceptance test scenario webUITwoFactorTOTP/keyVerification.feature:11 fails intermittently. https://drone.owncloud.com/owncloud/twofactor_totp/210/112

The DOM element that has the "Verified" text can exist but not yet be visible. We need to wait until it is visible before trying to get the text.

Also fix a couple of minor typos in method names.

codecov[bot] commented 5 years ago

Codecov Report

Merging #116 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #116   +/-   ##
=========================================
  Coverage     64.68%   64.68%           
  Complexity       61       61           
=========================================
  Files            13       13           
  Lines           252      252           
=========================================
  Hits            163      163           
  Misses           89       89

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8df8b51...92d6e6d. Read the comment docs.

codecov[bot] commented 5 years ago

Codecov Report

Merging #116 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #116   +/-   ##
=========================================
  Coverage     64.68%   64.68%           
  Complexity       61       61           
=========================================
  Files            13       13           
  Lines           252      252           
=========================================
  Hits            163      163           
  Misses           89       89

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8df8b51...92d6e6d. Read the comment docs.

phil-davis commented 5 years ago

PR #115 runs the scenario a lot of times, and it passes every time.

skshetry commented 5 years ago

As far as I remember, the Verified message is only visible for a few seconds. Idk if that'll be a problem, but, the test may be fast to take a note of the message.

P.S: I wonder why the test is failing now continuously there as it passed every time for last 2-3 months.

phil-davis commented 5 years ago

P.S: I wonder why the test is failing now continuously there as it passed every time for last 2-3 months.

I wonder that also. Because we automatically rerun UI tests, we only really notice a failure if it happens twice in the same run. To get a reasonable amount of fails I had to run the scenario 5 times.

The extra waiting until visible fixes it - I did not get any fails. So I guess that the code was looking too early for the "Verified" text. The text does stay there for a few seconds, so I do not expect that waiting properly for it to become visible will take us near the time when the message disappears.