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

Make certificate tests run with python 3 #1418

Merged
merged 4 commits into from Feb 5, 2019

Conversation

Projects
None yet
2 participants
@plettich
Copy link
Contributor

plettich commented Feb 1, 2019

This also fixes the certificate token tests.

Working on #676

Make certificate tests run with python 3
This also fixes the certificatetoken tests.

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

filename += value+"_"
filename = filename[:-1] + "." + file_extension
return filename
filename = "_".join([value.decode('utf8') for (key, value) in name_components])

This comment has been minimized.

@cornelinux

cornelinux Feb 1, 2019

Member

Here you are doing a decdode. When should this be a to_unicode?

This comment has been minimized.

@plettich

plettich Feb 1, 2019

Author Contributor

You are right, very inconsequential of me...

This comment has been minimized.

@plettich

plettich Feb 4, 2019

Author Contributor

fixed in 1fb2839

@@ -402,15 +401,9 @@ def sign_request(self, csr, options=None):
csr_obj.get_subject(), file_extension="pem")
#csr_extensions = csr_obj.get_extensions()
csr_filename = csr_filename.replace(" ", "_")
if type(csr_filename) == str:
csr_filename = csr_filename.decode("utf-8")
csr_filename = csr_filename.encode("ascii", "ignore")

This comment has been minimized.

@cornelinux

cornelinux Feb 1, 2019

Member

So why would this suddently be not necessary under python 2.7 anymore?

This comment has been minimized.

@plettich

plettich Feb 1, 2019

Author Contributor

You are right, i misinterpreted this. Since we are using the updated path later on there should be no trouble. I'll fix this.

This comment has been minimized.

@plettich

plettich Feb 4, 2019

Author Contributor

fixed in 1fb2839

This comment has been minimized.

@cornelinux

cornelinux Feb 5, 2019

Member

I think we also should use to_unicode instead of decode?

@cornelinux
Copy link
Member

cornelinux left a comment

I am not sure, if we can remove the lines, where the file name gets created out of the contents of the certificate!

@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 1, 2019

Codecov Report

Merging #1418 into master will decrease coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1418      +/-   ##
=========================================
- Coverage   96.78%   96.7%   -0.09%     
=========================================
  Files         144     144              
  Lines       17183   17177       -6     
=========================================
- Hits        16631   16611      -20     
- Misses        552     566      +14
Impacted Files Coverage Δ
privacyidea/lib/tokens/certificatetoken.py 93.75% <100%> (+0.11%) ⬆️
privacyidea/lib/caconnectors/localca.py 96.44% <100%> (-0.11%) ⬇️
privacyidea/lib/log.py 34.24% <0%> (-16.44%) ⬇️
privacyidea/lib/security/default.py 96.11% <0%> (-1.67%) ⬇️
privacyidea/lib/subscriptions.py 87.2% <0%> (-0.8%) ⬇️
privacyidea/lib/crypto.py 95.57% <0%> (-0.35%) ⬇️
privacyidea/lib/tokens/u2f.py 95.83% <0%> (+2.5%) ⬆️

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 4133f5e...41c9198. Read the comment docs.

plettich added some commits Feb 4, 2019

Fix remarks from review
- The encode to 'ascii' ensures, that the file name is ascii only. Since
  this returns a byte string, we have to decode it again.
@cornelinux

This comment has been minimized.

Copy link
Member

cornelinux commented on privacyidea/lib/caconnectors/localca.py in 1fb2839 Feb 5, 2019

Should this be

csr_filename = to_unicode(csr_filename.encode('ascii', 'ignore'))

@cornelinux cornelinux merged commit 367f334 into master Feb 5, 2019

1 of 3 checks passed

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
ci/circleci Your tests passed on CircleCI!
Details

@cornelinux cornelinux deleted the python3_update_ca_tests branch Feb 5, 2019

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