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

Update daplug token tests to work with Python 3 #1423

Merged
merged 2 commits into from Feb 5, 2019

Conversation

Projects
None yet
2 participants
@plettich
Copy link
Contributor

plettich commented Feb 4, 2019

Working on #676

@plettich plettich requested a review from privacyidea/core Feb 4, 2019

@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 4, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@367f334). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1423   +/-   ##
=========================================
  Coverage          ?   96.78%           
=========================================
  Files             ?      144           
  Lines             ?    17185           
  Branches          ?        0           
=========================================
  Hits              ?    16633           
  Misses            ?      552           
  Partials          ?        0
Impacted Files Coverage Δ
privacyidea/lib/tokens/daplugtoken.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 367f334...913b996. Read the comment docs.

def _daplug2digit(daplug_otp):
hex_otp = ""
for i in daplug_otp:
digit = MAPPING.get(i)
hex_otp += digit
otp = binascii.unhexlify(hex_otp)
# we know the result is a string of digits
otp = to_unicode(binascii.unhexlify(hex_otp))

This comment has been minimized.

@cornelinux

cornelinux Feb 5, 2019

Member

We have a function hexlify_and_unicode --- should we have a similar function unhexlify_and_unicode?
Till now there seems only one appearance of such a construct in the code.

grep -r unhexlify privacyidea/* | grep unicode

This comment has been minimized.

@plettich

plettich Feb 5, 2019

Author Contributor

The problem is, that we can't be sure if it is a printable string that gets returned from unhexlify. It might as well be a cryptographic key in a bytes-representation.
So this would depend very much on the context.

This comment has been minimized.

@cornelinux

cornelinux Feb 5, 2019

Member

Actually, I understand this ;-)

@cornelinux
Copy link
Member

cornelinux left a comment

It looks good to me. I would merge unless you think we should do something about my one comment.
But it even does not look that important to me :-)

@cornelinux cornelinux merged commit fe367df into master Feb 5, 2019

0 of 3 checks passed

ci/circleci CircleCI is running your tests
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@cornelinux cornelinux deleted the python3_update_daplug_test branch Feb 5, 2019

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