New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate test_lib_apps.py, HOTP and TOTP tests #1415

Merged
merged 4 commits into from Feb 4, 2019

Conversation

Projects
None yet
3 participants
@fredreichbier
Copy link
Member

fredreichbier commented Feb 1, 2019

This migrates test_lib_tokens_{hotp,totp}.py and test_lib_apps.py.

@fredreichbier fredreichbier requested a review from privacyidea/core Feb 1, 2019

r = token.get_otp(current_time=datetime.datetime.now())
self.assertTrue(r > 47251648, r)
# simple OTPs of current time
# TODO: Not sure if that's what should be tested here

This comment has been minimized.

@fredreichbier

fredreichbier Feb 1, 2019

Author Member

This is a bit tricky here. The assertion r > 47251648 apparently always succeeded in Python 2, because r is a tuple:

>>> (1, "pin", "otp", "pinotp") > 47251648
True

But under Python 3, tuples and ints are incomparable:

>>> (1, "pin", "otp", "pinotp") > 47251648
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: '>' not supported between instances of 'tuple' and 'int'

so the test above fails.

So I tried to find out what should be tested here: If I understand correctly, the test was intended to check that the TOTP counter value is bigger than 47251648. I tried to fix the test accordingly, but I'm not sure if it makes sense. @cornelinux you got any idea?

This comment has been minimized.

@cornelinux

cornelinux Feb 1, 2019

Member

Yes, it is probably the OTP counter. I also stumbled upon this at another python3 PR I issued some days ago.

This comment has been minimized.

@fredreichbier

fredreichbier Feb 4, 2019

Author Member

OK thanks, then I'll just keep the updated test and remove the TODO comment.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 1, 2019

Codecov Report

Merging #1415 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1415      +/-   ##
=========================================
+ Coverage   96.78%   96.8%   +0.02%     
=========================================
  Files         144     144              
  Lines       17185   17183       -2     
=========================================
+ Hits        16632   16634       +2     
+ Misses        553     549       -4
Impacted Files Coverage Δ
privacyidea/lib/apps.py 100% <100%> (ø) ⬆️
privacyidea/lib/utils.py 96.84% <100%> (+0.91%) ⬆️
privacyidea/lib/tokens/hotptoken.py 100% <100%> (ø) ⬆️

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 911ac0d...0a4db6a. Read the comment docs.

Show resolved Hide resolved privacyidea/lib/apps.py Outdated
Show resolved Hide resolved privacyidea/lib/apps.py Outdated
@plettich
Copy link
Contributor

plettich left a comment

looks good.

@fredreichbier fredreichbier force-pushed the python3_tokens_hotp branch from d04a7be to 0a4db6a Feb 4, 2019

@plettich plettich merged commit 4133f5e into master Feb 4, 2019

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 96.78%)
Details
codecov/project 96.8% (+0.02%) compared to 911ac0d
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@plettich plettich deleted the python3_tokens_hotp branch Feb 4, 2019

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